[reland] Browser Side Text Input State Tracking for OOPIF (Aura Only)

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
new file mode 100644
index 0000000000000000000000000000000000000000..45500fd84d7bb57ba721d41dee027c5e164dc332
--- /dev/null
+++ b/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
@@ -0,0 +1,537 @@
+// 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 "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/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/text_input_client.h"
+#include "ui/base/ime/text_input_mode.h"
+#include "ui/base/ime/text_input_type.h"
+
+#include "url/gurl.h"
+
+#ifdef USE_AURA

[Charlie Reis, 2016/05/26 06:22:04]

Having the whole file ifdef'd out on other platforms looks a bit weird.  We
could restrict it to Aura in the build files, but it's probably better to just
put a TODO here to enable it on more platforms (since we intend to do that
soon).  I think you had one before that you could put here.

[EhsanK, 2016/05/30 15:06:07]

Yes. I think I had that TODO in the original file but missed it when moved the
tests to this one.

+///////////////////////////////////////////////////////////////////////////////
+// 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.).
+
+namespace {
+// TextInputManager Observers

[Charlie Reis, 2016/05/26 06:22:04]

nit: Blank line before and after.

[EhsanK, 2016/05/30 15:06:07]

Done.

+// Observing the |TextInputState.value|.

[Charlie Reis, 2016/05/26 06:22:04]

Please put a more thorough comment here.  This is a base class for several types
of observers below, each of which may wait for a different
TextInputState-related event.

[EhsanK, 2016/05/30 15:06:07]

Done. This comment was wrong to begin with.

+class TextInputManagerObserverBase {
+ public:
+  explicit TextInputManagerObserverBase(content::WebContents* web_contents)
+      : success_(false) {
+    test_observer_ =
+        content::TestTextInputManagerObserver::Create(web_contents);
+  }
+
+  virtual ~TextInputManagerObserverBase() {}
+
+  void Wait() {

[Charlie Reis, 2016/05/26 06:22:04]

// Wait for the derived class's definition of success, at which point it will
run on_success().

[EhsanK, 2016/05/30 15:06:08]

I am a bit unclear on the second statement in this comment. What Wait() does is
to wait for the derived class's definition of success. It does not run
on_success().

+    if (success_)
+      return;
+    message_loop_runner_ = new content::MessageLoopRunner();
+    message_loop_runner_->Run();
+  }
+
+  bool success() const { return success_; }
+
+ protected:
+  base::Closure on_success() {

[Charlie Reis, 2016/05/26 06:22:04]

Why does this return a closure, rather than just making OnSuccess() protected
and calling it directly?

If there's a reason, please document it, but hopefully we can avoid it to
simplify this.

[EhsanK, 2016/05/30 15:06:07]

No reasons and calling OnSuccess() is better.

Initially, I was using external functions and statics to verify conditions and
passing a closure was the way to go. Now, as you pointed out, it is not the case
so it better to just make it protected  and call it directly in subclasses'
verification methods.

+    return base::Bind(&TextInputManagerObserverBase::OnSuccess,
+                      base::Unretained(this));
+  }
+
+  content::TestTextInputManagerObserver* observer() {
+    return test_observer_.get();
+  }
+
+ private:
+  void OnSuccess() {
+    success_ = true;
+    if (message_loop_runner_)
+      message_loop_runner_->Quit();
+  }
+
+  std::unique_ptr<content::TestTextInputManagerObserver> test_observer_;
+  bool success_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(TextInputManagerObserverBase);
+};
+
+// This class observes TextInputManager for updated |TextInputState.value|.
+class TextInputManagerValueObserver : public TextInputManagerObserverBase {
+ public:
+  explicit TextInputManagerValueObserver(content::WebContents* web_contents,

[Charlie Reis, 2016/05/26 06:22:03]

No explicit.

[EhsanK, 2016/05/30 15:06:08]

Done.

+                                         const std::string& expected_value)
+      : TextInputManagerObserverBase(web_contents),
+        expected_value_(expected_value) {
+    observer()->SetUpdateTextInputStateCalledCallback(base::Bind(
+        &TextInputManagerValueObserver::VerifyValue, base::Unretained(this)));
+  }
+
+ private:
+  void VerifyValue(content::TestTextInputManagerObserver* observer) {
+    std::string value;
+    if (observer->GetTextInputValue(&value) && expected_value_ == value)
+      on_success().Run();
+  }
+
+  std::string expected_value_;
+};
+
+// This class observes the TextInputManager for updated |TextInputState.type|.
+class TextInputManagerTypeObserver : public TextInputManagerObserverBase {
+ public:
+  explicit TextInputManagerTypeObserver(content::WebContents* web_contents,
+                                        ui::TextInputType expected_type)
+      : TextInputManagerObserverBase(web_contents),
+        expected_type_(expected_type) {
+    observer()->SetUpdateTextInputStateCalledCallback(base::Bind(
+        &TextInputManagerTypeObserver::VerifyType, base::Unretained(this)));
+  }
+
+ private:
+  void VerifyType(content::TestTextInputManagerObserver* observer) {
+    if (expected_type_ == observer->GetTextInputType())
+      on_success().Run();
+  }
+
+  ui::TextInputType expected_type_;
+};
+
+// An observer class which observes the TextInputManager until the first time
+// the TextInputManager detects a change in TextInputState.
+class TextInputManagerChangeObserver : public TextInputManagerObserverBase {
+ public:
+  explicit TextInputManagerChangeObserver(content::WebContents* web_contents)
+      : TextInputManagerObserverBase(web_contents) {
+    observer()->SetUpdateTextInputStateCalledCallback(base::Bind(
+        &TextInputManagerChangeObserver::VerifyChange, base::Unretained(this)));
+  }
+
+ private:
+  void VerifyChange(content::TestTextInputManagerObserver* observer) {
+    if (observer->IsTextInputStateChanged())
+      on_success().Run();
+  }
+};
+
+// The following class observers |TextInputState.type| for a specific RWHV.

[Charlie Reis, 2016/05/26 06:22:04]

nit: observes

[EhsanK, 2016/05/30 15:06:08]

Done.

+class ViewTextInputTypeObserver : public TextInputManagerObserverBase {
+ public:
+  explicit ViewTextInputTypeObserver(content::WebContents* web_contents,
+                                     content::RenderWidgetHostView* rwhv,
+                                     ui::TextInputType expected_type)
+      : TextInputManagerObserverBase(web_contents),
+        web_contents_(web_contents),
+        view_(rwhv),
+        expected_type_(expected_type) {
+    observer()->SetUpdateTextInputStateCalledCallback(base::Bind(
+        &ViewTextInputTypeObserver::VerifyType, base::Unretained(this)));
+  }
+
+ private:
+  void VerifyType(content::TestTextInputManagerObserver* observer) {
+    std::unordered_map<content::RenderWidgetHostView*, ui::TextInputType>
+        type_map = content::GetTextInputTypeMapFromWebContents(web_contents_);

[Charlie Reis, 2016/05/26 06:22:03]

Can we replace this API with a GetTextInputTypeForView(web_contents, view) (and
optionally a HasInputTypeForView if you want to check that as well)?  That would
avoid copying the whole map and exposing it, and it seems to meet the needs of
this test file.

[EhsanK, 2016/05/30 15:06:07]

Done. Replacing the API with a simple function seems like a much better idea. 

I am only adding a GetTextInputTypeForView(view, &type) which will return false
if we cannot access view's TextInputState (e.g., not registered or host or
host->delegate() are missing).

+    if (expected_type_ == type_map[view_])
+      on_success().Run();
+  }
+
+  content::WebContents* web_contents_;
+  content::RenderWidgetHostView* view_;
+  ui::TextInputType expected_type_;
+};
+
+}  // namespace
+
+// Main class for all TextInputState and IME related tests.
+class SitePerProcessTextInputManagerTest : public InProcessBrowserTest {
+ public:
+  SitePerProcessTextInputManagerTest() {}
+  ~SitePerProcessTextInputManagerTest() override {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    content::IsolateAllSitesForTesting(command_line);
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+
+    // Add content/test/data 'cross_site_iframe_factory.html'.
+    embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+
+ protected:
+  content::WebContents* active_contents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
+  }
+
+  // static
+  // Adds an <input> field to a given frame by executing javascript code.
+  // The input can be added as the first element or the last element of
+  // |document.body|.
+  static void AddInputFieldToFrame(content::RenderFrameHost* rfh,
+                                   const std::string& type,
+                                   const std::string& value,
+                                   bool append_as_first_child) {
+    std::string script = base::StringPrintf(
+        "var input = document.createElement('input');"
+        "input.setAttribute('type', '%s');"
+        "input.setAttribute('value', '%s');"
+        "if (%s && !!document.body.firstChild) {"

[Charlie Reis, 2016/05/26 06:22:04]

Maybe "document.body.%s", with the third param being append_as_first_child ?
"insertBefore(...)" : "appendChild(...)".

[EhsanK, 2016/05/30 15:06:07]

Agreed. Looks confusing the way it was.

+        "  document.body.insertBefore(input, document.body.firstChild);"
+        "} else {"
+        "  document.body.appendChild(input);"
+        "}",
+        type.c_str(), value.c_str(), append_as_first_child ? "true" : "false");
+    EXPECT_TRUE(ExecuteScript(rfh, script));
+  }
+
+  // Uses 'cross_site_iframe_factory.html'. The main frame's domain is
+  // 'a.com'.
+  void CreateIframePage(const std::string& structure) {
+    std::string path = base::StringPrintf("/cross_site_iframe_factory.html?%s",
+                                          structure.c_str());
+    GURL main_url(embedded_test_server()->GetURL("a.com", path));
+    ui_test_utils::NavigateToURL(browser(), main_url);
+  }
+
+  // Recrusively uses ChildFrameAt(frame, i) to get the i-th child frame

[Charlie Reis, 2016/05/26 06:22:04]

nit: Recursively
Actually, Iteratively, since this isn't recursive.

[EhsanK, 2016/05/30 15:06:08]

Done.

+  // inside frame. For example, for 'a(b(c, d(e)))', [0] returns b, and
+  // [0, 1, 0] returns e;
+  content::RenderFrameHost* GetFrame(const std::vector<size_t>& indices) {
+    content::RenderFrameHost* current = active_contents()->GetMainFrame();
+    for (size_t index : indices)
+      current = ChildFrameAt(current, index);
+    return current;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SitePerProcessTextInputManagerTest);
+};
+
+// The following test loads a page with multiple nested <iframe> elements which
+// are in or out of process with the main frame. Then an <input> field with
+// unique value is added to every single frame on the frame tree. The test then
+// creates a sequence of tab presses and verifies that after each key press, the
+// TextInputState.value reflects that of the focused input, i.e., the
+// TextInputManager is correctly tracking TextInputState across frames.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       TrackStateWhenSwitchingFocusedFrames) {
+  CreateIframePage("a(a,b,c(a,b,d(e, f)),g)");
+  std::vector<std::string> values{
+      "main",     "node_a",   "node_b",     "node_c",     "node_c_a",
+      "node_c_b", "node_c_d", "node_c_d_e", "node_c_d_f", "node_g"};
+
+  // TODO(ekaramad): This should not be needed and uniform initialization should
+  // work. However, in some bots, this is failing.
+  using vector = std::vector<size_t>;

[Charlie Reis, 2016/05/26 06:22:04]

nit: Blank line after, since it looks like the TODO applies to the whole block.

[EhsanK, 2016/05/30 15:06:07]

Done.

+  std::vector<content::RenderFrameHost*> frames{
+      GetFrame(vector{}),        GetFrame(vector{0}),
+      GetFrame(vector{1}),       GetFrame(vector{2}),
+      GetFrame(vector{2, 0}),    GetFrame(vector{2, 1}),
+      GetFrame(vector{2, 2}),    GetFrame(vector{2, 2, 0}),
+      GetFrame(vector{2, 2, 1}), GetFrame(vector{3})};
+
+  for (size_t i = 0; i < frames.size(); ++i)
+    AddInputFieldToFrame(frames[i], "text", values[i], true);
+
+  for (size_t i = 0; i < frames.size(); ++i) {
+    TextInputManagerValueObserver observer(active_contents(), values[i]);
+    SimulateKeyPress(active_contents(), ui::VKEY_TAB, false, false, false,
+                     false);
+    observer.Wait();
+  }
+}
+
+// The following test loads a page with two OOPIFs. An <input> is added to
+// both frames and tab key is faked until the one in the second OOPIF is

[Charlie Reis, 2016/05/26 06:22:04]

s/faked/pressed/?  (I think it's implicit that we're simulating.)

[EhsanK, 2016/05/30 15:06:07]

Done. Don't we do manual testing of chrome though? TEST=Press tab key until...
(just kidding ;))

+// focused. Then, the renderer process for both frames are crashed. The test

[Charlie Reis, 2016/05/26 06:22:04]

nit: processes

[EhsanK, 2016/05/30 15:06:07]

Done.

+// verifies that the TextInputManager stop tracking the RWHVs as well as

[Charlie Reis, 2016/05/26 06:22:03]

nit: stops

[EhsanK, 2016/05/30 15:06:07]

Done.

+// properly resetting the TextInputState after the second (active) RWHV goes

[Charlie Reis, 2016/05/26 06:22:04]

nit: resets

[EhsanK, 2016/05/30 15:06:08]

Done.

+// away.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       StopTrackingCrashedChildFrame) {
+  CreateIframePage("a(b, c)");
+  std::vector<std::string> values{"node_b", "node_c"};
+  using vector = std::vector<size_t>;
+  std::vector<content::RenderFrameHost*> frames{GetFrame(vector{0}),
+                                                GetFrame(vector{1})};
+
+  for (size_t i = 0; i < frames.size(); ++i)
+    AddInputFieldToFrame(frames[i], "text", values[i], true);
+
+  // Tab into both inputs. And make sure we correctly receive their
+  // TextInputState. For the second tab two IPC's arrive one from the first

[Charlie Reis, 2016/05/26 06:22:04]

nit: arrive:

[EhsanK, 2016/05/30 15:06:07]

Done.

+  // frame to set the state to none, and another one from the second frame to
+  // set it to TEXT. To avoid the race between them, we shall also observe the
+  // first frame setting its state to NONE after the second tab.
+  ViewTextInputTypeObserver view_type_observer(
+      active_contents(), frames[0]->GetView(), ui::TEXT_INPUT_TYPE_NONE);
+
+  for (size_t i = 0; i < frames.size(); ++i) {
+    TextInputManagerValueObserver observer(active_contents(), values[i]);
+    SimulateKeyPress(active_contents(), ui::VKEY_TAB, false, false, false,
+                     false);
+    observer.Wait();
+  }
+
+  // Make sure that the first view has set its TextInputState.type to NONE.
+  view_type_observer.Wait();
+
+  // Verify that we are tracking the TextInputState from the first frame.
+  content::RenderWidgetHostView* first_view = frames[0]->GetView();
+  std::unordered_map<content::RenderWidgetHostView*, ui::TextInputType>
+      type_map = content::GetTextInputTypeMapFromWebContents(active_contents());
+  EXPECT_EQ(1UL, type_map.count(first_view));
+  EXPECT_EQ(ui::TEXT_INPUT_TYPE_NONE, type_map[first_view]);
+
+  // Now that the second frame's <input> is focused, we crash the first frame
+  // and observe that text input state is updated for the view.
+  std::unique_ptr<content::RenderWidgetHostViewDestructionObserver>
+      destruction_observer =
+          content::RenderWidgetHostViewDestructionObserver::Create(first_view);
+  frames[0]->GetProcess()->Shutdown(0, false);
+  destruction_observer->Wait();
+
+  // Verifying that the TextInputManager has smaller cleaned the memory

[Charlie Reis, 2016/05/26 06:22:03]

Typo?

[EhsanK, 2016/05/30 15:06:08]

Sorry...typo seems to be an understatement in this case. 

+  // allocated to the view.
+  type_map = content::GetTextInputTypeMapFromWebContents(active_contents());
+  EXPECT_EQ(0UL, type_map.count(first_view));
+
+  // Now crash the second <iframe> which has an active view.
+  content::RenderWidgetHostView* second_view = frames[1]->GetView();
+  TextInputManagerChangeObserver change_observer(active_contents());
+  frames[1]->GetProcess()->Shutdown(0, false);
+  change_observer.Wait();
+  EXPECT_EQ(ui::TEXT_INPUT_TYPE_NONE,
+            content::GetTextInputTypeFromWebContents(active_contents()));
+  EXPECT_FALSE(!!content::GetActiveViewFromWebContents(active_contents()));
+  type_map = content::GetTextInputTypeMapFromWebContents(active_contents());
+  EXPECT_EQ(0UL, type_map.count(second_view));
+}
+
+// The following test loads a page with two child frames; one in process and one

[Charlie Reis, 2016/05/26 06:22:04]

nit: Colon, not semicolon.  (The second phrase would need to be a complete
sentence of its own to use a semicolon.)

[EhsanK, 2016/05/30 15:06:07]

Acknowledged.

+// out of process with main frame. The test inserts an <input> inside each frame
+// and focuses the first frame and observes the TextInputManager setting the
+// state to ui::TEXT_INPUT_TYPE_TEXT. Then, the frame is detached and the test
+// observes that the state type is reset to ui::TEXT_INPUT_TYPE_NONE. The same
+// sequence of actions is then performed on the out of process frame.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       ResetStateAfterFrameDetached) {
+  CreateIframePage("a(a, b)");
+  using vector = std::vector<size_t>;
+  std::vector<content::RenderFrameHost*> frames{GetFrame(vector{0}),
+                                                GetFrame(vector{1})};
+
+  for (size_t i = 0; i < frames.size(); ++i)
+    AddInputFieldToFrame(frames[i], "text", "", true);
+
+  // Press tab key to focus the <input> in the first frame.
+  TextInputManagerTypeObserver type_observer_text_a(active_contents(),
+                                                    ui::TEXT_INPUT_TYPE_TEXT);
+  SimulateKeyPress(active_contents(), ui::VKEY_TAB, false, false, false, false);
+  type_observer_text_a.Wait();
+
+  // Detach first frame and observe |TextInputState.type| resetting to
+  // ui::TEXT_INPUT_TYPE_NONE.
+  TextInputManagerTypeObserver type_observer_none_a(active_contents(),
+                                                    ui::TEXT_INPUT_TYPE_NONE);
+  EXPECT_TRUE(ExecuteScript(
+      frames[0], "document.body.removeChild(document.body.firstChild);"));
+  type_observer_none_a.Wait();
+
+  // Press tab to focus the <input> in the second frame.
+  TextInputManagerTypeObserver type_observer_text_b(active_contents(),
+                                                    ui::TEXT_INPUT_TYPE_TEXT);
+  SimulateKeyPress(active_contents(), ui::VKEY_TAB, false, false, false, false);
+  type_observer_text_b.Wait();
+
+  // Detach first frame and observe |TextInputState.type| resetting to
+  // ui::TEXT_INPUT_TYPE_NONE.
+  TextInputManagerTypeObserver type_observer_none_b(active_contents(),
+                                                    ui::TEXT_INPUT_TYPE_NONE);
+  EXPECT_TRUE(ExecuteScript(
+      frames[1], "document.body.removeChild(document.body.firstChild);"));
+  type_observer_none_b.Wait();
+}
+
+// This test creates a page with one OOPIF 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 child frame is navigated away and the
+// test verifies that |TextInputState.type| resets to ui::TEXT_INPUT_TYPE_NONE.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       ResetStateAfterChildNavigation) {
+  CreateIframePage("a(b)");
+  using vector = std::vector<size_t>;
+  content::RenderFrameHost* main_frame = GetFrame(vector{});
+  content::RenderFrameHost* child_frame = GetFrame(vector{0});
+
+  AddInputFieldToFrame(child_frame, "text", "child", false);
+
+  // Focus <input> in child frame and verify the |TextInputState.value|.
+  TextInputManagerValueObserver child_set_state_observer(active_contents(),
+                                                         "child");
+  SimulateKeyPress(active_contents(), ui::VKEY_TAB, false, false, false, false);
+  child_set_state_observer.Wait();
+
+  // Navigate the child frame to about:blank and verify that TextInputManager
+  // correctly sets its |TextInputState.type| to ui::TEXT_INPUT_TYPE_NONE.
+  TextInputManagerTypeObserver child_reset_state_observer(
+      active_contents(), ui::TEXT_INPUT_TYPE_NONE);
+  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) {
+  CreateIframePage("a()");
+  content::RenderFrameHost* main_frame = GetFrame(std::vector<size_t>{});
+  AddInputFieldToFrame(main_frame, "text", "", false);
+
+  TextInputManagerTypeObserver set_state_observer(active_contents(),
+                                                  ui::TEXT_INPUT_TYPE_TEXT);
+  SimulateKeyPress(active_contents(), 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();
+}
+#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).
+#ifdef 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_)
+      return;
+    message_loop_runner_ = new content::MessageLoopRunner();
+    message_loop_runner_->Run();
+  }
+
+  bool success() const { return success_; }
+
+ protected:
+  content::TestInputMethodObserver* test_observer() {
+    return test_observer_.get();
+  }
+
+  const base::Closure success_closure() {
+    return base::Bind(&InputMethodObserverBase::OnSuccess,
+                      base::Unretained(this));
+  }
+
+ private:
+  void OnSuccess() {
+    success_ = true;
+    if (message_loop_runner_)
+      message_loop_runner_->Quit();
+  }
+
+  bool success_;
+  std::unique_ptr<content::TestInputMethodObserver> test_observer_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(InputMethodObserverBase);
+};
+
+class InputMethodObserverForShowIme : public InputMethodObserverBase {
+ public:
+  explicit InputMethodObserverForShowIme(content::WebContents* web_contents)
+      : InputMethodObserverBase(web_contents) {
+    test_observer()->SetOnShowImeIfNeededCallback(success_closure());
+  }
+};
+
+// 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.

[Charlie Reis, 2016/05/26 06:22:04]

Even if what?

[EhsanK, 2016/05/30 15:06:07]

Done.

+// TODO(ekaramad): This test is actually a unit test and should be moved to some
+// place more appropriate.
+IN_PROC_BROWSER_TEST_F(SitePerProcessTextInputManagerTest,
+                       CorrectlyShowImeIfNeeded) {
+  // We only need the <iframe> page to create RWHV.
+  CreateIframePage("a()");
+  content::RenderFrameHost* main_frame = GetFrame(std::vector<size_t>{});
+  content::RenderWidgetHostView* view = main_frame->GetView();
+  content::WebContents* web_contents = active_contents();
+
+  content::TextInputStateSender sender(view);
+
+  auto send_and_check_show_ime = [&sender, &web_contents]() {
+    InputMethodObserverForShowIme observer(web_contents);
+    sender.Send();
+    return observer.success();
+  };
+
+  // Sending an empty state should not trigger ime.
+  EXPECT_FALSE(send_and_check_show_ime());
+
+  // Set |TextInputState.type| to text. Expect no IME.
+  sender.SetType(ui::TEXT_INPUT_TYPE_TEXT);
+  EXPECT_FALSE(send_and_check_show_ime());
+
+  // Set |TextInputState.show_ime_if_needed| to true. Expect IME.
+  sender.SetShowImeIfNeeded(true);
+  EXPECT_TRUE(send_and_check_show_ime());
+
+  // Send the same message. Expect IME (no change).
+  EXPECT_TRUE(send_and_check_show_ime());
+
+  // Reset |TextInputState.show_ime_if_needed|. Expect no IME.
+  sender.SetShowImeIfNeeded(false);
+  EXPECT_FALSE(send_and_check_show_ime());
+
+  // Setting an irrelevant field. Expect no IME.
+  sender.SetMode(ui::TEXT_INPUT_MODE_LATIN);
+  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  // USE_AURA