Refactoring for InputMethodEngine and InputMethodEventRouter.

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

Index: chrome/browser/extensions/api/input_ime/input_ime_api.cc
diff --git a/chrome/browser/extensions/api/input_ime/input_ime_api.cc b/chrome/browser/extensions/api/input_ime/input_ime_api.cc
index b1d15c6cec63e6c77ccdf3b66a7cb0bf0111abae..5bc0ed6638e83f2461f1cf96b028dd814a63d590 100644
--- a/chrome/browser/extensions/api/input_ime/input_ime_api.cc
+++ b/chrome/browser/extensions/api/input_ime/input_ime_api.cc
@@ -14,6 +14,9 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/extensions/api/input_ime.h"
 #include "chrome/common/extensions/api/input_ime/input_components_handler.h"
+#include "chromeos/ime/component_extension_ime_manager.h"
+#include "chromeos/ime/extension_ime_util.h"
+#include "chromeos/ime/input_method_manager.h"

[Shu Chen, 2014/08/04 15:59:31]

Nona, I guess the original design was to avoid dependency to InputMethodManager,
but I don't know why.
This cl would introduce dependency to InputMethodManager. With reasonable
justification, I can move the dependency to InputMethodEngine.

[Seigo Nonaka, 2014/08/04 23:29:47]

Probably, it is historical reasons. It used to be planed that this API would be
used also in other platforms.
However, that plan is no longer active and already chrome os specific dependency
was introduced.
So I think we no longer consider the dependency to chrome os specific headers.

On 2014/08/04 15:59:31, Shu Chen wrote:

[Shu Chen, 2014/08/05 01:23:09]

Done.

 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_function_registry.h"
 #include "extensions/browser/extension_registry.h"
@@ -75,10 +78,10 @@ void SetMenuItemToMenu(const input_ime::MenuItem& input,
   out->enabled = input.enabled ? *input.enabled : true;
 }
 
-static void DispatchEventToExtension(Profile* profile,
-                                     const std::string& extension_id,
+static void DispatchEventToExtension(const std::string& extension_id,
                                      const std::string& event_name,
                                      scoped_ptr<base::ListValue> args) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
   scoped_ptr<extensions::Event> event(new extensions::Event(
       event_name, args.Pass()));
   event->restrict_to_browser_context = profile;
@@ -99,46 +102,36 @@ void CallbackKeyEventHandle(chromeos::input_method::KeyEventHandle* key_data,
 namespace chromeos {
 class ImeObserver : public InputMethodEngineInterface::Observer {
  public:
-  ImeObserver(Profile* profile, const std::string& extension_id)
-      : profile_(profile), extension_id_(extension_id), has_background_(false) {
-    extensions::ExtensionSystem* extension_system =
-        extensions::ExtensionSystem::Get(profile_);
-    ExtensionService* extension_service = extension_system->extension_service();
-    const extensions::Extension* extension =
-        extension_service->GetExtensionById(extension_id, false);
-    DCHECK(extension);
-    extensions::BackgroundInfo* info = static_cast<extensions::BackgroundInfo*>(
-        extension->GetManifestData("background"));
-    if (info)
-      has_background_ = info->has_background_page();
+  explicit ImeObserver(const std::string& extension_id)
+      : extension_id_(extension_id) {
   }
 
   virtual ~ImeObserver() {}
 
   virtual void OnActivate(const std::string& engine_id) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     scoped_ptr<base::ListValue> args(input_ime::OnActivate::Create(engine_id));
 
-    DispatchEventToExtension(profile_, extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnActivate::kEventName, args.Pass());
   }
 
   virtual void OnDeactivated(const std::string& engine_id) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     scoped_ptr<base::ListValue> args(
         input_ime::OnDeactivated::Create(engine_id));
 
-    DispatchEventToExtension(profile_, extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnDeactivated::kEventName, args.Pass());
   }
 
   virtual void OnFocus(
       const InputMethodEngineInterface::InputContext& context) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     input_ime::InputContext context_value;
@@ -165,23 +158,23 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
         val->SetStringWithoutPathExpansion("screen", screen_type);
     }
 
-    DispatchEventToExtension(profile_, extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnFocus::kEventName, args.Pass());
   }
 
   virtual void OnBlur(int context_id) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     scoped_ptr<base::ListValue> args(input_ime::OnBlur::Create(context_id));
 
-    DispatchEventToExtension(profile_, extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnBlur::kEventName, args.Pass());
   }
 
   virtual void OnInputContextUpdate(
       const InputMethodEngineInterface::InputContext& context) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     input_ime::InputContext context_value;
@@ -191,8 +184,7 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
     scoped_ptr<base::ListValue> args(
         input_ime::OnInputContextUpdate::Create(context_value));
 
-    DispatchEventToExtension(profile_,
-                             extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnInputContextUpdate::kEventName,
                              args.Pass());
   }
@@ -201,7 +193,7 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
       const std::string& engine_id,
       const InputMethodEngineInterface::KeyboardEvent& event,
       chromeos::input_method::KeyEventHandle* key_data) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     // If there is no listener for the event, no need to dispatch the event to
@@ -234,7 +226,7 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
     scoped_ptr<base::ListValue> args(
         input_ime::OnKeyEvent::Create(engine_id, key_data_value));
 
-    DispatchEventToExtension(profile_, extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnKeyEvent::kEventName, args.Pass());
   }
 
@@ -242,7 +234,7 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
       const std::string& engine_id,
       int candidate_id,
       InputMethodEngineInterface::MouseButtonEvent button) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     input_ime::OnCandidateClicked::Button button_enum =
@@ -268,22 +260,20 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
                                               candidate_id,
                                               button_enum));
 
-    DispatchEventToExtension(profile_,
-                             extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnCandidateClicked::kEventName,
                              args.Pass());
   }
 
   virtual void OnMenuItemActivated(const std::string& engine_id,
                                    const std::string& menu_id) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     scoped_ptr<base::ListValue> args(
         input_ime::OnMenuItemActivated::Create(engine_id, menu_id));
 
-    DispatchEventToExtension(profile_,
-                             extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnMenuItemActivated::kEventName,
                              args.Pass());
   }
@@ -292,7 +282,7 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
                                         const std::string& text,
                                         int cursor_pos,
                                         int anchor_pos) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     input_ime::OnSurroundingTextChanged::SurroundingInfo info;
@@ -302,20 +292,18 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
     scoped_ptr<base::ListValue> args(
         input_ime::OnSurroundingTextChanged::Create(engine_id, info));
 
-    DispatchEventToExtension(profile_,
-                             extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnSurroundingTextChanged::kEventName,
                              args.Pass());
   }
 
   virtual void OnReset(const std::string& engine_id) OVERRIDE {
-    if (profile_ == NULL || extension_id_.empty())
+    if (extension_id_.empty())
       return;
 
     scoped_ptr<base::ListValue> args(input_ime::OnReset::Create(engine_id));
 
-    DispatchEventToExtension(profile_,
-                             extension_id_,
+    DispatchEventToExtension(extension_id_,
                              input_ime::OnReset::kEventName,
                              args.Pass());
   }
@@ -327,14 +315,12 @@ class ImeObserver : public InputMethodEngineInterface::Observer {
     // Need to check the background page first since the
     // ExtensionHasEventListner returns true if the extension does not have a
     // background page. See crbug.com/394682.
-    return has_background_ && extensions::EventRouter::Get(profile_)
+    return extensions::EventRouter::Get(ProfileManager::GetActiveUserProfile())
         ->ExtensionHasEventListener(extension_id_,
                                     input_ime::OnKeyEvent::kEventName);
   }
 
-  Profile* profile_;
   std::string extension_id_;
-  bool has_background_;
 
   DISALLOW_COPY_AND_ASSIGN(ImeObserver);
 };
@@ -348,40 +334,56 @@ InputImeEventRouter::GetInstance() {
   return Singleton<InputImeEventRouter>::get();
 }
 
-bool InputImeEventRouter::RegisterIme(
+bool InputImeEventRouter::RegisterImeExtension(
     const std::string& extension_id,
-    const extensions::InputComponentInfo& component) {
+    const std::vector<extensions::InputComponentInfo>& input_components) {
 #if defined(USE_X11)
-  VLOG(1) << "RegisterIme: " << extension_id << " id: " << component.id;
+  VLOG(1) << "RegisterImeExtension: " << extension_id;
 
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-  // Avoid potential mem leaks due to duplicated component IDs.
-  if (!profile_engine_map_[profile][extension_id][component.id]) {
-    std::vector<std::string> layouts;
-    layouts.assign(component.layouts.begin(), component.layouts.end());
-
-    std::vector<std::string> languages;
-    languages.assign(component.languages.begin(), component.languages.end());
-
-    // Ideally Observer should be per (extension_id + Profile), and multiple
-    // InputMethodEngine can share one Observer. But it would become tricky
-    // to maintain an internal map for observers which does nearly nothing
-    // but just make sure they can properly deleted.
-    // Making Obesrver per InputMethodEngine can make things cleaner.
-    scoped_ptr<chromeos::InputMethodEngineInterface::Observer> observer(
-        new chromeos::ImeObserver(profile, extension_id));
-    chromeos::InputMethodEngine* engine = new chromeos::InputMethodEngine();
-    engine->Initialize(observer.Pass(),
-                       component.name.c_str(),
-                       extension_id.c_str(),
-                       component.id.c_str(),
-                       languages,
-                       layouts,
-                       component.options_page_url,
-                       component.input_view_url);
-    profile_engine_map_[profile][extension_id][component.id] = engine;
+  if (engine_map_[extension_id])
+    return false;
+
+  chromeos::input_method::InputMethodManager* manager =
+      chromeos::input_method::InputMethodManager::Get();
+  chromeos::ComponentExtensionIMEManager* comp_ext_ime_manager =
+      manager->GetComponentExtensionIMEManager();
+
+  chromeos::input_method::InputMethodDescriptors descriptors;
+  // Only creates descriptors for 3rd party IME extension, because the
+  // descriptors for component IME extensions are managed by InputMethodUtil.
+  if (!comp_ext_ime_manager->IsWhitelistedExtension(extension_id)) {
+    for (std::vector<extensions::InputComponentInfo>::const_iterator it =
+         input_components.begin(); it != input_components.end(); ++it) {
+      const extensions::InputComponentInfo& component = *it;
+      DCHECK(component.type == extensions::INPUT_COMPONENT_TYPE_IME);
+
+      std::vector<std::string> layouts;
+      layouts.assign(component.layouts.begin(), component.layouts.end());
+      std::vector<std::string> languages;
+      languages.assign(component.languages.begin(), component.languages.end());
+
+      std::string input_method_id =
+          chromeos::extension_ime_util::GetInputMethodID(
+              extension_id, component.id);
+      descriptors.push_back(chromeos::input_method::InputMethodDescriptor(
+          input_method_id,
+          component.name,
+          std::string(), // TODO(uekawa): Set short name.
+          layouts,
+          languages,
+          false,  // 3rd party IMEs are always not for login.
+          component.options_page_url,
+          component.input_view_url));
+    }
   }
 
+  scoped_ptr<chromeos::InputMethodEngineInterface::Observer> observer(
+      new chromeos::ImeObserver(extension_id));
+  chromeos::InputMethodEngine* engine = new chromeos::InputMethodEngine();
+  engine->Initialize(observer.Pass(), extension_id.c_str());
+  engine_map_[extension_id] = engine;
+  manager->AddInputMethodExtension(extension_id, descriptors, engine);
+
   return true;
 #else
   // TODO(spang): IME support under ozone.
@@ -391,58 +393,28 @@ bool InputImeEventRouter::RegisterIme(
 }
 
 void InputImeEventRouter::UnregisterAllImes(const std::string& extension_id) {
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-  ProfileEngineMap::iterator extension_map =
-      profile_engine_map_.find(profile);
-  if (extension_map == profile_engine_map_.end())
-    return;
-  ExtensionMap::iterator engine_map = extension_map->second.find(extension_id);
-  if (engine_map == extension_map->second.end())
-    return;
-  STLDeleteContainerPairSecondPointers(engine_map->second.begin(),
-                                       engine_map->second.end());
-  extension_map->second.erase(extension_id);
-  profile_engine_map_.erase(profile);
+  if (engine_map_.find(extension_id) != engine_map_.end()) {
+    chromeos::input_method::InputMethodManager::Get()
+        ->RemoveInputMethodExtension(extension_id);
+    delete engine_map_[extension_id];
+    engine_map_.erase(extension_id);
+  }
 }
 
 InputMethodEngineInterface* InputImeEventRouter::GetEngine(
     const std::string& extension_id, const std::string& engine_id) {
-  // IME can only work on active user profile.
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-
-  ProfileEngineMap::const_iterator extension_map =
-      profile_engine_map_.find(profile);
-  if (extension_map == profile_engine_map_.end())
-    return NULL;
-  ExtensionMap::const_iterator engine_map =
-      extension_map->second.find(extension_id);
-  if (engine_map == extension_map->second.end())
-    return NULL;
-  EngineMap::const_iterator engine = engine_map->second.find(engine_id);
-  if (engine == engine_map->second.end())
-    return NULL;
-  return engine->second;
+  if (engine_map_.find(extension_id) != engine_map_.end()) {

[Seigo Nonaka, 2014/08/04 23:29:48]

nit: it is totally up to you but I prefer

if (engine_map_.find(extension_id) != engine_map_.end() &&
  engine_map_[extension_id]->GetActiveEngineId() == engine_id)
  return engine_map_[extension_id];
else
  return NULL;

or

if (engine_map_.find(extension_id) == engine_map_.end())
  return NULL;

if (engine_map_[extension_id]->GetActiveEngineId() == engine _id)
  return engine_map_[extension_id];

return NULL;

[Shu Chen, 2014/08/05 01:23:09]

Done.

+    if (engine_map_[extension_id]->GetActiveEngineId() == engine_id)
+      return engine_map_[extension_id];
+  }
+  return NULL;
 }
 
 InputMethodEngineInterface* InputImeEventRouter::GetActiveEngine(
     const std::string& extension_id) {
-  // IME can only work on active user profile.
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-
-  ProfileEngineMap::const_iterator extension_map =
-      profile_engine_map_.find(profile);
-  if (extension_map == profile_engine_map_.end())
-    return NULL;
-  ExtensionMap::const_iterator engine_map =
-      extension_map->second.find(extension_id);
-  if (engine_map == extension_map->second.end())
-    return NULL;
-
-  for (EngineMap::const_iterator i = engine_map->second.begin();
-       i != engine_map->second.end();
-       ++i) {
-    if (i->second->IsActive())
-      return i->second;
+  if (engine_map_.find(extension_id) != engine_map_.end()) {

[Seigo Nonaka, 2014/08/04 23:29:47]

same as above.

[Shu Chen, 2014/08/05 01:23:09]

Done.

+    if (engine_map_[extension_id]->IsActive())
+      return engine_map_[extension_id];
   }
   return NULL;
 }
@@ -827,7 +799,6 @@ InputImeAPI::InputImeAPI(content::BrowserContext* context)
   extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_));
 
   EventRouter* event_router = EventRouter::Get(browser_context_);
-  event_router->RegisterObserver(this, input_ime::OnActivate::kEventName);
   event_router->RegisterObserver(this, input_ime::OnFocus::kEventName);
 }
 
@@ -847,23 +818,9 @@ void InputImeAPI::OnExtensionLoaded(content::BrowserContext* browser_context,
                                     const Extension* extension) {
   const std::vector<InputComponentInfo>* input_components =
       extensions::InputComponents::GetInputComponents(extension);
-  if (!input_components)
-    return;
-  for (std::vector<extensions::InputComponentInfo>::const_iterator component =
-           input_components->begin();
-       component != input_components->end();
-       ++component) {
-    if (component->type == extensions::INPUT_COMPONENT_TYPE_IME) {
-      // Don't pass profile_ to register ime, instead always use
-      // GetActiveUserProfile. It is because:
-      // The original profile for login screen is called signin profile.
-      // And the active profile is the incognito profile based on signin
-      // profile. So if |profile_| is signin profile, we need to make sure
-      // the router/observer runs under its incognito profile, because the
-      // component extensions were installed under its incognito profile.
-      input_ime_event_router()->RegisterIme(extension->id(), *component);
-    }
-  }
+  if (input_components)
+    input_ime_event_router()->RegisterImeExtension(
+        extension->id(), *input_components);
 }
 
 void InputImeAPI::OnExtensionUnloaded(content::BrowserContext* browser_context,
@@ -880,8 +837,9 @@ void InputImeAPI::OnExtensionUnloaded(content::BrowserContext* browser_context,
 void InputImeAPI::OnListenerAdded(const EventListenerInfo& details) {
   InputMethodEngineInterface* engine =
       input_ime_event_router()->GetActiveEngine(details.extension_id);
+  // Notifies the IME extension for IME ready with onActivate/onFocus events.
   if (engine)
-    engine->NotifyImeReady();
+    engine->Enable(engine->GetActiveEngineId());
 }
 
 InputImeEventRouter* InputImeAPI::input_ime_event_router() {