Make ExtensionFunction::ArgumentList (PKA MultipleArguments) take a scoped_ptr

rather than a raw pointer, because it plays better with the JSON schema compiler
generated code. Also add a TwoArguments method, and rename SingleArgument to
OneArgument to be consistent with that.

BUG=365732
R=rockot@chromium.org

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

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

apps/shell/browser/api/shell/shell_api.cc

@@ -67,7 +67,7 @@ ExtensionFunction::ResponseAction ShellCreateWindowFunction::Run() {
   app_window->LoadURL(url);
 
   // Create the reply to send to the renderer.
-  return RespondNow(SingleArgument(CreateResult(app_window)));
+  return RespondNow(OneArgument(CreateResult(app_window)));
 }
 
 }  // namespace apps

chrome/browser/extensions/api/automation_internal/automation_internal_api.cc

@@ -76,10 +76,9 @@ AutomationInternalEnableCurrentTabFunction::Run() {
     return RespondNow(Error("Could not enable accessibility for active tab"));
   AutomationWebContentsObserver::CreateForWebContents(contents);
   rwh->EnableTreeOnlyAccessibilityMode();
-  scoped_ptr<base::ListValue> results =
-      api::automation_internal::EnableCurrentTab::Results::Create(
-          rwh->GetProcess()->GetID(), rwh->GetRoutingID());
-  return RespondNow(MultipleArguments(results.release()));
+  return RespondNow(
+      ArgumentList(api::automation_internal::EnableCurrentTab::Results::Create(
+          rwh->GetProcess()->GetID(), rwh->GetRoutingID())));
 }
 
 ExtensionFunction::ResponseAction

chrome/browser/extensions/api/identity/identity_api.cc

@@ -275,7 +275,7 @@ ExtensionFunction::ResponseAction IdentityGetAccountsFunction::Run() {
     infos->Append(account_info.ToValue().release());
   }
 
-  return RespondNow(SingleArgument(infos));
+  return RespondNow(OneArgument(infos));
 }
 
 IdentityGetAuthTokenFunction::IdentityGetAuthTokenFunction()

chrome/browser/extensions/chrome_extension_function.cc

@@ -131,8 +131,7 @@ ChromeSyncExtensionFunction::ChromeSyncExtensionFunction() {
 ChromeSyncExtensionFunction::~ChromeSyncExtensionFunction() {}
 
 ExtensionFunction::ResponseAction ChromeSyncExtensionFunction::Run() {
-  return RespondNow(RunSync() ? MultipleArguments(results_.get())
-                              : Error(error_));
+  return RespondNow(RunSync() ? ArgumentList(results_.Pass()) : Error(error_));
 }
 
 // static

extensions/browser/api/runtime/runtime_api.cc

@@ -460,14 +460,12 @@ ExtensionFunction::ResponseAction RuntimeRequestUpdateCheckFunction::Run() {
 void RuntimeRequestUpdateCheckFunction::CheckComplete(
     const RuntimeAPIDelegate::UpdateCheckResult& result) {
   if (result.success) {
-    base::ListValue* results = new base::ListValue;
-    results->AppendString(result.response);
     base::DictionaryValue* details = new base::DictionaryValue;
-    results->Append(details);
     details->SetString("version", result.version);
-    Respond(MultipleArguments(results));
+    Respond(TwoArguments(new base::StringValue(result.response), details));
   } else {
-    Respond(SingleArgument(new base::StringValue(result.response)));
+    // HMM(kalman): Why does !success not imply Error()?
+    Respond(OneArgument(new base::StringValue(result.response)));
   }
 }
 
@@ -489,8 +487,8 @@ ExtensionFunction::ResponseAction RuntimeGetPlatformInfoFunction::Run() {
            ->GetPlatformInfo(&info)) {
     return RespondNow(Error(kPlatformInfoUnavailable));
   }
-  return RespondNow(MultipleArguments(
-      runtime::GetPlatformInfo::Results::Create(info).release()));
+  return RespondNow(
+      ArgumentList(runtime::GetPlatformInfo::Results::Create(info)));
 }
 
 ExtensionFunction::ResponseAction
@@ -511,7 +509,7 @@ RuntimeGetPackageDirectoryEntryFunction::Run() {
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->SetString("fileSystemId", filesystem_id);
   dict->SetString("baseName", relative_path);
-  return RespondNow(SingleArgument(dict));
+  return RespondNow(OneArgument(dict));
 }
 
 }  // namespace extensions

extensions/browser/api/storage/storage_api.cc

@@ -78,7 +78,7 @@ ExtensionFunction::ResponseValue SettingsFunction::UseReadResult(
 
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->Swap(&result->settings());
-  return SingleArgument(dict);
+  return OneArgument(dict);
 }
 
 ExtensionFunction::ResponseValue SettingsFunction::UseWriteResult(
@@ -249,7 +249,7 @@ StorageStorageAreaGetBytesInUseFunction::RunWithStorage(ValueStore* storage) {
       return BadMessage();
   }
 
-  return SingleArgument(
+  return OneArgument(
       new base::FundamentalValue(static_cast<int>(bytes_in_use)));
 }
 

extensions/browser/extension_function.cc

@@ -24,30 +24,30 @@ using extensions::Feature;
 
 namespace {
 
-class MultipleArgumentsResponseValue
+class ArgumentListResponseValue
     : public ExtensionFunction::ResponseValueObject {
  public:
-  MultipleArgumentsResponseValue(const std::string& function_name,
-                                 const char* title,
-                                 ExtensionFunction* function,
-                                 base::ListValue* result)
+  ArgumentListResponseValue(const std::string& function_name,
+                            const char* title,
+                            ExtensionFunction* function,
+                            scoped_ptr<base::ListValue> result)
       : function_name_(function_name), title_(title) {
     if (function->GetResultList()) {
-      DCHECK_EQ(function->GetResultList(), result)
+      DCHECK_EQ(function->GetResultList(), result.get())
           << "The result set on this function (" << function_name_ << ") "
           << "either by calling SetResult() or directly modifying |result_| is "
           << "different to the one passed to " << title_ << "(). "
           << "The best way to fix this problem is to exclusively use " << title_
           << "(). SetResult() and |result_| are deprecated.";
     } else {
-      function->SetResultList(make_scoped_ptr(result));
+      function->SetResultList(result.Pass());
     }
     // It would be nice to DCHECK(error.empty()) but some legacy extension
     // function implementations... I'm looking at chrome.input.ime... do this
     // for some reason.
   }
 
-  virtual ~MultipleArgumentsResponseValue() {}
+  virtual ~ArgumentListResponseValue() {}
 
   virtual bool Apply() OVERRIDE { return true; }
 
@@ -222,22 +222,32 @@ void ExtensionFunction::SetError(const std::string& error) {
 }
 
 ExtensionFunction::ResponseValue ExtensionFunction::NoArguments() {
-  return ResponseValue(new MultipleArgumentsResponseValue(
-      name(), "NoArguments", this, new base::ListValue()));
+  return ResponseValue(new ArgumentListResponseValue(
+      name(), "NoArguments", this, make_scoped_ptr(new base::ListValue())));
 }
 
-ExtensionFunction::ResponseValue ExtensionFunction::SingleArgument(
+ExtensionFunction::ResponseValue ExtensionFunction::OneArgument(
     base::Value* arg) {
-  base::ListValue* args = new base::ListValue();
+  scoped_ptr<base::ListValue> args(new base::ListValue());
   args->Append(arg);
   return ResponseValue(
-      new MultipleArgumentsResponseValue(name(), "SingleArgument", this, args));
+      new ArgumentListResponseValue(name(), "OneArgument", this, args.Pass()));
 }
 
-ExtensionFunction::ResponseValue ExtensionFunction::MultipleArguments(
-    base::ListValue* args) {
-  return ResponseValue(new MultipleArgumentsResponseValue(
-      name(), "MultipleArguments", this, args));
+ExtensionFunction::ResponseValue ExtensionFunction::TwoArguments(
+    base::Value* arg1,
+    base::Value* arg2) {
+  scoped_ptr<base::ListValue> args(new base::ListValue());
+  args->Append(arg1);
+  args->Append(arg2);
+  return ResponseValue(
+      new ArgumentListResponseValue(name(), "TwoArguments", this, args.Pass()));
+}
+
+ExtensionFunction::ResponseValue ExtensionFunction::ArgumentList(
+    scoped_ptr<base::ListValue> args) {
+  return ResponseValue(
+      new ArgumentListResponseValue(name(), "ArgumentList", this, args.Pass()));
 }
 
 ExtensionFunction::ResponseValue ExtensionFunction::Error(
@@ -407,8 +417,7 @@ SyncExtensionFunction::~SyncExtensionFunction() {
 }
 
 ExtensionFunction::ResponseAction SyncExtensionFunction::Run() {
-  return RespondNow(RunSync() ? MultipleArguments(results_.get())
-                              : Error(error_));
+  return RespondNow(RunSync() ? ArgumentList(results_.Pass()) : Error(error_));
 }
 
 // static
@@ -423,8 +432,7 @@ SyncIOThreadExtensionFunction::~SyncIOThreadExtensionFunction() {
 }
 
 ExtensionFunction::ResponseAction SyncIOThreadExtensionFunction::Run() {
-  return RespondNow(RunSync() ? MultipleArguments(results_.get())
-                              : Error(error_));
+  return RespondNow(RunSync() ? ArgumentList(results_.Pass()) : Error(error_));
 }
 
 // static

extensions/browser/extension_function.h

@@ -115,7 +115,7 @@ class ExtensionFunction
 
   // The result of a function call.
   //
-  // Use NoArguments(), SingleArgument(), MultipleArguments(), or Error()
+  // Use NoArguments(), OneArgument(), ArgumentList(), or Error()
   // rather than this class directly.
   class ResponseValueObject {
    public:
@@ -142,8 +142,8 @@ class ExtensionFunction
   //
   // Typical return values might be:
   //   * RespondNow(NoArguments())
-  //   * RespondNow(SingleArgument(42))
-  //   * RespondNow(MultipleArguments(my_result.ToValue()))
+  //   * RespondNow(OneArgument(42))
+  //   * RespondNow(ArgumentList(my_result.ToValue()))
   //   * RespondNow(Error("Warp core breach"))
   //   * RespondLater(), then later,
   //     * Respond(NoArguments())
@@ -248,10 +248,20 @@ class ExtensionFunction
   //
   // Success, no arguments to pass to caller
   ResponseValue NoArguments();
-  // Success, a single argument |result| to pass to caller. TAKES OWNERSHIP.
-  ResponseValue SingleArgument(base::Value* result);
-  // Success, a list of arguments |results| to pass to caller. TAKES OWNERSHIP.
-  ResponseValue MultipleArguments(base::ListValue* results);
+  // Success, a single argument |arg| to pass to caller. TAKES OWNERSHIP -- a
+  // raw pointer for convenience, since callers usually construct the argument
+  // to this by hand.
+  ResponseValue OneArgument(base::Value* arg);
+  // Success, two arguments |arg1| and |arg2| to pass to caller. TAKES
+  // OWNERSHIP -- raw pointers for convenience, since callers usually construct
+  // the argument to this by hand. Note that use of this function may imply you
+  // should be using the generated Result struct and ArgumentList.
+  ResponseValue TwoArguments(base::Value* arg1, base::Value* arg2);
+  // Success, a list of arguments |results| to pass to caller. TAKES OWNERSHIP
+  // --
+  // a scoped_ptr<> for convenience, since callers usually get this from the
+  // result of a ToValue() call on the generated Result struct.
+  ResponseValue ArgumentList(scoped_ptr<base::ListValue> results);
   // Error. chrome.runtime.lastError.message will be set to |error|.
   ResponseValue Error(const std::string& error);
   // Bad message. A ResponseValue equivalent to EXTENSION_FUNCTION_VALIDATE().