[Extensions] Isolate ExtensionFunction results_ and error_

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/input_ime/input_ime_api.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/macros.h"
@@ skipping 352 matching lines @@
   if (!engine) {
     return RespondNow(
         OneArgument(base::MakeUnique<base::FundamentalValue>(false)));
   }
 
   std::unique_ptr<ClearComposition::Params> parent_params(
       ClearComposition::Params::Create(*args_));
   const ClearComposition::Params::Parameters& params =
       parent_params->parameters;
 
-  return RespondNow(OneArgument(base::MakeUnique<base::FundamentalValue>(
-      engine->ClearComposition(params.context_id, &error_))));
+  std::string error;
+  bool success = engine->ClearComposition(params.context_id, &error);
+  SetError(error);

[lazyboy, 2016/09/30 01:16:04]

if (!success) SetError(), unless I'm missing something.

[Devlin, 2016/09/30 18:29:09]

In this CL, I was trying to keep things as close to the original implementation
as possible, which for the IME api, is kind of unfortunate (see
crbug.com/641425).  But it looks like this doesn't break tests, so done.

[lazyboy, 2016/09/30 20:43:32]

The only implementation (non-mock) of ClearComposition I found was in:
InputMethodEngineBase::ClearComposition(), which doesn't set error if it's
returning true, so this *should* be equivalent.

Same is true for
SetCandidates/SetCandidateWindowVisible/SetCursorPosition/DeleteSurroundingText.

[Devlin, 2016/10/01 01:03:39]

Acknowledged.

+  return RespondNow(
+      OneArgument(base::MakeUnique<base::FundamentalValue>(success)));
 }
 
 bool InputImeHideInputViewFunction::RunAsync() {
   InputMethodEngine* engine = GetActiveEngine(
       Profile::FromBrowserContext(browser_context()), extension_id());
   if (!engine) {
     return true;
   }
   engine->HideInputView();
   return true;
@@ skipping 12 matching lines @@
       event_router ? event_router->GetEngine(extension_id(), params.engine_id)
                    : nullptr;
   if (!engine) {
     return RespondNow(
         OneArgument(base::MakeUnique<base::FundamentalValue>(false)));
   }
 
   const SetCandidateWindowProperties::Params::Parameters::Properties&
       properties = params.properties;
 
+  std::string error;
   if (properties.visible &&
-      !engine->SetCandidateWindowVisible(*properties.visible, &error_)) {
+      !engine->SetCandidateWindowVisible(*properties.visible, &error)) {
+    SetError(error);
     return RespondNow(
         OneArgument(base::MakeUnique<base::FundamentalValue>(false)));
   }
 
   InputMethodEngine::CandidateWindowProperty properties_out =
       engine->GetCandidateWindowProperty();
   bool modified = false;
 
   if (properties.cursor_visible) {
     properties_out.is_cursor_visible = *properties.cursor_visible;
@@ skipping 58 matching lines @@
     if (candidate_in.label)
       candidates_out.back().label = *candidate_in.label;
     if (candidate_in.annotation)
       candidates_out.back().annotation = *candidate_in.annotation;
     if (candidate_in.usage) {
       candidates_out.back().usage.title = candidate_in.usage->title;
       candidates_out.back().usage.body = candidate_in.usage->body;
     }
   }
 
-  return RespondNow(OneArgument(base::MakeUnique<base::FundamentalValue>(
-      engine->SetCandidates(params.context_id, candidates_out, &error_))));
+  std::string error;
+  bool success =
+      engine->SetCandidates(params.context_id, candidates_out, &error);
+  SetError(error);

[lazyboy, 2016/09/30 01:16:04]

Same here and in other places.
Unless we implicitly depend on errors to be always set, in that case, add
comments.

[Devlin, 2016/09/30 18:29:09]

Done.

+  return RespondNow(
+      OneArgument(base::MakeUnique<base::FundamentalValue>(success)));
 }
 
 ExtensionFunction::ResponseAction InputImeSetCursorPositionFunction::Run() {
   InputMethodEngine* engine = GetActiveEngine(
       Profile::FromBrowserContext(browser_context()), extension_id());
   if (!engine) {
     return RespondNow(
         OneArgument(base::MakeUnique<base::FundamentalValue>(false)));
   }
 
   std::unique_ptr<SetCursorPosition::Params> parent_params(
       SetCursorPosition::Params::Create(*args_));
   const SetCursorPosition::Params::Parameters& params =
       parent_params->parameters;
 
-  return RespondNow(OneArgument(
-      base::MakeUnique<base::FundamentalValue>(engine->SetCursorPosition(
-          params.context_id, params.candidate_id, &error_))));
+  std::string error;
+  bool success =
+      engine->SetCursorPosition(params.context_id, params.candidate_id, &error);
+  SetError(error);
+  return RespondNow(
+      OneArgument(base::MakeUnique<base::FundamentalValue>(success)));
 }
 
 ExtensionFunction::ResponseAction InputImeSetMenuItemsFunction::Run() {
   std::unique_ptr<SetMenuItems::Params> parent_params(
       SetMenuItems::Params::Create(*args_));
   const SetMenuItems::Params::Parameters& params =
       parent_params->parameters;
 
   InputImeEventRouter* event_router =
       GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
@@ skipping 46 matching lines @@
       parent_params->parameters;
 
   InputImeEventRouter* event_router =
       GetInputImeEventRouter(Profile::FromBrowserContext(browser_context()));
   InputMethodEngine* engine =
       event_router ? event_router->GetEngine(extension_id(), params.engine_id)
                    : nullptr;
   if (!engine)
     return RespondNow(Error(kErrorEngineNotAvailable));
 
+  std::string error;
   engine->DeleteSurroundingText(params.context_id, params.offset, params.length,
-                                &error_);
+                                &error);
+  SetError(error);
   return RespondNow(NoArguments());
 }
 
 ExtensionFunction::ResponseAction
 InputMethodPrivateNotifyImeMenuItemActivatedFunction::Run() {
   chromeos::input_method::InputMethodDescriptor current_input_method =
       chromeos::input_method::InputMethodManager::Get()
           ->GetActiveIMEState()
           ->GetCurrentInputMethod();
   std::string active_extension_id =
@@ skipping 42 matching lines @@
     return;
   InputMethodEngine* engine =
       GetActiveEngine(Profile::FromBrowserContext(details.browser_context),
                       details.extension_id);
   // Notifies the IME extension for IME ready with onActivate/onFocus events.
   if (engine)
     engine->Enable(engine->GetActiveComponentId());
 }
 
 }  // namespace extensions