Remove {Set, Get}OverrideContentUrl in keyboard_content_util.

Before this change, URL of custom input view of IMEs, which is loaded
by ChromeKeyboardUI when an user invokes Virtual Keyboard, is stored
in global variables in keyboard_content_util and everyone can override
it accidentally.
This change moves that state into InputMethodManagerImpl and adds some
methods to interact with that state.

Bug: 819018
Test: Related unit tests pass and manual testing with ime menu tray.
Change-Id: Ic6f8e0e108c3706ad1ff2ce90aeeb1388704ff36
Reviewed-on: https://chromium-review.googlesource.com/953023
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543864}

chrome/browser/chromeos/input_method/input_method_engine.cc

@@ -31,7 +31,6 @@
 #include "ui/events/event_utils.h"
 #include "ui/events/keycodes/dom/dom_code.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
 #include "ui/keyboard/keyboard_controller.h"
 
 using input_method::InputMethodEngineBase;
@@ -225,10 +224,9 @@ void InputMethodEngine::HideInputView() {
 }
 
 void InputMethodEngine::EnableInputView() {
-  keyboard::SetOverrideContentUrl(input_method::InputMethodManager::Get()
-                                      ->GetActiveIMEState()
-                                      ->GetCurrentInputMethod()
-                                      .input_view_url());
+  input_method::InputMethodManager::Get()
+      ->GetActiveIMEState()
+      ->EnableInputView();
   keyboard::KeyboardController* keyboard_controller =
       keyboard::KeyboardController::GetInstance();
   if (keyboard_controller)

chrome/browser/chromeos/input_method/input_method_manager_impl.cc

@@ -48,7 +48,7 @@
 #include "ui/base/ime/ime_bridge.h"
 #include "ui/chromeos/ime/input_method_menu_item.h"
 #include "ui/chromeos/ime/input_method_menu_manager.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
+#include "ui/keyboard/content/keyboard_constants.h"
 #include "ui/keyboard/keyboard_controller.h"
 
 namespace chromeos {
@@ -129,6 +129,7 @@ void InputMethodManagerImpl::StateImpl::InitFrom(const StateImpl& other) {
   menu_activated = other.menu_activated;
   allowed_keyboard_layout_input_method_ids =
       other.allowed_keyboard_layout_input_method_ids;
+  input_view_url = other.input_view_url;
 }
 
 bool InputMethodManagerImpl::StateImpl::IsActive() const {
@@ -166,6 +167,7 @@ std::string InputMethodManagerImpl::StateImpl::Dump() const {
     os << " '" << it->first << "' => '" << it->second.id() << "',\n";
   }
   os << "pending_input_method_id: '" << pending_input_method_id << "'\n";
+  os << "input_view_url: '" << input_view_url << "'\n";
 
   return os.str();
 }
@@ -819,6 +821,18 @@ bool InputMethodManagerImpl::StateImpl::InputMethodIsActivated(
   return base::ContainsValue(active_input_method_ids, input_method_id);
 }
 
+void InputMethodManagerImpl::StateImpl::EnableInputView() {
+  input_view_url = current_input_method.input_view_url();
+}
+
+void InputMethodManagerImpl::StateImpl::DisableInputView() {
+  input_view_url = GURL();
+}
+
+const GURL& InputMethodManagerImpl::StateImpl::GetInputViewUrl() const {
+  return input_view_url;
+}
+
 // ------------------------ InputMethodManagerImpl
 bool InputMethodManagerImpl::IsLoginKeyboard(
     const std::string& layout) const {
@@ -1060,7 +1074,7 @@ void InputMethodManagerImpl::ChangeInputMethodInternal(
   } else {
     // If no engine to enable, cancel the virtual keyboard url override so that
     // it can use the fallback system virtual keyboard UI.
-    keyboard::SetOverrideContentUrl(GURL());
+    state_->DisableInputView();
     keyboard::KeyboardController* keyboard_controller =
         keyboard::KeyboardController::GetInstance();
     if (keyboard_controller)
@@ -1247,13 +1261,7 @@ void InputMethodManagerImpl::MaybeNotifyImeMenuActivationChanged() {
 }
 
 void InputMethodManagerImpl::OverrideKeyboardUrlRef(const std::string& keyset) {
-  GURL input_view_url;
-  if (GetActiveIMEState()) {
-    input_view_url =
-        GetActiveIMEState()->GetCurrentInputMethod().input_view_url();
-  }
-  GURL url = input_view_url.is_empty() ? keyboard::GetOverrideContentUrl()
-                                       : input_view_url;
+  GURL url = state_->GetInputViewUrl();
 
   // If fails to find ref or tag "id" in the ref, it means the current IME is
   // not system IME, and we don't support show emoji, handwriting or voice
@@ -1269,7 +1277,7 @@ void InputMethodManagerImpl::OverrideKeyboardUrlRef(const std::string& keyset) {
   if (keyset.empty()) {
     // Resets the url as the input method default url and notify the hash
     // changed to VK.
-    keyboard::SetOverrideContentUrl(input_view_url);
+    state_->input_view_url = state_->current_input_method.input_view_url();
     keyboard::KeyboardController* keyboard_controller =
         keyboard::KeyboardController::GetInstance();
     if (keyboard_controller)
@@ -1291,7 +1299,7 @@ void InputMethodManagerImpl::OverrideKeyboardUrlRef(const std::string& keyset) {
 
   GURL::Replacements replacements;
   replacements.SetRefStr(overridden_ref);
-  keyboard::SetOverrideContentUrl(url.ReplaceComponents(replacements));
+  state_->input_view_url = url.ReplaceComponents(replacements);
 
   keyboard::KeyboardController* keyboard_controller =
       keyboard::KeyboardController::GetInstance();

chrome/browser/chromeos/input_method/input_method_manager_impl.h

@@ -111,6 +111,9 @@ class InputMethodManagerImpl : public InputMethodManager,
     bool SetAllowedInputMethods(
         const std::vector<std::string>& new_allowed_input_method_ids) override;
     const std::vector<std::string>& GetAllowedInputMethods() override;
+    void EnableInputView() override;
+    void DisableInputView() override;
+    const GURL& GetInputViewUrl() const override;
 
     // ------------------------- Data members.
     Profile* const profile;
@@ -140,6 +143,10 @@ class InputMethodManagerImpl : public InputMethodManager,
     // True if the opt-in IME menu is activated.
     bool menu_activated;
 
+    // The URL of the input view of the active ime with parameters (e.g. layout,
+    // keyset).
+    GURL input_view_url;
+
    protected:
     friend base::RefCounted<chromeos::input_method::InputMethodManager::State>;
     ~StateImpl() override;

chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc

@@ -36,7 +36,6 @@
 #include "ui/base/ime/chromeos/mock_ime_engine_handler.h"
 #include "ui/base/ime/ime_bridge.h"
 #include "ui/base/ime/input_method_initializer.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
 
 namespace chromeos {
 
@@ -162,6 +161,11 @@ class InputMethodManagerImplTest :  public BrowserWithTestWindowTest {
     manager_.reset();
   }
 
+  scoped_refptr<InputMethodManagerImpl::StateImpl> GetActiveIMEState() {
+    return scoped_refptr<InputMethodManagerImpl::StateImpl>(
+        manager_->state_.get());
+  }
+
  protected:
   // Helper function to initialize component extension stuff for testing.
   void InitComponentExtension() {
@@ -1264,12 +1268,13 @@ TEST_F(InputMethodManagerImplTest, MigrateInputMethodTest) {
 }
 
 TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
+  InitComponentExtension();
   const GURL inputview_url(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty&language=en-US&passwordLayout=us."
       "compact.qwerty&name=keyboard_us");
-  keyboard::SetOverrideContentUrl(inputview_url);
-  EXPECT_EQ(inputview_url, keyboard::GetOverrideContentUrl());
+  GetActiveIMEState()->input_view_url = inputview_url;
+  EXPECT_EQ(inputview_url, GetActiveIMEState()->GetInputViewUrl());
 
   // Override the keyboard url ref with 'emoji'.
   const GURL overridden_url_emoji(
@@ -1277,35 +1282,36 @@ TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
       "inputview.html#id=us.compact.qwerty.emoji&language=en-US&passwordLayout="
       "us.compact.qwerty&name=keyboard_us");
   manager_->OverrideKeyboardUrlRef("emoji");
-  EXPECT_EQ(overridden_url_emoji, keyboard::GetOverrideContentUrl());
+  EXPECT_EQ(overridden_url_emoji, GetActiveIMEState()->GetInputViewUrl());
 
   // Override the keyboard url ref with 'hwt'.
-  keyboard::SetOverrideContentUrl(inputview_url);
+  GetActiveIMEState()->input_view_url = inputview_url;
   const GURL overridden_url_hwt(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty.hwt&language=en-US&passwordLayout="
       "us.compact.qwerty&name=keyboard_us");
   manager_->OverrideKeyboardUrlRef("hwt");
-  EXPECT_EQ(overridden_url_hwt, keyboard::GetOverrideContentUrl());
+  EXPECT_EQ(overridden_url_hwt, GetActiveIMEState()->GetInputViewUrl());
 
   // Override the keyboard url ref with 'voice'.
-  keyboard::SetOverrideContentUrl(inputview_url);
+  GetActiveIMEState()->input_view_url = inputview_url;
   const GURL overridden_url_voice(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty.voice&language=en-US"
       "&passwordLayout=us.compact.qwerty&name=keyboard_us");
   manager_->OverrideKeyboardUrlRef("voice");
-  EXPECT_EQ(overridden_url_voice, keyboard::GetOverrideContentUrl());
+  EXPECT_EQ(overridden_url_voice, GetActiveIMEState()->GetInputViewUrl());
 }
 
 TEST_F(InputMethodManagerImplTest, OverrideDefaultKeyboardUrlRef) {
+  InitComponentExtension();
   const GURL default_url("chrome://inputview.html");
-  keyboard::SetOverrideContentUrl(default_url);
+  GetActiveIMEState()->input_view_url = default_url;
 
-  EXPECT_EQ(default_url, keyboard::GetOverrideContentUrl());
+  EXPECT_EQ(default_url, GetActiveIMEState()->GetInputViewUrl());
 
   manager_->OverrideKeyboardUrlRef("emoji");
-  EXPECT_EQ(default_url, keyboard::GetOverrideContentUrl());
+  EXPECT_EQ(default_url, GetActiveIMEState()->GetInputViewUrl());
 }
 
 TEST_F(InputMethodManagerImplTest, AllowedKeyboardLayoutsValid) {

chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc

@@ -24,7 +24,6 @@
 #include "ui/base/ime/chromeos/extension_ime_util.h"
 #include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/base/ime/ime_engine_handler_interface.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
 #include "ui/keyboard/keyboard_controller.h"
 
 namespace input_ime = extensions::api::input_ime;
@@ -680,7 +679,7 @@ void InputImeAPI::OnExtensionUnloaded(content::BrowserContext* browser_context,
       // Empties the content url and reload the controller to unload the
       // current page.
       // TODO(wuyingbing): Should add a new method to unload the document.
-      keyboard::SetOverrideContentUrl(GURL());
+      manager->GetActiveIMEState()->DisableInputView();
       keyboard_controller->Reload();
     }
     event_router->SetUnloadedExtensionId(extension->id());

chrome/browser/ui/ash/chrome_keyboard_ui.cc

@@ -39,12 +39,12 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
+#include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/keyboard/content/keyboard_constants.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/keyboard_controller_observer.h"
 #include "ui/keyboard/keyboard_switches.h"
@@ -56,6 +56,8 @@ namespace virtual_keyboard_private = extensions::api::virtual_keyboard_private;
 
 namespace {
 
+base::Optional<GURL> g_override_virtual_keyboard_url;
+
 class WindowBoundsChangeObserver : public aura::WindowObserver {
  public:
   explicit WindowBoundsChangeObserver(ChromeKeyboardUI* ui) : ui_(ui) {}
@@ -248,6 +250,11 @@ class AshKeyboardControllerObserver
 
 }  // namespace
 
+void ChromeKeyboardUI::TestApi::SetOverrideVirtualKeyboardUrl(
+    base::Optional<GURL> url) {
+  g_override_virtual_keyboard_url = url;
+}
+
 ChromeKeyboardUI::ChromeKeyboardUI(content::BrowserContext* context)
     : browser_context_(context),
       default_url_(keyboard::kKeyboardURL),
@@ -436,12 +443,18 @@ void ChromeKeyboardUI::LoadContents(const GURL& url) {
 }
 
 const GURL& ChromeKeyboardUI::GetVirtualKeyboardUrl() {
-  if (keyboard::IsInputViewEnabled()) {
-    const GURL& override_url = keyboard::GetOverrideContentUrl();
-    return override_url.is_valid() ? override_url : default_url_;
-  } else {
+  if (g_override_virtual_keyboard_url.has_value())
+    return g_override_virtual_keyboard_url.value();
+
+  chromeos::input_method::InputMethodManager* ime_manager =
+      chromeos::input_method::InputMethodManager::Get();
+  if (!keyboard::IsInputViewEnabled() || !ime_manager ||
+      !ime_manager->GetActiveIMEState())
     return default_url_;
-  }
+
+  const GURL& input_view_url =
+      ime_manager->GetActiveIMEState()->GetInputViewUrl();
+  return input_view_url.is_valid() ? input_view_url : default_url_;
 }
 
 bool ChromeKeyboardUI::ShouldEnableInsets(aura::Window* window) {

chrome/browser/ui/ash/chrome_keyboard_ui.h

@@ -43,16 +43,10 @@ class ChromeKeyboardUI : public keyboard::KeyboardUI,
  public:
   class TestApi {
    public:
-    explicit TestApi(ChromeKeyboardUI* ui) : ui_(ui) {}
-
-    const content::WebContents* keyboard_contents() const {
-      return ui_->keyboard_contents_.get();
-    }
+    static void SetOverrideVirtualKeyboardUrl(base::Optional<GURL> url);
 
    private:
-    ChromeKeyboardUI* ui_;
-
-    DISALLOW_COPY_AND_ASSIGN(TestApi);
+    DISALLOW_IMPLICIT_CONSTRUCTORS(TestApi);
   };
 
   explicit ChromeKeyboardUI(content::BrowserContext* context);

chrome/browser/ui/ash/keyboard_controller_browsertest.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/ash/chrome_keyboard_ui.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
@@ -19,7 +20,6 @@
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/input_method_factory.h"
 #include "ui/keyboard/content/keyboard_constants.h"
-#include "ui/keyboard/content/keyboard_content_util.h"
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/keyboard_switches.h"
 #include "ui/keyboard/keyboard_test_util.h"
@@ -39,6 +39,11 @@ class VirtualKeyboardWebContentTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUp();
   }
 
+  void TearDown() override {
+    ChromeKeyboardUI::TestApi::SetOverrideVirtualKeyboardUrl(base::nullopt);
+    InProcessBrowserTest::TearDown();
+  }
+
   // Ensure that the virtual keyboard is enabled.
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(keyboard::switches::kEnableVirtualKeyboard);
@@ -67,7 +72,7 @@ class VirtualKeyboardWebContentTest : public InProcessBrowserTest {
 
   void MockEnableIMEInDifferentExtension(const std::string& url,
                                          const gfx::Rect& init_bounds) {
-    keyboard::SetOverrideContentUrl(GURL(url));
+    ChromeKeyboardUI::TestApi::SetOverrideVirtualKeyboardUrl(GURL(url));
     keyboard::KeyboardController::GetInstance()->Reload();
     // Mock window.resizeTo that is expected to be called after navigate to a
     // new virtual keyboard.

ui/base/ime/chromeos/input_method_manager.h

@@ -231,6 +231,15 @@ class UI_BASE_IME_EXPORT InputMethodManager {
     // are allowed.
     virtual const std::vector<std::string>& GetAllowedInputMethods() = 0;
 
+    // Methods related to custom input view of the input method.
+    // Enables custom input view of the active input method.
+    virtual void EnableInputView() = 0;
+    // Disables custom input view of the active input method.
+    // The fallback system input view will be used.
+    virtual void DisableInputView() = 0;
+    // Returns the URL of the input view of the active input method.
+    virtual const GURL& GetInputViewUrl() const = 0;
+
    protected:
     friend base::RefCounted<InputMethodManager::State>;
 

ui/base/ime/chromeos/mock_input_method_manager.cc

@@ -103,6 +103,14 @@ MockInputMethodManager::State::GetAllowedInputMethods() {
   return allowed_input_method_ids_;
 }
 
+void MockInputMethodManager::State::EnableInputView() {}
+
+void MockInputMethodManager::State::DisableInputView() {}
+
+const GURL& MockInputMethodManager::State::GetInputViewUrl() const {
+  return GURL::EmptyGURL();
+}
+
 MockInputMethodManager::State::~State() {}
 
 MockInputMethodManager::MockInputMethodManager()

ui/base/ime/chromeos/mock_input_method_manager.h

@@ -57,6 +57,9 @@ class UI_BASE_IME_EXPORT MockInputMethodManager : public InputMethodManager {
     bool SetAllowedInputMethods(
         const std::vector<std::string>& new_allowed_input_method_ids) override;
     const std::vector<std::string>& GetAllowedInputMethods() override;
+    void EnableInputView() override;
+    void DisableInputView() override;
+    const GURL& GetInputViewUrl() const override;
 
     // The active input method ids cache (actually default only)
     std::vector<std::string> active_input_method_ids;

ui/keyboard/content/keyboard_content_util.cc

@@ -4,27 +4,11 @@
 
 #include "ui/keyboard/content/keyboard_content_util.h"
 
-#include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "ui/keyboard/grit/keyboard_resources.h"
 #include "ui/keyboard/grit/keyboard_resources_map.h"
-#include "url/gurl.h"
 
 namespace keyboard {
-namespace {
-
-base::LazyInstance<GURL>::DestructorAtExit g_override_content_url =
-    LAZY_INSTANCE_INITIALIZER;
-
-}  // namespace
-
-void SetOverrideContentUrl(const GURL& url) {
-  g_override_content_url.Get() = url;
-}
-
-const GURL& GetOverrideContentUrl() {
-  return g_override_content_url.Get();
-}
 
 const GritResourceMap* GetKeyboardExtensionResources(size_t* size) {
   // This looks a lot like the contents of a resource map; however it is

ui/keyboard/content/keyboard_content_util.h

@@ -7,22 +7,12 @@
 
 #include <stddef.h>
 
-#include "base/strings/string16.h"
 #include "ui/keyboard/keyboard_export.h"
 
-class GURL;
-
 struct GritResourceMap;
 
 namespace keyboard {
 
-// Sets the override content url.
-// This is used by for input view for extension IMEs.
-KEYBOARD_EXPORT void SetOverrideContentUrl(const GURL& url);
-
-// Gets the override content url.
-KEYBOARD_EXPORT const GURL& GetOverrideContentUrl();
-
 // Get the list of keyboard resources. |size| is populated with the number of
 // resources in the returned array.
 KEYBOARD_EXPORT const GritResourceMap* GetKeyboardExtensionResources(