Track TextInputState from multiple RenderWidgets in TextInputManager (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"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/public/test/text_input_test_utils.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "ui/base/ime/composition_underline.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/base/ime/text_input_mode.h"
 #include "ui/base/ime/text_input_type.h"
 #include "url/gurl.h"
 
-// TODO(ekaramad): The following tests are only active on aura platforms. After
-// fixing crbug.com/578168 for all platforms, the following tests should be
-// activated for other platforms, e.g., Mac and Android (crbug.com/602723).
+// TODO(ekaramad): The following tests should be active on all platforms. After
+// fixing https://crbug.com/578168, this test file should be built for android
+// as well and most of the following tests should be enabled for all platforms
+//(https://crbug.com/602723).
 
 ///////////////////////////////////////////////////////////////////////////////
 // TextInputManager and IME Tests
 //
 // The following tests verify the correctness of TextInputState tracking on the
 // browser side. They also make sure the IME logic works correctly. The baseline
 // for comparison is the default functionality in the non-OOPIF case (i.e., the
 // legacy implementation in RWHV's other than RWHVCF).
 // These tests live outside content/ because they rely on being part of the
 // interactive UI test framework (to avoid flakiness).
@@ skipping 470 matching lines @@
   EXPECT_TRUE(ExecuteScript(
       main_frame, "document.querySelector('iframe').src = 'about:blank'"));
   child_reset_state_observer.Wait();
 }
 
 // This test creates a blank page and adds an <input> to it. Then, the <input>
 // is focused and the test verfies that the |TextInputState.type| is set to
 // ui::TEXT_INPUT_TYPE_TEXT. Next, the browser is navigated away and the test
 // verifies that |TextInputState.type| resets to ui::TEXT_INPUT_TYPE_NONE.
 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
                        ResetStateAfterBrowserNavigation) {

[Charlie Reis, 2016/07/27 16:14:18]

Ok, glad to see the tests here and above are running on Mac now.

[EhsanK, 2016/07/27 16:46:32]

Yes, and these should also run on Android when we implement IME for android.

   CreateIframePage("a()");
   content::RenderFrameHost* main_frame = GetFrame(IndexVector{});
   AddInputFieldToFrame(main_frame, "text", "", false);
 
   TextInputManagerTypeObserver set_state_observer(active_contents(),
                                                   ui::TEXT_INPUT_TYPE_TEXT);
   SimulateKeyPress(active_contents(), ui::DomKey::TAB, ui::DomCode::TAB,
                    ui::VKEY_TAB, false, false, false, false);
   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();
 }
 
+// TODO(ekaramad): Enable the following tests on other platforms when the
+// corresponding feature is implemented (http://crbug.com/578168).
+#if defined(USE_AURA)

[EhsanK, 2016/07/27 16:46:32]

I guess all of the following will eventually be active run on Mac as well. But
some, if not all, will not run on Android as the selection related state is
updated differently on Android. So we will have to deal with some #ifdefs for
life.

 // 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{
@@ skipping 90 matching lines @@
     ViewTextSelectionObserver text_selection_observer(
         web_contents, frame->GetView(), expected_length);
     SimulateKeyPress(web_contents, ui::DomKey::TAB, ui::DomCode::TAB,
                      ui::VKEY_TAB, false, false, false, false);
     text_selection_observer.Wait();
   };
 
   for (size_t i = 0; i < frames.size(); ++i)
     send_tab_and_wait_for_selection_change(frames[i], input_text[i].size());
 }
+#endif  // USE_AURA
 
 // 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).

[Charlie Reis, 2016/07/27 16:14:18]

This part confuses me a bit.  Is there a meaningful difference between the
Aura-only block above and the Aura-only block below?  Both seem to have TODOs to
make them work on other platforms, so I'm wondering if it's simpler to combine
them.

(Or do the InputMethodObserver-specific tests deserve a block of their own?  I'm
ok with that if so.)

[EhsanK, 2016/07/27 16:46:32]

Sorry for the confusion. The block above is Aura only because I have not
implemented the features. I expect them to move to Mac + Aura 1by1 as I finish
IME for Mac.

The ones below were originally intended to be InputMethodObserver tests as you
suggest. But as of now I have the only one below for aura which is actually a
unit test. I guess I should be able to move it to unit tests and get rid of this
block for good.

It will be ideal though to have proper end-to-end tests for IME which will only
use IME extension (which only works on ChromeOS for now). I will try to revisit
this issue after I investigate the IME api in more depths. Although, the IME API
has its own tests for ChromeOS.

 
+#if defined(USE_AURA)
 // 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)) {
   }
 
   void Wait() {
     if (success_)
@@ skipping 38 matching lines @@
  private:
   DISALLOW_COPY_AND_ASSIGN(InputMethodObserverForShowIme);
 };
 
 // This test verifies that the IME for Aura is shown if and only if the current
 // client's |TextInputState.type| is not ui::TEXT_INPUT_TYPE_NONE and the flag
 // |TextInputState.show_ime_if_needed| is true. This should happen even when
 // the TextInputState has not changed (according to the platform), e.g., in
 // aura when receiving two consecutive updates with same |TextInputState.type|.
 // TODO(ekaramad): This test is actually a unit test not necessarily an OOPIF
 // test. We should move it to somewhere more relevant.

[EhsanK, 2016/07/27 16:46:31]

I might remove this test and put it with other unit tests as soon as I figure
out the window focus issue I had before. I have added the bug number to this
TODO btw.

 IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
                        CorrectlyShowImeIfNeeded) {
   // We only need the <iframe> page to create RWHV.
   CreateIframePage("a()");
   content::RenderFrameHost* main_frame = GetFrame(IndexVector{});
   content::RenderWidgetHostView* view = main_frame->GetView();
   content::WebContents* web_contents = active_contents();
 
   content::TextInputStateSender sender(view);
 
@@ skipping 26 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());
 }
+#endif