Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc

Index: chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
diff --git a/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc b/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
index 8950f6056b3ee81d32bb519614442893a0688a86..cff3a474b2334f765bb416dc22cf2c34fcfe5ac9 100644
--- a/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
+++ b/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
@@ -567,11 +567,12 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
 
 // This test creates a page with multiple child frames and adds an <input> to
 // each frame. Then, sequentially, each <input> is focused by sending a tab key.
-// Then, after |TextInputState.type| for a view is changed to text, the test
-// sends a set composition IPC to the active widget and waits until the widget
-// updates its composition range.
+// Then, after |TextInputState.type| for a view is changed to text, another key
+// is pressed (a character) and then the test verifies that TextInputManager
+// receives the corresponding update on the change in selection bounds on the
+// browser side.
 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
-                       TrackCompositionRangeForAllFrames) {
+                       TrackSelectionBoundsForAllFrames) {
   CreateIframePage("a(b,c(a,b),d)");
   std::vector<content::RenderFrameHost*> frames{
       GetFrame(IndexVector{}),     GetFrame(IndexVector{0}),
@@ -581,38 +582,34 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
   for (auto* frame : frames)
     views.push_back(frame->GetView());
   for (size_t i = 0; i < frames.size(); ++i)
-    AddInputFieldToFrame(frames[i], "text", "text", true);
+    AddInputFieldToFrame(frames[i], "text", "", true);
 
   content::WebContents* web_contents = active_contents();
 
-  auto send_tab_set_composition_wait_for_bounds_change = [&web_contents](
+  auto send_tab_insert_text_wait_for_bounds_change = [&web_contents](
       content::RenderWidgetHostView* view) {
     ViewTextInputTypeObserver type_observer(web_contents, view,
                                             ui::TEXT_INPUT_TYPE_TEXT);
     SimulateKeyPress(web_contents, ui::DomKey::TAB, ui::DomCode::TAB,
                      ui::VKEY_TAB, false, false, false, false);
     type_observer.Wait();
-
-    ViewCompositionRangeChangedObserver range_observer(web_contents, view);
-    EXPECT_TRUE(content::RequestCompositionInfoFromActiveWidget(web_contents));
-    range_observer.Wait();
+    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();

[erikchen, 2016/08/17 23:31:16]

I know that this is the same test we're using on Aura, but should we be checking
some properties of the new selection bounds, rather than just checking that the
observer got a callback? Ditto for TrackCompositionRangeForAllFrames.

[EhsanK, 2016/08/22 14:48:28]

That is a good question. I think we should have tests which validate the IME
experience we expect in response to the input we are sending to the renderer.
But I think they should stay as separate tests.

IMO, the tests here should only verify that any generated IME state by an
<iframe> is tracked by TextInputManager on the browser side. I think keeping
these specific tests at this level of complexity is better since mixing the
tracking and correctness of the tracked state might make a test too complicated.
But that is only my opinion.

   };
 
   for (auto* view : views)
-    send_tab_set_composition_wait_for_bounds_change(view);
+    send_tab_insert_text_wait_for_bounds_change(view);
 }
 
-// TODO(ekaramad): Enable the following tests on other platforms when the
-// corresponding feature is implemented (http://crbug.com/578168).
-#if defined(USE_AURA)
 // This test creates a page with multiple child frames and adds an <input> to
 // each frame. Then, sequentially, each <input> is focused by sending a tab key.
-// Then, after |TextInputState.type| for a view is changed to text, another key
-// is pressed (a character) and then the test verifies that TextInputManager
-// receives the corresponding update on the change in selection bounds on the
-// browser side.
+// Then, after |TextInputState.type| for a view is changed to text, the test
+// sends a set composition IPC to the active widget and waits until the widget
+// updates its composition range.
 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
-                       TrackSelectionBoundsForAllFrames) {
+                       TrackCompositionRangeForAllFrames) {
   CreateIframePage("a(b,c(a,b),d)");
   std::vector<content::RenderFrameHost*> frames{
       GetFrame(IndexVector{}),     GetFrame(IndexVector{0}),
@@ -622,27 +619,30 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
   for (auto* frame : frames)
     views.push_back(frame->GetView());
   for (size_t i = 0; i < frames.size(); ++i)
-    AddInputFieldToFrame(frames[i], "text", "", true);
+    AddInputFieldToFrame(frames[i], "text", "text", true);
 
   content::WebContents* web_contents = active_contents();
 
-  auto send_tab_insert_text_wait_for_bounds_change = [&web_contents](
+  auto send_tab_set_composition_wait_for_bounds_change = [&web_contents](
       content::RenderWidgetHostView* view) {
     ViewTextInputTypeObserver type_observer(web_contents, view,
                                             ui::TEXT_INPUT_TYPE_TEXT);
     SimulateKeyPress(web_contents, ui::DomKey::TAB, ui::DomCode::TAB,
                      ui::VKEY_TAB, false, false, false, false);
     type_observer.Wait();
-    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();
+
+    ViewCompositionRangeChangedObserver range_observer(web_contents, view);
+    EXPECT_TRUE(content::RequestCompositionInfoFromActiveWidget(web_contents));
+    range_observer.Wait();
   };
 
   for (auto* view : views)
-    send_tab_insert_text_wait_for_bounds_change(view);
+    send_tab_set_composition_wait_for_bounds_change(view);
 }
 
+// TODO(ekaramad): Enable the following tests on other platforms when the
+// corresponding feature is implemented (http://crbug.com/578168).
+#if defined(USE_AURA)
 // This test creates a page with multiple child frames and adds an <input> to
 // each frame. Then, sequentially, each <input> is focused by sending a tab key.
 // After focusing each input, a sequence of key presses (character 'E') are sent