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

[Charlie Reis, 2016/07/22 20:59:35]

This looks stale now.

[EhsanK, 2016/07/25 17:12:02]

Acknowledged.

 // fixing crbug.com/578168 for all platforms, the following tests should be
 // activated for other platforms, e.g., Mac and Android (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).
@@ skipping 459 matching lines @@
   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).

[kenrb, 2016/07/21 21:48:16]

Is it much extra to implement all of it in this CL? It appears to be just a few
more changes to RenderWidgetHostViewMac.

[Charlie Reis, 2016/07/22 20:59:35]

I'm ok either way, but I have a slight preference to do it incrementally like
this.  The smaller CLs have been easy to review when they have less going on
(assuming the intermediate stages are safe).

[EhsanK, 2016/07/25 17:12:02]

I have fixes for some of the other IME-related tracking in Mac but not all. I
figured 1) make CLs small and 2) land changes asap so that I can catch
regressions sooner. My own suggestion is to have at least TextInputState
tracking on a different CL. I guess I will be able to manage to fit all the
other state trackings in one CL.

That being said I guess I should be able to have one CL for all of Mac by the
end of the week if that is the direction we are going.

[Charlie Reis, 2016/07/25 19:43:55]

Let's keep the CLs small and proceed incrementally if possible.

+#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{
@@ skipping 55 matching lines @@
         ViewCompositionRangeChangedObserver range_observer(web_contents, view);
         content::SetCompositionForRenderWidgetHost(
             view->GetRenderWidgetHost(), base::ASCIIToUTF16("text"),
             {ui::CompositionUnderline()}, gfx::Range::InvalidRange(), 0, 0);
         range_observer.Wait();
       };
 
   for (auto view : views)
     send_tab_set_composition_wait_for_bounds_change(view);
 }
+#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).
 
+#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 85 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