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

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 "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ skipping 549 matching lines @@
   set_state_observer.Wait();
 
   TextInputManagerTypeObserver reset_state_observer(active_contents(),
                                                     ui::TEXT_INPUT_TYPE_NONE);
   ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
   reset_state_observer.Wait();
 }
 
 // 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}),
       GetFrame(IndexVector{1}),    GetFrame(IndexVector{1, 0}),
       GetFrame(IndexVector{1, 1}), GetFrame(IndexVector{2})};
   std::vector<content::RenderWidgetHostView*> views;
   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}),
       GetFrame(IndexVector{1}),    GetFrame(IndexVector{1, 0}),
       GetFrame(IndexVector{1, 1}), GetFrame(IndexVector{2})};
   std::vector<content::RenderWidgetHostView*> views;
   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
 // to the focused widget and then the whole text is selected using Ctrl+A. The
 // test then verifies that the selection length equals the length of the
 // sequence of 'E's.
 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
                        TrackTextSelectionForAllFrames) {
   CreateIframePage("a(b,c(a,b),d)");
   std::vector<content::RenderFrameHost*> frames{
@@ skipping 202 matching lines @@
 
   // 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());
 }
 #endif  // USE_AURA