Track text selection 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 584 matching lines @@
 
     ViewCompositionRangeChangedObserver range_observer(web_contents, view);
     EXPECT_TRUE(content::RequestCompositionInfoFromActiveWidget(web_contents));
     range_observer.Wait();
   };
 
   for (auto* view : views)
     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.
-// 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,
-                       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", "", true);
-
-  content::WebContents* web_contents = active_contents();
-
-  auto send_tab_insert_text_wait_for_bounds_change = [&web_contents](
-      content::RenderWidgetHostView* view) {

[EhsanK, 2016/08/17 21:36:00]

This test is just copied over (actually not copied just looks like it) in green
(should be intact).

-    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();
-  };
-
-  for (auto* view : views)
-    send_tab_insert_text_wait_for_bounds_change(view);
-}
-
 // 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.
+// to the focused widget. 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{
       GetFrame(IndexVector{}),     GetFrame(IndexVector{0}),
       GetFrame(IndexVector{1}),    GetFrame(IndexVector{1, 0}),
       GetFrame(IndexVector{1, 1}), GetFrame(IndexVector{2})};
   std::vector<std::string> values{"main", "b", "c", "ca", "cb", "d"};
   std::vector<content::RenderWidgetHostView*> views;
   for (auto* frame : frames)
@@ skipping 11 matching lines @@
   };
 
   auto send_keys_select_all_wait_for_selection_change = [&web_contents](
       content::RenderWidgetHostView* view, size_t count) {
     ViewTextSelectionObserver observer(web_contents, view, count);
     for (size_t i = 0; i < count; ++i) {
       SimulateKeyPress(web_contents, ui::DomKey::FromCharacter('E'),
                        ui::DomCode::US_E, ui::VKEY_E, false, false, false,
                        false);
     }
-    // Send Ctrl+A to select the whole text.
-    SimulateKeyPress(web_contents, ui::DomKey::FromCharacter('a'),
-                     ui::DomCode::US_A, ui::VKEY_A, true, false, false, false);

[EhsanK, 2016/08/17 21:36:00]

This was removed since it was not needed nor correctly working.

     observer.Wait();
   };
 
   size_t count = 2;
   for (size_t i = 0; i < views.size(); ++i) {
     // First focus the <input>.
     send_tab_and_wait_for_value(values[i]);
 
     // Send a sequence of |count| 'E' keys and wait until the view receives a
     // selection change update for a text of the corresponding size, |count|.
     send_keys_select_all_wait_for_selection_change(views[i], count++);
   }
 }
 
+// TODO(ekaramad): Enable the following tests on other platforms when the

[sky, 2016/08/17 20:42:52]

Is there a compelling reason to move the test here? It makes the diff harder to
follow.

[EhsanK, 2016/08/17 21:36:00]

Sorry for the ugly diff. Hmmm...actually, the intent was to move
TrackTextSelectionForAllFrames from inside #ifdef USE_AURA to outside #ifdef
USE_AURA.

I will keep this diff issue in mind for the upcoming CL which moves
TrackSelectionBounds outside of USE_AURA.

[sky, 2016/08/17 23:32:47]

Any chance you could add ifdefs instead? Again, it makes the review easier.

[EhsanK, 2016/08/18 14:43:29]

Done (assuming that I understood the suggestion correctly).

+// 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.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       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", "", true);
+
+  content::WebContents* web_contents = active_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();
+    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);
+}
+
 // The following test verifies that when the active widget changes value, it is
 // always from nullptr to non-null or vice versa.
 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
                        ResetTextInputStateOnActiveWidgetChange) {
   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;
@@ skipping 148 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