Tracking composition range on the browser side (Aura)

chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ skipping 497 matching lines @@
     ViewSelectionBoundsChangedObserver bounds_observer(web_contents, view);
     SimulateKeyPress(web_contents, ui::DomKey::FromCharacter('E'),
                      ui::DomCode::US_E, ui::VKEY_E, false, false, false, false);
     bounds_observer.Wait();
   };
 
   for (auto view : views)
     send_tab_insert_text_wait_for_bounds_change(view);
 }
 
+// TODO(ekaramad): Add a test here to show that the composition range is being
+// tracked for all of the child frames.Composition range update could occur in

[Charlie Reis, 2016/07/08 18:55:24]

nit: Missing space between sentences.

[EhsanK, 2016/07/11 18:07:21]

Acknowledged.

+// response to WebWidget::setComposition. Write a test when that method is
+// implemented for WebFrameWidgetImpl (crbug.com/602723).

[Charlie Reis, 2016/07/08 18:55:24]

This comment would be clearer if you started with the prerequisite for writing
the test.  (At first, it sounds like we could just write it now if we wanted
to.)  Maybe something like:

Once WebWidget::setComposition is implemented for WebFrameWidgetImpl, add a test
here to show that...

Also, is there a bug you can reference for that particular work, as opposed to
just the "add tests" bug?

And is there anything else you can test in this CL?  If there's nothing
observable, maybe we should be fixing WebFrameWidgetImpl first?

[EhsanK, 2016/07/11 18:07:21]

Fair enough. After giving it some more thought, I guess a dangling TODO does not
look really nice.

I did add the test I had in mind, but right now it is only on a page with one
frame (main frame). It passes because WebViewImpl takes care of IME for that. I
added a short TODO with a new bug number to say that we should change the test
after that bug is fixed.

We could also wait for the IME CL to land first. But I am not sure how long
would that be.

[Charlie Reis, 2016/07/11 23:56:19]

Acknowledged.

+
 // TODO(ekaramad): The following tests are specifically written for Aura and are
 // based on InputMethodObserver. Write similar tests for Mac/Android/Mus
 // (crbug.com/602723).
 
 // Observes current input method for state changes.
 class InputMethodObserverBase {
  public:
   explicit InputMethodObserverBase(content::WebContents* web_contents)
       : success_(false),
         test_observer_(content::TestInputMethodObserver::Create(web_contents)) {
@@ skipping 89 matching lines @@
   EXPECT_FALSE(send_and_check_show_ime());
 
   // Set |TextInputState.show_ime_if_needed|. Expect IME.
   sender.SetShowImeIfNeeded(true);
   EXPECT_TRUE(send_and_check_show_ime());
 
   // Set |TextInputState.type| to ui::TEXT_INPUT_TYPE_NONE. Expect no IME.
   sender.SetType(ui::TEXT_INPUT_TYPE_NONE);
   EXPECT_FALSE(send_and_check_show_ime());
 }