Redesigns the text input focus handling.

ui/views/controls/webview/webview_unittest.cc

Index: ui/views/controls/webview/webview_unittest.cc
diff --git a/ui/views/controls/webview/webview_unittest.cc b/ui/views/controls/webview/webview_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d82975ec1251aa29597939204526510dd89300c1
--- /dev/null
+++ b/ui/views/controls/webview/webview_unittest.cc
@@ -0,0 +1,114 @@
+// Copyright 2014 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 "ui/views/controls/webview/webview.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/aura/test/aura_test_helper.h"
+#include "ui/base/ime/text_input_focus_manager.h"
+#include "ui/base/ui_base_switches_util.h"
+#include "ui/compositor/test/context_factories_for_test.h"
+#include "ui/views/test/webview_test_helper.h"
+#include "ui/views/widget/widget.h"
+
+namespace {
+
+class WebViewTest : public testing::Test {
+ public:
+  WebViewTest()
+      : widget_(NULL), webview_(NULL),

[msw, 2014/04/18 19:57:57]

nit: one initializer per line

[Yuki, 2014/04/22 09:09:02]

After applying your comment, we don't need webview_test_helper_ here.  And now
everything can be on one line.

+        webview_test_helper_(new views::WebViewTestHelper()) {}
+
+  virtual void SetUp() {
+    testing::Test::SetUp();
+
+    ui::InitializeContextFactoryForTests(false);
+
+    aura_test_helper_.reset(
+        new aura::test::AuraTestHelper(base::MessageLoopForUI::current()));
+    aura_test_helper_->SetUp();
+
+    InitWebView();
+  }
+
+  virtual void TearDown() {
+    if (widget_)
+      widget_->Close();
+
+    aura_test_helper_->TearDown();
+
+    ui::TerminateContextFactoryForTests();
+
+    testing::Test::TearDown();
+  }
+
+ protected:
+  views::WebView* webview() { return webview_; }
+  content::BrowserContext* browser_context() { return browser_context_.get(); }
+
+ private:
+  void InitWebView() {
+    widget_ = new views::Widget();
+    views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
+    params.context = aura_test_helper_->root_window();
+    params.bounds = gfx::Rect(100, 100, 100, 100);
+    widget_->Init(params);
+
+    views::View* container = new views::View();

[msw, 2014/04/18 19:57:57]

nit: is container needed, can you just widget_->SetContentsView(webview_);?

[Yuki, 2014/04/22 09:09:02]

Done.

+    widget_->SetContentsView(container);
+
+    browser_context_.reset(new content::TestBrowserContext());
+    webview_ = new views::WebView(browser_context_.get());
+    container->AddChildView(webview_);
+
+    widget_->Activate();
+    widget_->Show();
+  }
+
+  views::Widget* widget_;
+  views::WebView* webview_;
+  scoped_ptr<aura::test::AuraTestHelper> aura_test_helper_;
+  scoped_ptr<content::TestBrowserContext> browser_context_;

[msw, 2014/04/18 19:57:57]

nit: drop scoped_ptr for content::TestBrowserContext browser_context_;

[Yuki, 2014/04/22 09:09:02]

Done.

+  scoped_ptr<views::WebViewTestHelper> webview_test_helper_;

[msw, 2014/04/18 19:57:57]

nit: drop scoped_ptr for views::WebViewTestHelper webview_test_helper_;

[Yuki, 2014/04/22 09:09:02]

Done.

+  content::TestBrowserThreadBundle browser_thread_bundle_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebViewTest);
+};
+
+TEST_F(WebViewTest, TextInputClientIsUpToDate) {
+  switches::ScopedTextInputFocusManagerForTesting
+      text_input_focus_manager_for_testing;
+
+  webview()->SetEmbedFullscreenWidgetMode(true);
+
+  ui::TextInputFocusManager* const text_input_focus_manager =

[msw, 2014/04/18 19:57:57]

nit: this usage of const is uncommon, is it intended?

[Yuki, 2014/04/22 09:09:02]

Removed.  It was intentional, but I don't care much about this.

+      ui::TextInputFocusManager::GetInstance();
+  const ui::TextInputClient* const null_text_input_client = NULL;

[msw, 2014/04/18 19:57:57]

nit: this usage of const is uncommon, is it intended?

[Yuki, 2014/04/22 09:09:02]

Done.

+
+  EXPECT_EQ(null_text_input_client, webview()->GetTextInputClient());
+  EXPECT_EQ(text_input_focus_manager->GetFocusedTextInputClient(),
+            webview()->GetTextInputClient());
+
+  // Case 1: Creates a new WebContents.
+  webview()->GetWebContents();  // Creates a new WebContents.

[msw, 2014/04/18 19:57:57]

nit: remove comment, it's redundant with the one above.

[Yuki, 2014/04/22 09:09:02]

Done.

+  webview()->RequestFocus();
+  ui::TextInputClient* client1 = webview()->GetTextInputClient();
+  EXPECT_NE(null_text_input_client, client1);
+  EXPECT_EQ(text_input_focus_manager->GetFocusedTextInputClient(), client1);
+
+  // Case 2: Replaces a WebContents by SetWebContents().
+  content::WebContents::CreateParams params(browser_context());
+  content::WebContents* web_contents = content::WebContents::Create(params);

[msw, 2014/04/18 19:57:57]

Use a ScopedPtr, WebView doesn't take ownership below.

[Yuki, 2014/04/22 09:09:02]

Done.

+  webview()->SetWebContents(web_contents);
+  ui::TextInputClient* client2 = webview()->GetTextInputClient();
+  EXPECT_NE(null_text_input_client, client2);
+  EXPECT_EQ(text_input_focus_manager->GetFocusedTextInputClient(), client2);
+  EXPECT_NE(client1, client2);
+}
+
+}  // namespace