GetActiveUserProfile() should not be called before profile is initialized.

During extension initialization we should not rely on GetActiveUserProfile().

BUG=397971
TEST=manual

Review URL: https://codereview.chromium.org/428783008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287371 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 118 insertions, 81 deletions

diff --git a/chrome/browser/chromeos/input_method/input_method_engine.cc b/chrome/browser/chromeos/input_method/input_method_engine.cc
index 47a3c5a..271f095 100644
--- a/chrome/browser/chromeos/input_method/input_method_engine.cc
+++ b/chrome/browser/chromeos/input_method/input_method_engine.cc
@@ -119,15 +119,19 @@ InputMethodEngine::InputMethodEngine()
       composition_cursor_(0),
       candidate_window_(new ui::CandidateWindow()),
       window_visible_(false),
-      sent_key_event_(NULL) {}
+      sent_key_event_(NULL),
+      profile_(NULL) {
+}
 
 InputMethodEngine::~InputMethodEngine() {
   if (start_time_.ToInternalValue())
     RecordHistogram("WorkingTime", (end_time_ - start_time_).InSeconds());
-  input_method::InputMethodManager::Get()->RemoveInputMethodExtension(imm_id_);
+  input_method::InputMethodManager::Get()->RemoveInputMethodExtension(profile_,
+                                                                      imm_id_);
 }
 
 void InputMethodEngine::Initialize(
+    Profile* profile,
     scoped_ptr<InputMethodEngineInterface::Observer> observer,
     const char* engine_name,
     const char* extension_id,
@@ -138,6 +142,8 @@ void InputMethodEngine::Initialize(
     const GURL& input_view) {
   DCHECK(observer) << "Observer must not be null.";
 
+  profile_ = profile;
+
   // TODO(komatsu): It is probably better to set observer out of Initialize.
   observer_ = observer.Pass();
   engine_id_ = engine_id;
@@ -168,7 +174,7 @@ void InputMethodEngine::Initialize(
 
   // TODO(komatsu): It is probably better to call AddInputMethodExtension
   // out of Initialize.
-  manager->AddInputMethodExtension(imm_id_, this);
+  manager->AddInputMethodExtension(profile, imm_id_, this);
 }
 
 const input_method::InputMethodDescriptor& InputMethodEngine::GetDescriptor()
diff --git a/chrome/browser/chromeos/input_method/input_method_engine.h b/chrome/browser/chromeos/input_method/input_method_engine.h
index d841a57..22a8006 100644
--- a/chrome/browser/chromeos/input_method/input_method_engine.h
+++ b/chrome/browser/chromeos/input_method/input_method_engine.h
@@ -13,6 +13,8 @@
 #include "chromeos/ime/input_method_descriptor.h"
 #include "url/gurl.h"
 
+class Profile;
+
 namespace ui {
 class CandidateWindow;
 class KeyEvent;
@@ -38,7 +40,8 @@ class InputMethodEngine : public InputMethodEngineInterface {
 
   virtual ~InputMethodEngine();
 
-  void Initialize(scoped_ptr<InputMethodEngineInterface::Observer> observer,
+  void Initialize(Profile* profile,
+                  scoped_ptr<InputMethodEngineInterface::Observer> observer,
                   const char* engine_name,
                   const char* extension_id,
                   const char* engine_id,
@@ -162,6 +165,9 @@ class InputMethodEngine : public InputMethodEngineInterface {
   // The start & end time of using this input method. This is for UMA.
   base::Time start_time_;
   base::Time end_time_;
+
+  // User profile that owns this method.
+  Profile* profile_;
 };
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/input_method/input_method_engine_unittest.cc b/chrome/browser/chromeos/input_method/input_method_engine_unittest.cc
index 8210b17..a5df161 100644
--- a/chrome/browser/chromeos/input_method/input_method_engine_unittest.cc
+++ b/chrome/browser/chromeos/input_method/input_method_engine_unittest.cc
@@ -175,7 +175,8 @@ class InputMethodEngineTest :  public testing::Test {
     engine_.reset(new InputMethodEngine());
     observer_ = new TestObserver();
     scoped_ptr<InputMethodEngineInterface::Observer> observer_ptr(observer_);
-    engine_->Initialize(observer_ptr.Pass(),
+    engine_->Initialize(NULL /* profile */,
+                        observer_ptr.Pass(),
                         "",
                         whitelisted ? kTestExtensionId : kTestExtensionId2,
                         kTestImeEngineId,
diff --git a/chrome/browser/chromeos/input_method/input_method_manager_impl.cc b/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
index 8db20e0..bc87740 100644
--- a/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
+++ b/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
@@ -433,6 +433,7 @@ void InputMethodManagerImpl::ActivateInputMethodMenuItem(
 }
 
 void InputMethodManagerImpl::AddInputMethodExtension(
+    Profile* profile,
     const std::string& id,
     InputMethodEngineInterface* engine) {
   if (state_ == STATE_TERMINATING)
@@ -440,7 +441,7 @@ void InputMethodManagerImpl::AddInputMethodExtension(
 
   DCHECK(engine);
 
-  profile_engine_map_[GetProfile()][id] = engine;
+  profile_engine_map_[profile][id] = engine;
 
   if (id == current_input_method_.id()) {
     IMEBridge::Get()->SetCurrentEngineHandler(engine);
@@ -469,7 +470,8 @@ void InputMethodManagerImpl::AddInputMethodExtension(
   }
 }
 
-void InputMethodManagerImpl::RemoveInputMethodExtension(const std::string& id) {
+void InputMethodManagerImpl::RemoveInputMethodExtension(Profile* profile,
+                                                        const std::string& id) {
   if (!extension_ime_util::IsExtensionIME(id))
     DVLOG(1) << id << " is not a valid extension input method ID.";
 
@@ -479,7 +481,7 @@ void InputMethodManagerImpl::RemoveInputMethodExtension(const std::string& id) {
     active_input_method_ids_.erase(i);
   extra_input_methods_.erase(id);
 
-  EngineMap& engine_map = profile_engine_map_[GetProfile()];
+  EngineMap& engine_map = profile_engine_map_[profile];
   if (IMEBridge::Get()->GetCurrentEngineHandler() == engine_map[id])
     IMEBridge::Get()->SetCurrentEngineHandler(NULL);
   engine_map.erase(id);
diff --git a/chrome/browser/chromeos/input_method/input_method_manager_impl.h b/chrome/browser/chromeos/input_method/input_method_manager_impl.h
index 1984776..e6b6a0f 100644
--- a/chrome/browser/chromeos/input_method/input_method_manager_impl.h
+++ b/chrome/browser/chromeos/input_method/input_method_manager_impl.h
@@ -65,9 +65,11 @@ class InputMethodManagerImpl : public InputMethodManager,
   virtual void ChangeInputMethod(const std::string& input_method_id) OVERRIDE;
   virtual void ActivateInputMethodMenuItem(const std::string& key) OVERRIDE;
   virtual void AddInputMethodExtension(
+      Profile* profile,
       const std::string& id,
       InputMethodEngineInterface* instance) OVERRIDE;
-  virtual void RemoveInputMethodExtension(const std::string& id) OVERRIDE;
+  virtual void RemoveInputMethodExtension(Profile* profile,
+                                          const std::string& id) OVERRIDE;
   virtual void GetInputMethodExtensions(
       InputMethodDescriptors* result) OVERRIDE;
   virtual void SetEnabledExtensionImes(std::vector<std::string>* ids) OVERRIDE;
@@ -101,6 +103,8 @@ class InputMethodManagerImpl : public InputMethodManager,
       scoped_ptr<ComponentExtensionIMEManagerDelegate> delegate);
 
  private:
+  friend class InputMethodManagerImplTest;
+
   // CandidateWindowController::Observer overrides:
   virtual void CandidateClicked(int index) OVERRIDE;
   virtual void CandidateWindowOpened() OVERRIDE;
@@ -153,6 +157,9 @@ class InputMethodManagerImpl : public InputMethodManager,
   void ReconfigureIMFramework();
 
   // Gets the current active user profile.
+  // Note: this method is deprecated as ActiveUserProfile might change
+  // during asynchronous operations that leads to strange crashes.
+  // Use with caution!
   Profile* GetProfile() const;
 
   scoped_ptr<InputMethodDelegate> delegate_;
diff --git a/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
index a1a4ad8..0f21840 100644
--- a/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
+++ b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
@@ -58,6 +58,59 @@ std::string ImeIdFromEngineId(const std::string& id) {
   return extension_ime_util::GetInputMethodIDByEngineID(id);
 }
 
+class TestObserver : public InputMethodManager::Observer,
+                     public ash::ime::InputMethodMenuManager::Observer {
+ public:
+  TestObserver()
+      : input_method_changed_count_(0),
+        input_method_menu_item_changed_count_(0),
+        last_show_message_(false) {
+  }
+  virtual ~TestObserver() {}
+
+  virtual void InputMethodChanged(InputMethodManager* manager,
+                                  bool show_message) OVERRIDE {
+    ++input_method_changed_count_;
+    last_show_message_ = show_message;
+  }
+  virtual void InputMethodMenuItemChanged(
+      ash::ime::InputMethodMenuManager* manager) OVERRIDE {
+    ++input_method_menu_item_changed_count_;
+  }
+
+  int input_method_changed_count_;
+  int input_method_menu_item_changed_count_;
+  bool last_show_message_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+class TestCandidateWindowObserver
+    : public InputMethodManager::CandidateWindowObserver {
+ public:
+  TestCandidateWindowObserver()
+      : candidate_window_opened_count_(0),
+        candidate_window_closed_count_(0) {
+  }
+
+  virtual ~TestCandidateWindowObserver() {}
+
+  virtual void CandidateWindowOpened(InputMethodManager* manager) OVERRIDE {
+    ++candidate_window_opened_count_;
+  }
+  virtual void CandidateWindowClosed(InputMethodManager* manager) OVERRIDE {
+    ++candidate_window_closed_count_;
+  }
+
+  int candidate_window_opened_count_;
+  int candidate_window_closed_count_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestCandidateWindowObserver);
+};
+}  // namespace
+
 class InputMethodManagerImplTest :  public BrowserWithTestWindowTest {
  public:
   InputMethodManagerImplTest()
@@ -108,6 +161,7 @@ class InputMethodManagerImplTest :  public BrowserWithTestWindowTest {
   }
 
  protected:
+  Profile* GetProfile() { return manager_->GetProfile(); }
   // Helper function to initialize component extension stuff for testing.
   void InitComponentExtension() {
     mock_delegate_ = new MockComponentExtIMEManagerDelegate();
@@ -117,13 +171,17 @@ class InputMethodManagerImplTest :  public BrowserWithTestWindowTest {
     // Note, for production, these SetEngineHandler are called when
     // IMEEngineHandlerInterface is initialized via
     // InitializeComponentextension.
-    manager_->AddInputMethodExtension(ImeIdFromEngineId(kNaclMozcUsId),
+    manager_->AddInputMethodExtension(GetProfile(),
+                                      ImeIdFromEngineId(kNaclMozcUsId),
                                       mock_engine_handler_.get());
-    manager_->AddInputMethodExtension(ImeIdFromEngineId(kNaclMozcJpId),
+    manager_->AddInputMethodExtension(GetProfile(),
+                                      ImeIdFromEngineId(kNaclMozcJpId),
                                       mock_engine_handler_.get());
-    manager_->AddInputMethodExtension(ImeIdFromEngineId(kExt2Engine1Id),
+    manager_->AddInputMethodExtension(GetProfile(),
+                                      ImeIdFromEngineId(kExt2Engine1Id),
                                       mock_engine_handler_.get());
-    manager_->AddInputMethodExtension(ImeIdFromEngineId(kExt2Engine2Id),
+    manager_->AddInputMethodExtension(GetProfile(),
+                                      ImeIdFromEngineId(kExt2Engine2Id),
                                       mock_engine_handler_.get());
     manager_->InitializeComponentExtensionForTesting(delegate.Pass());
   }
@@ -269,58 +327,6 @@ class InputMethodManagerImplTest :  public BrowserWithTestWindowTest {
   DISALLOW_COPY_AND_ASSIGN(InputMethodManagerImplTest);
 };
 
-class TestObserver : public InputMethodManager::Observer,
-                     public ash::ime::InputMethodMenuManager::Observer{
- public:
-  TestObserver()
-      : input_method_changed_count_(0),
-        input_method_menu_item_changed_count_(0),
-        last_show_message_(false) {
-  }
-  virtual ~TestObserver() {}
-
-  virtual void InputMethodChanged(InputMethodManager* manager,
-                                  bool show_message) OVERRIDE {
-    ++input_method_changed_count_;
-    last_show_message_ = show_message;
-  }
-  virtual void InputMethodMenuItemChanged(
-      ash::ime::InputMethodMenuManager* manager) OVERRIDE {
-    ++input_method_menu_item_changed_count_;
-  }
-
-  int input_method_changed_count_;
-  int input_method_menu_item_changed_count_;
-  bool last_show_message_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestObserver);
-};
-
-class TestCandidateWindowObserver
-    : public InputMethodManager::CandidateWindowObserver {
- public:
-  TestCandidateWindowObserver()
-      : candidate_window_opened_count_(0),
-        candidate_window_closed_count_(0) {
-  }
-  virtual ~TestCandidateWindowObserver() {}
-
-  virtual void CandidateWindowOpened(InputMethodManager* manager) OVERRIDE {
-    ++candidate_window_opened_count_;
-  }
-  virtual void CandidateWindowClosed(InputMethodManager* manager) OVERRIDE {
-    ++candidate_window_closed_count_;
-  }
-
-  int candidate_window_opened_count_;
-  int candidate_window_closed_count_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestCandidateWindowObserver);
-};
-}  // namespace
-
 TEST_F(InputMethodManagerImplTest, TestGetImeKeyboard) {
   EXPECT_TRUE(manager_->GetImeKeyboard());
   EXPECT_EQ(keyboard_, manager_->GetImeKeyboard());
@@ -1129,7 +1135,7 @@ TEST_F(InputMethodManagerImplTest, TestAddRemoveExtensionInputMethods) {
                                           GURL(),
                                           GURL());
   MockInputMethodEngine engine(descriptor1);
-  manager_->AddInputMethodExtension(ext1_id, &engine);
+  manager_->AddInputMethodExtension(GetProfile(), ext1_id, &engine);
 
   // Extension IMEs are not enabled by default.
   EXPECT_EQ(1U, manager_->GetNumActiveInputMethods());
@@ -1158,7 +1164,7 @@ TEST_F(InputMethodManagerImplTest, TestAddRemoveExtensionInputMethods) {
                                           GURL(),
                                           GURL());
   MockInputMethodEngine engine2(descriptor2);
-  manager_->AddInputMethodExtension(ext2_id, &engine2);
+  manager_->AddInputMethodExtension(GetProfile(), ext2_id, &engine2);
   EXPECT_EQ(2U, manager_->GetNumActiveInputMethods());
 
   extension_ime_ids.push_back(ext2_id);
@@ -1174,9 +1180,9 @@ TEST_F(InputMethodManagerImplTest, TestAddRemoveExtensionInputMethods) {
   }
 
   // Remove them.
-  manager_->RemoveInputMethodExtension(ext1_id);
+  manager_->RemoveInputMethodExtension(GetProfile(), ext1_id);
   EXPECT_EQ(2U, manager_->GetNumActiveInputMethods());
-  manager_->RemoveInputMethodExtension(ext2_id);
+  manager_->RemoveInputMethodExtension(GetProfile(), ext2_id);
   EXPECT_EQ(1U, manager_->GetNumActiveInputMethods());
 }
 
@@ -1210,7 +1216,7 @@ TEST_F(InputMethodManagerImplTest, TestAddExtensionInputThenLockScreen) {
                                          GURL(),
                                          GURL());
   MockInputMethodEngine engine(descriptor);
-  manager_->AddInputMethodExtension(ext_id, &engine);
+  manager_->AddInputMethodExtension(GetProfile(), ext_id, &engine);
 
   // Extension IME is not enabled by default.
   EXPECT_EQ(1U, manager_->GetNumActiveInputMethods());
diff --git a/chrome/browser/chromeos/input_method/mock_input_method_manager.cc b/chrome/browser/chromeos/input_method/mock_input_method_manager.cc
index a57df70..6d9a572 100644
--- a/chrome/browser/chromeos/input_method/mock_input_method_manager.cc
+++ b/chrome/browser/chromeos/input_method/mock_input_method_manager.cc
@@ -100,11 +100,13 @@ void MockInputMethodManager::ActivateInputMethodMenuItem(
 }
 
 void MockInputMethodManager::AddInputMethodExtension(
+    Profile* profile,
     const std::string& id,
     InputMethodEngineInterface* instance) {
 }
 
-void MockInputMethodManager::RemoveInputMethodExtension(const std::string& id) {
+void MockInputMethodManager::RemoveInputMethodExtension(Profile* profile,
+                                                        const std::string& id) {
 }
 
 void MockInputMethodManager::GetInputMethodExtensions(
diff --git a/chrome/browser/chromeos/input_method/mock_input_method_manager.h b/chrome/browser/chromeos/input_method/mock_input_method_manager.h
index 5f3e3f0..137f9de 100644
--- a/chrome/browser/chromeos/input_method/mock_input_method_manager.h
+++ b/chrome/browser/chromeos/input_method/mock_input_method_manager.h
@@ -48,9 +48,11 @@ class MockInputMethodManager : public InputMethodManager {
   virtual void ChangeInputMethod(const std::string& input_method_id) OVERRIDE;
   virtual void ActivateInputMethodMenuItem(const std::string& key) OVERRIDE;
   virtual void AddInputMethodExtension(
+      Profile* profile,
       const std::string& id,
       InputMethodEngineInterface* instance) OVERRIDE;
-  virtual void RemoveInputMethodExtension(const std::string& id) OVERRIDE;
+  virtual void RemoveInputMethodExtension(Profile* profile,
+                                          const std::string& id) OVERRIDE;
   virtual void GetInputMethodExtensions(
       InputMethodDescriptors* result) OVERRIDE;
   virtual void SetEnabledExtensionImes(std::vector<std::string>* ids) OVERRIDE;
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 b1d15c6..ff86a32 100644
--- a/chrome/browser/extensions/api/input_ime/input_ime_api.cc
+++ b/chrome/browser/extensions/api/input_ime/input_ime_api.cc
@@ -349,12 +349,12 @@ InputImeEventRouter::GetInstance() {
 }
 
 bool InputImeEventRouter::RegisterIme(
+    Profile* profile,
     const std::string& extension_id,
     const extensions::InputComponentInfo& component) {
 #if defined(USE_X11)
   VLOG(1) << "RegisterIme: " << extension_id << " id: " << component.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;
@@ -371,7 +371,8 @@ bool InputImeEventRouter::RegisterIme(
     scoped_ptr<chromeos::InputMethodEngineInterface::Observer> observer(
         new chromeos::ImeObserver(profile, extension_id));
     chromeos::InputMethodEngine* engine = new chromeos::InputMethodEngine();
-    engine->Initialize(observer.Pass(),
+    engine->Initialize(profile,
+                       observer.Pass(),
                        component.name.c_str(),
                        extension_id.c_str(),
                        component.id.c_str(),
@@ -854,14 +855,17 @@ void InputImeAPI::OnExtensionLoaded(content::BrowserContext* browser_context,
        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
+      // If |browser_context| looks like signin profile, use the real signin
+      // profile. This is because IME extensions for signin profile are run
+      // in Off-The-Record profile, based on given static defaults.
+      // 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);
+      Profile* profile = Profile::FromBrowserContext(browser_context);
+      if (chromeos::ProfileHelper::IsSigninProfile(profile))
+        profile = chromeos::ProfileHelper::GetSigninProfile();
+      input_ime_event_router()->RegisterIme(
+          profile, extension->id(), *component);
     }
   }
 }
diff --git a/chrome/browser/extensions/api/input_ime/input_ime_api.h b/chrome/browser/extensions/api/input_ime/input_ime_api.h
index 773d13b..0090e5d 100644
--- a/chrome/browser/extensions/api/input_ime/input_ime_api.h
+++ b/chrome/browser/extensions/api/input_ime/input_ime_api.h
@@ -36,7 +36,8 @@ class InputImeEventRouter {
  public:
   static InputImeEventRouter* GetInstance();
 
-  bool RegisterIme(const std::string& extension_id,
+  bool RegisterIme(Profile*,
+                   const std::string& extension_id,
                    const extensions::InputComponentInfo& component);
   void UnregisterAllImes(const std::string& extension_id);
   chromeos::InputMethodEngineInterface* GetEngine(