[Extensions] Isolate ExtensionFunction results_ and error_

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

Index: chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc
diff --git a/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc b/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc
index a1d5b2f94d5a67e6c4a7ece74168fdb36ecf2479..6497c532aea3d00d778d82c7b554beb3737f0327 100644
--- a/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc
+++ b/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc
@@ -370,8 +370,11 @@ ExtensionFunction::ResponseAction InputImeClearCompositionFunction::Run() {
   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() {
@@ -404,8 +407,10 @@ InputImeSetCandidateWindowPropertiesFunction::Run() {
   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)));
   }
@@ -484,8 +489,12 @@ ExtensionFunction::ResponseAction InputImeSetCandidatesFunction::Run() {
     }
   }
 
-  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() {
@@ -501,9 +510,12 @@ ExtensionFunction::ResponseAction InputImeSetCursorPositionFunction::Run() {
   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() {
@@ -570,8 +582,10 @@ ExtensionFunction::ResponseAction InputImeDeleteSurroundingTextFunction::Run() {
   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());
 }