Handle Engine failure better.

1. Add an extra error param to GetEngineIfActive, so that we can tell if
it fails because the engine is unavailable or inactive.
2. Add LOGs for engine errors.
3. Add error handling to more functions.
4. Add some helper functions to reduce code duplication.
5. Minor change on constants name for readability.
6. Disable some browser tests, they were actually testing nothing before (because there was no active engine, they actually skipped everything...). Eventually we should rewrite those tests in the new fashion as https://cs.chromium.org/chromium/src/chrome/browser/chromeos/input_method/input_method_engine_browsertests.cc

R=keithlee, shend, shuchen

Bug: 1024606
Change-Id: I9902e42a472122c5c1a02c36e70f71b52cdd80d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1933855Reviewed-by: Keith Lee <keithlee@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Keith Lee <keithlee@chromium.org>
Commit-Queue: Jing Wang <jiwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719454}

chrome/browser/chromeos/extensions/input_method_api.cc

@@ -80,7 +80,20 @@ namespace {
 const char kXkbPrefix[] = "xkb:";
 const char kErrorFailToShowInputView[] =
     "Unable to show the input view window.";
-const char kInputImeApiEngineNotAvaliable[] = "Engine is not available";
+const char kErrorRouterNotAvailable[] = "The router is not available.";
+
+InputMethodEngineBase* GetEngineIfActive(
+    content::BrowserContext* browser_context,
+    const std::string& extension_id,
+    std::string* error) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  extensions::InputImeEventRouter* event_router =
+      extensions::GetInputImeEventRouter(profile);
+  CHECK(event_router) << kErrorRouterNotAvailable;
+  InputMethodEngineBase* engine =
+      event_router->GetEngineIfActive(extension_id, error);
+  return engine;
+}
 
 }  // namespace
 
@@ -344,11 +357,12 @@ ExtensionFunction::ResponseAction InputMethodPrivateSetSettingFunction::Run() {
 
 ExtensionFunction::ResponseAction
 InputMethodPrivateSetCompositionRangeFunction::Run() {
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
-  if (engine) {
+      GetEngineIfActive(browser_context(), extension_id(), &error);
+  if (!engine)
+    return RespondNow(Error(error));
+
   const auto parent_params = SetCompositionRange::Params::Create(*args_);
   const auto& params = parent_params->parameters;
   std::vector<InputMethodEngineBase::SegmentInfo> segments;
@@ -383,38 +397,29 @@ InputMethodPrivateSetCompositionRangeFunction::Run() {
     segment_info.style = InputMethodEngineBase::SEGMENT_STYLE_UNDERLINE;
     segments.push_back(segment_info);
   }
-    std::string error;
+
   if (!engine->SetCompositionRange(params.context_id, params.selection_before,
-                                     params.selection_after, segments,
-                                     &error)) {
+                                   params.selection_after, segments, &error)) {
     auto results = std::make_unique<base::ListValue>();
     results->Append(std::make_unique<base::Value>(false));
     return RespondNow(ErrorWithArguments(std::move(results), error));
   }
-  }
   return RespondNow(OneArgument(std::make_unique<base::Value>(true)));
 }
 
 ExtensionFunction::ResponseAction
 InputMethodPrivateSetSelectionRangeFunction::Run() {
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
-
-  if (!engine) {
-    auto results = std::make_unique<base::ListValue>();
-    results->Append(std::make_unique<base::Value>(false));
-    return RespondNow(
-        ErrorWithArguments(std::move(results), kInputImeApiEngineNotAvaliable));
-  }
+      GetEngineIfActive(browser_context(), extension_id(), &error);
+  if (!engine)
+    return RespondNow(Error(error));
 
   std::unique_ptr<SetSelectionRange::Params> parent_params(
       SetSelectionRange::Params::Create(*args_));
   const SetSelectionRange::Params::Parameters& params =
       parent_params->parameters;
 
-  std::string error;
   if (!engine->SetSelectionRange(params.context_id, *params.selection_start,
                                  *params.selection_end, &error)) {
     auto results = std::make_unique<base::ListValue>();

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

@@ -19,9 +19,22 @@ namespace SendKeyEvents = extensions::api::input_ime::SendKeyEvents;
 using input_method::InputMethodEngineBase;
 
 namespace {
-const char kInputImeApiErrorEngineNotAvailable[] = "Engine is not available";
-const char kInputImeApiErrorSetKeyEventsFail[] = "Could not send key events";
+const char kErrorRouterNotAvailable[] = "The router is not available.";
+const char kErrorSetKeyEventsFail[] = "Could not send key events";
+
+InputMethodEngineBase* GetEngineIfActive(Profile* profile,
+                                         const std::string& extension_id,
+                                         std::string* error) {
+  extensions::InputImeEventRouter* event_router =
+      extensions::GetInputImeEventRouter(profile);
+  CHECK(event_router) << kErrorRouterNotAvailable;
+  InputMethodEngineBase* engine =
+      event_router->GetEngineIfActive(extension_id, error);
+  return engine;
+}
+
 }  // namespace
+
 namespace ui {
 
 ImeObserver::ImeObserver(const std::string& extension_id, Profile* profile)
@@ -89,13 +102,13 @@ void ImeObserver::OnKeyEvent(
     return;
   }
 
-  extensions::InputImeEventRouter* event_router =
-      extensions::GetInputImeEventRouter(profile_);
-  if (!event_router || !event_router->GetEngineIfActive(extension_id_))
+  std::string error;
+  InputMethodEngineBase* engine =
+      GetEngineIfActive(profile_, extension_id_, &error);
+  if (!engine)
     return;
   const std::string request_id =
-      event_router->GetEngineIfActive(extension_id_)
-          ->AddPendingKeyEvent(component_id, std::move(callback));
+      engine->AddPendingKeyEvent(component_id, std::move(callback));
 
   input_ime::KeyboardEvent key_data_value;
   key_data_value.type = input_ime::ParseKeyboardEventType(event.type);
@@ -303,27 +316,26 @@ void InputImeEventRouterFactory::RemoveProfile(Profile* profile) {
 ExtensionFunction::ResponseAction InputImeKeyEventHandledFunction::Run() {
   std::unique_ptr<KeyEventHandled::Params> params(
       KeyEventHandled::Params::Create(*args_));
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
-  InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
-  if (engine) {
-    engine->KeyEventHandled(extension_id(), params->request_id,
-                            params->response);
-  }
+  std::string error;
+  InputMethodEngineBase* engine = GetEngineIfActive(
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
+  if (!engine)
+    return RespondNow(Error(error));
+
+  engine->KeyEventHandled(extension_id(), params->request_id, params->response);
   return RespondNow(NoArguments());
 }
 
 ExtensionFunction::ResponseAction InputImeSetCompositionFunction::Run() {
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
-  InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
-  if (engine) {
+  std::string error;
+  InputMethodEngineBase* engine = GetEngineIfActive(
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
+  if (!engine)
+    return RespondNow(Error(error));
+
   std::unique_ptr<SetComposition::Params> parent_params(
       SetComposition::Params::Create(*args_));
-    const SetComposition::Params::Parameters& params =
-        parent_params->parameters;
+  const SetComposition::Params::Parameters& params = parent_params->parameters;
   std::vector<InputMethodEngineBase::SegmentInfo> segments;
   if (params.segments) {
     for (const auto& segments_arg : *params.segments) {
@@ -339,8 +351,7 @@ ExtensionFunction::ResponseAction InputImeSetCompositionFunction::Run() {
         segment_info.style =
             InputMethodEngineBase::SEGMENT_STYLE_DOUBLE_UNDERLINE;
       } else {
-          segment_info.style =
-              InputMethodEngineBase::SEGMENT_STYLE_NO_UNDERLINE;
+        segment_info.style = InputMethodEngineBase::SEGMENT_STYLE_NO_UNDERLINE;
       }
       segments.push_back(segment_info);
     }
@@ -349,7 +360,6 @@ ExtensionFunction::ResponseAction InputImeSetCompositionFunction::Run() {
       params.selection_start ? *params.selection_start : params.cursor;
   int selection_end =
       params.selection_end ? *params.selection_end : params.cursor;
-    std::string error;
   if (!engine->SetComposition(params.context_id, params.text.c_str(),
                               selection_start, selection_end, params.cursor,
                               segments, &error)) {
@@ -358,37 +368,34 @@ ExtensionFunction::ResponseAction InputImeSetCompositionFunction::Run() {
     results->Append(std::make_unique<base::Value>(false));
     return RespondNow(ErrorWithArguments(std::move(results), error));
   }
-  }
   return RespondNow(OneArgument(std::make_unique<base::Value>(true)));
 }
 
 ExtensionFunction::ResponseAction InputImeCommitTextFunction::Run() {
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
-  InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
-  if (engine) {
+  std::string error;
+  InputMethodEngineBase* engine = GetEngineIfActive(
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
+  if (!engine)
+    return RespondNow(Error(error));
+
   std::unique_ptr<CommitText::Params> parent_params(
       CommitText::Params::Create(*args_));
   const CommitText::Params::Parameters& params = parent_params->parameters;
-    std::string error;
   if (!engine->CommitText(params.context_id, params.text.c_str(), &error)) {
     std::unique_ptr<base::ListValue> results =
         std::make_unique<base::ListValue>();
     results->Append(std::make_unique<base::Value>(false));
     return RespondNow(ErrorWithArguments(std::move(results), error));
   }
-  }
   return RespondNow(OneArgument(std::make_unique<base::Value>(true)));
 }
 
 ExtensionFunction::ResponseAction InputImeSendKeyEventsFunction::Run() {
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
-  InputMethodEngineBase* engine =
-      event_router ? event_router->GetEngineIfActive(extension_id()) : nullptr;
+  std::string error;
+  InputMethodEngineBase* engine = GetEngineIfActive(
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kInputImeApiErrorEngineNotAvailable));
+    return RespondNow(Error(error));
 
   std::unique_ptr<SendKeyEvents::Params> parent_params(
       SendKeyEvents::Params::Create(*args_));
@@ -411,7 +418,7 @@ ExtensionFunction::ResponseAction InputImeSendKeyEventsFunction::Run() {
     event.caps_lock = key_event.caps_lock ? *(key_event.caps_lock) : false;
   }
   if (!engine->SendKeyEvents(params.context_id, key_data_out))
-    return RespondNow(Error(kInputImeApiErrorSetKeyEventsFail));
+    return RespondNow(Error(kErrorSetKeyEventsFail));
   return RespondNow(NoArguments());
 }
 

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

@@ -57,14 +57,11 @@ using input_method::InputMethodEngineBase;
 using chromeos::InputMethodEngine;
 
 namespace {
-const char kInputImeApiChromeOSErrorEngineNotAvailable[] =
-    "Engine is not available";
-const char kInputImeApiChromeOSErrorSetMenuItemsFail[] =
-    "Could not create menu Items";
-const char kInputImeApiChromeOSErrorUpdateMenuItemsFail[] =
-    "Could not update menu Items";
-const char kInputImeApiChromeOSErrorEngineNotActive[] =
-    "The engine is not active.";
+const char kErrorEngineNotAvailable[] = "The engine is not available.";
+const char kErrorSetMenuItemsFail[] = "Could not create menu Items";
+const char kErrorUpdateMenuItemsFail[] = "Could not update menu Items";
+const char kErrorEngineNotActive[] = "The engine is not active.";
+const char kErrorRouterNotAvailable[] = "The router is not available.";
 
 void SetMenuItemToMenu(
     const input_ime::MenuItem& input,
@@ -387,12 +384,26 @@ class ImeObserverChromeOS : public ui::ImeObserver {
 namespace extensions {
 
 InputMethodEngine* GetEngineIfActive(Profile* profile,
-                                     const std::string& extension_id) {
+                                     const std::string& extension_id,
+                                     std::string* error) {
+  InputImeEventRouter* event_router = GetInputImeEventRouter(profile);
+  CHECK(event_router) << kErrorRouterNotAvailable;
+  InputMethodEngine* engine = static_cast<InputMethodEngine*>(
+      event_router->GetEngineIfActive(extension_id, error));
+  return engine;
+}
+
+InputMethodEngine* GetEngine(content::BrowserContext* browser_context,
+                             const std::string& extension_id,
+                             std::string* error) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
   InputImeEventRouter* event_router = GetInputImeEventRouter(profile);
+  CHECK(event_router) << kErrorRouterNotAvailable;
   InputMethodEngine* engine =
-      event_router ? static_cast<InputMethodEngine*>(
-                         event_router->GetEngineIfActive(extension_id))
-                   : nullptr;
+      static_cast<InputMethodEngine*>(event_router->GetEngine(extension_id));
+  DCHECK(engine) << kErrorEngineNotAvailable;
+  if (!engine)
+    *error = kErrorEngineNotAvailable;
   return engine;
 }
 
@@ -477,21 +488,37 @@ void InputImeEventRouter::UnregisterAllImes(const std::string& extension_id) {
 InputMethodEngine* InputImeEventRouter::GetEngine(
     const std::string& extension_id) {
   auto it = engine_map_.find(extension_id);
-  return (it != engine_map_.end()) ? it->second.get() : nullptr;
+  if (it == engine_map_.end()) {
+    LOG(ERROR) << kErrorEngineNotAvailable << " extension id: " << extension_id;
+    return nullptr;
+  } else {
+    return it->second.get();
+  }
 }
 
 InputMethodEngineBase* InputImeEventRouter::GetEngineIfActive(
-    const std::string& extension_id) {
+    const std::string& extension_id,
+    std::string* error) {
   auto it = engine_map_.find(extension_id);
-  return (it != engine_map_.end() && it->second->IsActive()) ? it->second.get()
-                                                             : nullptr;
+  if (it == engine_map_.end()) {
+    LOG(ERROR) << kErrorEngineNotAvailable << " extension id: " << extension_id;
+    *error = kErrorEngineNotAvailable;
+    return nullptr;
+  } else if (it->second->IsActive()) {
+    return it->second.get();
+  } else {
+    LOG(WARNING) << kErrorEngineNotActive << " extension id: " << extension_id;
+    *error = kErrorEngineNotActive;
+    return nullptr;
+  }
 }
 
 ExtensionFunction::ResponseAction InputImeClearCompositionFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine) {
-    return RespondNow(OneArgument(std::make_unique<base::Value>(false)));
+    return RespondNow(Error(error));
   }
 
   std::unique_ptr<ClearComposition::Params> parent_params(
@@ -499,7 +526,6 @@ ExtensionFunction::ResponseAction InputImeClearCompositionFunction::Run() {
   const ClearComposition::Params::Parameters& params =
       parent_params->parameters;
 
-  std::string error;
   bool success = engine->ClearComposition(params.context_id, &error);
   std::unique_ptr<base::ListValue> results =
       std::make_unique<base::ListValue>();
@@ -509,10 +535,11 @@ ExtensionFunction::ResponseAction InputImeClearCompositionFunction::Run() {
 }
 
 ExtensionFunction::ResponseAction InputImeHideInputViewFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine)
-    return RespondNow(NoArguments());
+    return RespondNow(Error(error));
   engine->HideInputView();
   return RespondNow(NoArguments());
 }
@@ -524,18 +551,16 @@ InputImeSetCandidateWindowPropertiesFunction::Run() {
   const SetCandidateWindowProperties::Params::Parameters&
       params = parent_params->parameters;
 
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngine* engine =
-      event_router ? event_router->GetEngine(extension_id()) : nullptr;
+      GetEngine(browser_context(), extension_id(), &error);
   if (!engine) {
-    return RespondNow(OneArgument(std::make_unique<base::Value>(false)));
+    return RespondNow(Error(error));
   }
 
   const SetCandidateWindowProperties::Params::Parameters::Properties&
       properties = params.properties;
 
-  std::string error;
   if (properties.visible &&
       !engine->SetCandidateWindowVisible(*properties.visible, &error)) {
     std::unique_ptr<base::ListValue> results =
@@ -590,10 +615,11 @@ InputImeSetCandidateWindowPropertiesFunction::Run() {
 }
 
 ExtensionFunction::ResponseAction InputImeSetCandidatesFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine) {
-    return RespondNow(OneArgument(std::make_unique<base::Value>(true)));
+    return RespondNow(Error(error));
   }
 
   std::unique_ptr<SetCandidates::Params> parent_params(
@@ -616,7 +642,6 @@ ExtensionFunction::ResponseAction InputImeSetCandidatesFunction::Run() {
     }
   }
 
-  std::string error;
   bool success =
       engine->SetCandidates(params.context_id, candidates_out, &error);
   std::unique_ptr<base::ListValue> results =
@@ -627,10 +652,11 @@ ExtensionFunction::ResponseAction InputImeSetCandidatesFunction::Run() {
 }
 
 ExtensionFunction::ResponseAction InputImeSetCursorPositionFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine) {
-    return RespondNow(OneArgument(std::make_unique<base::Value>(false)));
+    return RespondNow(Error(error));
   }
 
   std::unique_ptr<SetCursorPosition::Params> parent_params(
@@ -638,7 +664,6 @@ ExtensionFunction::ResponseAction InputImeSetCursorPositionFunction::Run() {
   const SetCursorPosition::Params::Parameters& params =
       parent_params->parameters;
 
-  std::string error;
   bool success =
       engine->SetCursorPosition(params.context_id, params.candidate_id, &error);
   std::unique_ptr<base::ListValue> results =
@@ -654,12 +679,12 @@ ExtensionFunction::ResponseAction InputImeSetMenuItemsFunction::Run() {
   const SetMenuItems::Params::Parameters& params =
       parent_params->parameters;
 
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngine* engine =
-      event_router ? event_router->GetEngine(extension_id()) : nullptr;
-  if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+      GetEngine(browser_context(), extension_id(), &error);
+  if (!engine) {
+    return RespondNow(Error(error));
+  }
 
   std::vector<chromeos::input_method::InputMethodManager::MenuItem> items_out;
   for (const input_ime::MenuItem& item_in : params.items) {
@@ -668,7 +693,7 @@ ExtensionFunction::ResponseAction InputImeSetMenuItemsFunction::Run() {
   }
 
   if (!engine->SetMenuItems(items_out))
-    return RespondNow(Error(kInputImeApiChromeOSErrorSetMenuItemsFail));
+    return RespondNow(Error(kErrorSetMenuItemsFail));
   return RespondNow(NoArguments());
 }
 
@@ -678,12 +703,12 @@ ExtensionFunction::ResponseAction InputImeUpdateMenuItemsFunction::Run() {
   const UpdateMenuItems::Params::Parameters& params =
       parent_params->parameters;
 
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngine* engine =
-      event_router ? event_router->GetEngine(extension_id()) : nullptr;
-  if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+      GetEngine(browser_context(), extension_id(), &error);
+  if (!engine) {
+    return RespondNow(Error(error));
+  }
 
   std::vector<chromeos::input_method::InputMethodManager::MenuItem> items_out;
   for (const input_ime::MenuItem& item_in : params.items) {
@@ -692,7 +717,7 @@ ExtensionFunction::ResponseAction InputImeUpdateMenuItemsFunction::Run() {
   }
 
   if (!engine->UpdateMenuItems(items_out))
-    return RespondNow(Error(kInputImeApiChromeOSErrorUpdateMenuItemsFail));
+    return RespondNow(Error(kErrorUpdateMenuItemsFail));
   return RespondNow(NoArguments());
 }
 
@@ -702,14 +727,13 @@ ExtensionFunction::ResponseAction InputImeDeleteSurroundingTextFunction::Run() {
   const DeleteSurroundingText::Params::Parameters& params =
       parent_params->parameters;
 
-  InputImeEventRouter* event_router =
-      GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
+  std::string error;
   InputMethodEngine* engine =
-      event_router ? event_router->GetEngine(extension_id()) : nullptr;
-  if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+      GetEngine(browser_context(), extension_id(), &error);
+  if (!engine) {
+    return RespondNow(Error(error));
+  }
 
-  std::string error;
   engine->DeleteSurroundingText(params.context_id, params.offset, params.length,
                                 &error);
   return RespondNow(error.empty() ? NoArguments() : Error(error));
@@ -717,15 +741,15 @@ ExtensionFunction::ResponseAction InputImeDeleteSurroundingTextFunction::Run() {
 
 ExtensionFunction::ResponseAction
 InputMethodPrivateFinishComposingTextFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+    return RespondNow(Error(error));
   std::unique_ptr<FinishComposingText::Params> parent_params(
       FinishComposingText::Params::Create(*args_));
   const FinishComposingText::Params::Parameters& params =
       parent_params->parameters;
-  std::string error;
   engine->FinishComposingText(params.context_id, &error);
   return RespondNow(error.empty() ? NoArguments() : Error(error));
 }
@@ -739,25 +763,28 @@ InputMethodPrivateNotifyImeMenuItemActivatedFunction::Run() {
   std::string active_extension_id =
       chromeos::extension_ime_util::GetExtensionIDFromInputMethodID(
           current_input_method.id());
-  InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), active_extension_id);
+  std::string error;
+  InputMethodEngine* engine =
+      GetEngineIfActive(Profile::FromBrowserContext(browser_context()),
+                        active_extension_id, &error);
   if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+    return RespondNow(Error(error));
 
   std::unique_ptr<NotifyImeMenuItemActivated::Params> params(
       NotifyImeMenuItemActivated::Params::Create(*args_));
   if (params->engine_id != engine->GetActiveComponentId())
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotActive));
+    return RespondNow(Error(kErrorEngineNotActive));
   engine->PropertyActivate(params->name);
   return RespondNow(NoArguments());
 }
 
 ExtensionFunction::ResponseAction
 InputMethodPrivateGetCompositionBoundsFunction::Run() {
+  std::string error;
   InputMethodEngine* engine = GetEngineIfActive(
-      Profile::FromBrowserContext(browser_context()), extension_id());
+      Profile::FromBrowserContext(browser_context()), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kInputImeApiChromeOSErrorEngineNotAvailable));
+    return RespondNow(Error(error));
 
   auto bounds_list = std::make_unique<base::ListValue>();
   for (const auto& bounds : engine->composition_bounds()) {
@@ -785,8 +812,10 @@ void InputImeAPI::OnExtensionLoaded(content::BrowserContext* browser_context,
       // reloaded we should reactivate the engine so that the IME extension
       // can receive the onActivate event to recover itself upon the
       // unexpected unload.
+      std::string error;
       InputMethodEngineBase* engine =
-          event_router->GetEngineIfActive(extension->id());
+          event_router->GetEngineIfActive(extension->id(), &error);
+      DCHECK(engine) << error;
       // When extension is unloaded unexpectedly and reloaded, OS doesn't pass
       // details.browser_context value in OnListenerAdded callback. So we need
       // to reactivate engine here.
@@ -842,9 +871,10 @@ void InputImeAPI::OnExtensionUnloaded(content::BrowserContext* browser_context,
 void InputImeAPI::OnListenerAdded(const EventListenerInfo& details) {
   if (!details.browser_context)
     return;
+  std::string error;
   InputMethodEngine* engine =
       GetEngineIfActive(Profile::FromBrowserContext(details.browser_context),
-                        details.extension_id);
+                        details.extension_id, &error);
   // Notifies the IME extension for IME ready with onActivate/onFocus events.
   if (engine)
     engine->Enable(engine->GetActiveComponentId());

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

@@ -177,7 +177,8 @@ class InputImeEventRouter : public InputImeEventRouterBase {
 
   chromeos::InputMethodEngine* GetEngine(const std::string& extension_id);
   input_method::InputMethodEngineBase* GetEngineIfActive(
-      const std::string& extension_id) override;
+      const std::string& extension_id,
+      std::string* error) override;
 
   std::string GetUnloadedExtensionId() const {
     return unloaded_component_extension_id_;

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

@@ -35,7 +35,8 @@ namespace input_ime = extensions::api::input_ime;
 
 namespace {
 
-const char kErrorNoActiveEngine[] = "The extension has not been activated.";
+const char kErrorEngineNotActive[] = "The engine is not active.";
+const char kErrorRouterNotAvailable[] = "The router is not available.";
 const char kErrorPermissionDenied[] = "User denied permission.";
 const char kErrorCouldNotFindActiveBrowser[] =
     "Cannot find the active browser.";
@@ -123,13 +124,13 @@ class ImeObserverNonChromeOS : public ui::ImeObserver {
 namespace extensions {
 
 InputMethodEngine* GetEngineIfActive(content::BrowserContext* browser_context,
-                                     const std::string& extension_id) {
+                                     const std::string& extension_id,
+                                     std::string* error) {
   Profile* profile = Profile::FromBrowserContext(browser_context);
   InputImeEventRouter* event_router = GetInputImeEventRouter(profile);
-  InputMethodEngine* engine =
-      event_router ? static_cast<InputMethodEngine*>(
-                         event_router->GetEngineIfActive(extension_id))
-                   : nullptr;
+  CHECK(event_router) << kErrorRouterNotAvailable;
+  InputMethodEngine* engine = static_cast<InputMethodEngine*>(
+      event_router->GetEngineIfActive(extension_id, error));
   return engine;
 }
 
@@ -175,12 +176,16 @@ InputImeEventRouter::~InputImeEventRouter() {
 }
 
 InputMethodEngineBase* InputImeEventRouter::GetEngineIfActive(
-    const std::string& extension_id) {
-  return (ui::IMEBridge::Get()->GetCurrentEngineHandler() &&
-          active_engine_ &&
-          active_engine_->GetExtensionId() == extension_id)
-             ? active_engine_
-             : nullptr;
+    const std::string& extension_id,
+    std::string* error) {
+  if (ui::IMEBridge::Get()->GetCurrentEngineHandler() && active_engine_ &&
+      active_engine_->GetExtensionId() == extension_id) {
+    return active_engine_;
+  } else {
+    LOG(WARNING) << kErrorEngineNotActive << " extension id: " << extension_id;
+    *error = kErrorEngineNotActive;
+    return nullptr;
+  }
 }
 
 void InputImeEventRouter::SetActiveEngine(const std::string& extension_id) {
@@ -229,7 +234,7 @@ ExtensionFunction::ResponseAction InputImeActivateFunction::Run() {
   Profile* profile = Profile::FromBrowserContext(browser_context());
   InputImeEventRouter* event_router = GetInputImeEventRouter(profile);
   if (!event_router)
-    return RespondNow(Error(kErrorNoActiveEngine));
+    return RespondNow(Error(kErrorEngineNotActive));
 
   ExtensionPrefs* prefs = ExtensionPrefs::Get(profile);
 
@@ -306,7 +311,7 @@ void InputImeActivateFunction::OnPermissionBubbleFinished(
   Profile* profile = Profile::FromBrowserContext(browser_context());
   InputImeEventRouter* event_router = GetInputImeEventRouter(profile);
   if (!event_router) {
-    Respond(Error(kErrorNoActiveEngine));
+    Respond(Error(kErrorEngineNotActive));
     return;
   }
   event_router->SetActiveEngine(extension_id());
@@ -323,8 +328,12 @@ void InputImeActivateFunction::OnPermissionBubbleFinished(
 }
 
 ExtensionFunction::ResponseAction InputImeDeactivateFunction::Run() {
+  std::string error;
   InputMethodEngine* engine =
-      GetEngineIfActive(browser_context(), extension_id());
+      GetEngineIfActive(browser_context(), extension_id(), &error);
+  if (!engine) {
+    return RespondNow(Error(error));
+  }
   ui::IMEBridge::Get()->SetCurrentEngineHandler(nullptr);
   if (engine)
     engine->CloseImeWindows();
@@ -351,12 +360,12 @@ ExtensionFunction::ResponseAction InputImeCreateWindowFunction::Run() {
     bounds.set_height(options.bounds->height);
   }
 
+  std::string error;
   InputMethodEngine* engine =
-      GetEngineIfActive(browser_context(), extension_id());
+      GetEngineIfActive(browser_context(), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kErrorNoActiveEngine));
+    return RespondNow(Error(error));
 
-  std::string error;
   int frame_id = engine->CreateImeWindow(
       extension(), render_frame_host(),
       options.url.get() ? *options.url : url::kAboutBlankURL,
@@ -374,10 +383,11 @@ ExtensionFunction::ResponseAction InputImeCreateWindowFunction::Run() {
 }
 
 ExtensionFunction::ResponseAction InputImeShowWindowFunction::Run() {
+  std::string error;
   InputMethodEngine* engine =
-      GetEngineIfActive(browser_context(), extension_id());
+      GetEngineIfActive(browser_context(), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kErrorNoActiveEngine));
+    return RespondNow(Error(error));
 
   std::unique_ptr<api::input_ime::ShowWindow::Params> params(
       api::input_ime::ShowWindow::Params::Create(*args_));
@@ -387,10 +397,11 @@ ExtensionFunction::ResponseAction InputImeShowWindowFunction::Run() {
 }
 
 ExtensionFunction::ResponseAction InputImeHideWindowFunction::Run() {
+  std::string error;
   InputMethodEngine* engine =
-      GetEngineIfActive(browser_context(), extension_id());
+      GetEngineIfActive(browser_context(), extension_id(), &error);
   if (!engine)
-    return RespondNow(Error(kErrorNoActiveEngine));
+    return RespondNow(Error(error));
 
   std::unique_ptr<api::input_ime::HideWindow::Params> params(
       api::input_ime::HideWindow::Params::Create(*args_));

chrome/browser/extensions/api/input_ime/input_ime_api_nonchromeos.h

@@ -35,7 +35,8 @@ class InputImeEventRouter : public InputImeEventRouterBase {
 
   // Gets the input method engine if the extension is active.
   input_method::InputMethodEngineBase* GetEngineIfActive(
-      const std::string& extension_id) override;
+      const std::string& extension_id,
+      std::string* error) override;
 
   // Actives the extension with new input method engine, and deletes the
   // previous engine if another extension was active.

chrome/browser/extensions/api/input_ime/input_ime_event_router_base.h

@@ -23,7 +23,8 @@ class InputImeEventRouterBase {
 
   // Gets the input method engine if the extension is active.
   virtual input_method::InputMethodEngineBase* GetEngineIfActive(
-      const std::string& extension_id) = 0;
+      const std::string& extension_id,
+      std::string* error) = 0;
 
   Profile* GetProfile() const { return profile_; }
 

chrome/test/data/extensions/api_test/input_ime/background.js

@@ -2,36 +2,36 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-function setCompositionTest() {
-  chrome.input.ime.setComposition({
-    "contextID": 1,
-    "text": "Pie",
-    "selectionStart": 1,
-    "selectionEnd": 2,
-    "cursor": 3,
-    "segments": [{
-      "start": 0,
-      "end": 1,
-      "style": "underline"
-    }]
-  }, chrome.test.callbackPass());
-}
-
-
-function clearCompositionTest() {
-  chrome.input.ime.clearComposition({
-    "contextID": 1
-  }, chrome.test.callbackPass());
-}
-
+// Disabled: crbug.com/641425.
+// function setCompositionTest() {
+//   chrome.input.ime.setComposition({
+//     "contextID": 1,
+//     "text": "Pie",
+//     "selectionStart": 1,
+//     "selectionEnd": 2,
+//     "cursor": 3,
+//     "segments": [{
+//       "start": 0,
+//       "end": 1,
+//       "style": "underline"
+//     }]
+//   }, chrome.test.callbackPass());
+// }
 
-function commitTextTest() {
-  chrome.input.ime.commitText({
-    "contextID": 2,
-    "text": "Seaguls"
-  }, chrome.test.callbackPass());
-}
+// Disabled: crbug.com/641425.
+// function clearCompositionTest() {
+//   chrome.input.ime.clearComposition({
+//     "contextID": 1
+//   }, chrome.test.callbackPass());
+// }
 
+// Disabled: crbug.com/641425.
+// function commitTextTest() {
+//   chrome.input.ime.commitText({
+//     "contextID": 2,
+//     "text": "Seaguls"
+//   }, chrome.test.callbackPass());
+// }
 
 // Disabled: crbug.com/641425.
 // function setCandidateWindowPropertiesTest() {
@@ -48,37 +48,36 @@ function commitTextTest() {
 //   }, chrome.test.callbackPass());
 // }
 
+// Disabled: crbug.com/641425.
+// function setCandidatesTest() {
+//   chrome.input.ime.setCandidates({
+//     "contextID": 8,
+//     "candidates": [{
+//       "candidate": "one",
+//       "id": 1,
+//       "label": "first",
+//       "annotation": "The first one"
+//     }, {
+//       "candidate": "two",
+//       "id": 2,
+//       "label": "second",
+//       "annotation": "The second one"
+//     }, {
+//       "candidate": "three",
+//       "id": 3,
+//       "label": "third",
+//       "annotation": "The third one"
+//     }]
+//   }, chrome.test.callbackPass());
+// }
 
-function setCandidatesTest() {
-  chrome.input.ime.setCandidates({
-    "contextID": 8,
-    "candidates": [{
-      "candidate": "one",
-      "id": 1,
-      "label": "first",
-      "annotation": "The first one"
-    }, {
-      "candidate": "two",
-      "id": 2,
-      "label": "second",
-      "annotation": "The second one"
-    }, {
-      "candidate": "three",
-      "id": 3,
-      "label": "third",
-      "annotation": "The third one"
-    }]
-  }, chrome.test.callbackPass());
-}
-
-
-function setCursorPositionTest() {
-  chrome.input.ime.setCursorPosition({
-    "contextID": 9,
-    "candidateID": 1
-  }, chrome.test.callbackPass());
-}
-
+// Disabled: crbug.com/641425.
+// function setCursorPositionTest() {
+//   chrome.input.ime.setCursorPosition({
+//     "contextID": 9,
+//     "candidateID": 1
+//   }, chrome.test.callbackPass());
+// }
 
 // Disabled: crbug.com/641425.
 // function setMenuItemsTest() {
@@ -125,12 +124,12 @@ function setCursorPositionTest() {
 // }
 
 chrome.test.runTests([
-    setCompositionTest,
-    clearCompositionTest,
-    commitTextTest,
+    // setCompositionTest,
+    // clearCompositionTest,
+    // commitTextTest,
     // setCandidateWindowPropertiesTest,
-    setCandidatesTest,
-    setCursorPositionTest,
+    // setCandidatesTest,
+    // setCursorPositionTest,
     // setMenuItemsTest,
     // updateMenuItemsTest,
     // deleteSurroundingText,