Introduce ExtensionFunction::RunImplTypesafe and SendResponseTypesafe,

new type-safe ways of responding to extension requests.

Use them in the Storage API as a proof of concept.

R=rockot@chromium.org
BUG=365732, 366282

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

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

6 files changed, 271 insertions, 71 deletions

diff --git a/extensions/browser/api/storage/storage_api.cc b/extensions/browser/api/storage/storage_api.cc
index c8f3fb4..3f1882d 100644
--- a/extensions/browser/api/storage/storage_api.cc
+++ b/extensions/browser/api/storage/storage_api.cc
@@ -32,27 +32,28 @@ bool SettingsFunction::ShouldSkipQuotaLimiting() const {
   std::string settings_namespace_string;
   if (!args_->GetString(0, &settings_namespace_string)) {
     // This should be EXTENSION_FUNCTION_VALIDATE(false) but there is no way
-    // to signify that from this function. It will be caught in RunImpl().
+    // to signify that from this function. It will be caught in
+    // RunImplTypesafe().
     return false;
   }
   return settings_namespace_string != "sync";
 }
 
-bool SettingsFunction::RunImpl() {
+ExtensionFunction::ResponseAction SettingsFunction::RunImplTypesafe() {
   std::string settings_namespace_string;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &settings_namespace_string));
+  EXTENSION_FUNCTION_VALIDATE_TYPESAFE(
+      args_->GetString(0, &settings_namespace_string));
   args_->Remove(0, NULL);
   settings_namespace_ =
       settings_namespace::FromString(settings_namespace_string);
-  EXTENSION_FUNCTION_VALIDATE(
-      settings_namespace_ != settings_namespace::INVALID);
+  EXTENSION_FUNCTION_VALIDATE_TYPESAFE(settings_namespace_ !=
+                                       settings_namespace::INVALID);
 
   StorageFrontend* frontend = StorageFrontend::Get(browser_context());
   if (!frontend->IsStorageEnabled(settings_namespace_)) {
-    error_ = base::StringPrintf(
-        "\"%s\" is not available in this instance of Chrome",
-        settings_namespace_string.c_str());
-    return false;
+    return RespondNow(Error(
+        base::StringPrintf("\"%s\" is not available in this instance of Chrome",
+                           settings_namespace_string.c_str())));
   }
 
   observers_ = frontend->GetObservers();
@@ -60,30 +61,32 @@ bool SettingsFunction::RunImpl() {
       GetExtension(),
       settings_namespace_,
       base::Bind(&SettingsFunction::AsyncRunWithStorage, this));
-  return true;
+  return RespondLater();
 }
 
 void SettingsFunction::AsyncRunWithStorage(ValueStore* storage) {
-  bool success = RunWithStorage(storage);
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&SettingsFunction::SendResponse, this, success));
+  ResponseValue response = RunWithStorage(storage);
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          base::Bind(&SettingsFunction::SendResponseTypesafe,
+                                     this,
+                                     base::Passed(&response)));
 }
 
-bool SettingsFunction::UseReadResult(ValueStore::ReadResult result,
-                                     ValueStore* storage) {
+ExtensionFunction::ResponseValue SettingsFunction::UseReadResult(
+    ValueStore::ReadResult result,
+    ValueStore* storage) {
   if (result->HasError())
     return HandleError(result->error(), storage);
 
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->Swap(&result->settings());
-  SetResult(dict);
-  return true;
+  return SingleArgument(dict);
 }
 
-bool SettingsFunction::UseWriteResult(ValueStore::WriteResult result,
-                                      ValueStore* storage) {
+ExtensionFunction::ResponseValue SettingsFunction::UseWriteResult(
+    ValueStore::WriteResult result,
+    ValueStore* storage) {
   if (result->HasError())
     return HandleError(result->error(), storage);
 
@@ -95,11 +98,12 @@ bool SettingsFunction::UseWriteResult(ValueStore::WriteResult result,
         ValueStoreChange::ToJson(result->changes()));
   }
 
-  return true;
+  return NoArguments();
 }
 
-bool SettingsFunction::HandleError(const ValueStore::Error& error,
-                                   ValueStore* storage) {
+ExtensionFunction::ResponseValue SettingsFunction::HandleError(
+    const ValueStore::Error& error,
+    ValueStore* storage) {
   // If the method failed due to corruption, and we haven't tried to fix it, we
   // can try to restore the storage and re-run it. Otherwise, the method has
   // failed.
@@ -117,8 +121,7 @@ bool SettingsFunction::HandleError(const ValueStore::Error& error,
       return RunWithStorage(storage);
   }
 
-  error_ = error.message;
-  return false;
+  return Error(error.message);
 }
 
 // Concrete settings functions
@@ -174,9 +177,11 @@ void GetModificationQuotaLimitHeuristics(QuotaLimitHeuristics* heuristics) {
 
 }  // namespace
 
-bool StorageStorageAreaGetFunction::RunWithStorage(ValueStore* storage) {
+ExtensionFunction::ResponseValue StorageStorageAreaGetFunction::RunWithStorage(
+    ValueStore* storage) {
   base::Value* input = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &input));
+  if (!args_->Get(0, &input))
+    return BadMessage();
 
   switch (input->GetType()) {
     case base::Value::TYPE_NULL:
@@ -211,15 +216,15 @@ bool StorageStorageAreaGetFunction::RunWithStorage(ValueStore* storage) {
     }
 
     default:
-      EXTENSION_FUNCTION_VALIDATE(false);
-      return false;
+      return BadMessage();
   }
 }
 
-bool StorageStorageAreaGetBytesInUseFunction::RunWithStorage(
-    ValueStore* storage) {
+ExtensionFunction::ResponseValue
+StorageStorageAreaGetBytesInUseFunction::RunWithStorage(ValueStore* storage) {
   base::Value* input = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &input));
+  if (!args_->Get(0, &input))
+    return BadMessage();
 
   size_t bytes_in_use = 0;
 
@@ -244,17 +249,18 @@ bool StorageStorageAreaGetBytesInUseFunction::RunWithStorage(
     }
 
     default:
-      EXTENSION_FUNCTION_VALIDATE(false);
-      return false;
+      return BadMessage();
   }
 
-  SetResult(new base::FundamentalValue(static_cast<int>(bytes_in_use)));
-  return true;
+  return SingleArgument(
+      new base::FundamentalValue(static_cast<int>(bytes_in_use)));
 }
 
-bool StorageStorageAreaSetFunction::RunWithStorage(ValueStore* storage) {
+ExtensionFunction::ResponseValue StorageStorageAreaSetFunction::RunWithStorage(
+    ValueStore* storage) {
   base::DictionaryValue* input = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &input));
+  if (!args_->GetDictionary(0, &input))
+    return BadMessage();
   return UseWriteResult(storage->Set(ValueStore::DEFAULTS, *input), storage);
 }
 
@@ -263,9 +269,11 @@ void StorageStorageAreaSetFunction::GetQuotaLimitHeuristics(
   GetModificationQuotaLimitHeuristics(heuristics);
 }
 
-bool StorageStorageAreaRemoveFunction::RunWithStorage(ValueStore* storage) {
+ExtensionFunction::ResponseValue
+StorageStorageAreaRemoveFunction::RunWithStorage(ValueStore* storage) {
   base::Value* input = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &input));
+  if (!args_->Get(0, &input))
+    return BadMessage();
 
   switch (input->GetType()) {
     case base::Value::TYPE_STRING: {
@@ -282,8 +290,7 @@ bool StorageStorageAreaRemoveFunction::RunWithStorage(ValueStore* storage) {
     }
 
     default:
-      EXTENSION_FUNCTION_VALIDATE(false);
-      return false;
+      return BadMessage();
   };
 }
 
@@ -292,7 +299,8 @@ void StorageStorageAreaRemoveFunction::GetQuotaLimitHeuristics(
   GetModificationQuotaLimitHeuristics(heuristics);
 }
 
-bool StorageStorageAreaClearFunction::RunWithStorage(ValueStore* storage) {
+ExtensionFunction::ResponseValue
+StorageStorageAreaClearFunction::RunWithStorage(ValueStore* storage) {
   return UseWriteResult(storage->Clear(), storage);
 }
 
diff --git a/extensions/browser/api/storage/storage_api.h b/extensions/browser/api/storage/storage_api.h
index ae3369e..9f1091c 100644
--- a/extensions/browser/api/storage/storage_api.h
+++ b/extensions/browser/api/storage/storage_api.h
@@ -22,13 +22,11 @@ class SettingsFunction : public UIThreadExtensionFunction {
 
   // ExtensionFunction:
   virtual bool ShouldSkipQuotaLimiting() const OVERRIDE;
-  virtual bool RunImpl() OVERRIDE;
+  virtual ResponseAction RunImplTypesafe() OVERRIDE;
 
   // Extension settings function implementations should do their work here.
   // The StorageFrontend makes sure this is posted to the appropriate thread.
-  // Implementations should fill in args themselves, though (like RunImpl)
-  // may return false to imply failure.
-  virtual bool RunWithStorage(ValueStore* storage) = 0;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) = 0;
 
   // Handles the |result| of a read function.
   // - If the result succeeded, this will set |result_| and return.
@@ -36,7 +34,8 @@ class SettingsFunction : public UIThreadExtensionFunction {
   //   RestoreStorageAndRetry(), and return that result.
   // - If the |result| failed with a different error, this will set |error_|
   //   and return.
-  bool UseReadResult(ValueStore::ReadResult result, ValueStore* storage);
+  ResponseValue UseReadResult(ValueStore::ReadResult result,
+                              ValueStore* storage);
 
   // Handles the |result| of a write function.
   // - If the result succeeded, this will set |result_| and return.
@@ -45,10 +44,11 @@ class SettingsFunction : public UIThreadExtensionFunction {
   // - If the |result| failed with a different error, this will set |error_|
   //   and return.
   // This will also send out a change notification, if appropriate.
-  bool UseWriteResult(ValueStore::WriteResult result, ValueStore* storage);
+  ResponseValue UseWriteResult(ValueStore::WriteResult result,
+                               ValueStore* storage);
 
  private:
-  // Called via PostTask from RunImpl.  Calls RunWithStorage and then
+  // Called via PostTask from RunImplTypesafe. Calls RunWithStorage and then
   // SendResponse with its success value.
   void AsyncRunWithStorage(ValueStore* storage);
 
@@ -57,7 +57,8 @@ class SettingsFunction : public UIThreadExtensionFunction {
   // If the storage cannot be restored or was due to some other error, then sets
   // error and returns. This also sets the |tried_restoring_storage_| flag to
   // ensure we don't enter a loop.
-  bool HandleError(const ValueStore::Error& error, ValueStore* storage);
+  ResponseValue HandleError(const ValueStore::Error& error,
+                            ValueStore* storage);
 
   // The settings namespace the call was for.  For example, SYNC if the API
   // call was chrome.settings.experimental.sync..., LOCAL if .local, etc.
@@ -79,7 +80,7 @@ class StorageStorageAreaGetFunction : public SettingsFunction {
   virtual ~StorageStorageAreaGetFunction() {}
 
   // SettingsFunction:
-  virtual bool RunWithStorage(ValueStore* storage) OVERRIDE;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) OVERRIDE;
 };
 
 class StorageStorageAreaSetFunction : public SettingsFunction {
@@ -90,7 +91,7 @@ class StorageStorageAreaSetFunction : public SettingsFunction {
   virtual ~StorageStorageAreaSetFunction() {}
 
   // SettingsFunction:
-  virtual bool RunWithStorage(ValueStore* storage) OVERRIDE;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) OVERRIDE;
 
   // ExtensionFunction:
   virtual void GetQuotaLimitHeuristics(
@@ -105,7 +106,7 @@ class StorageStorageAreaRemoveFunction : public SettingsFunction {
   virtual ~StorageStorageAreaRemoveFunction() {}
 
   // SettingsFunction:
-  virtual bool RunWithStorage(ValueStore* storage) OVERRIDE;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) OVERRIDE;
 
   // ExtensionFunction:
   virtual void GetQuotaLimitHeuristics(
@@ -120,7 +121,7 @@ class StorageStorageAreaClearFunction : public SettingsFunction {
   virtual ~StorageStorageAreaClearFunction() {}
 
   // SettingsFunction:
-  virtual bool RunWithStorage(ValueStore* storage) OVERRIDE;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) OVERRIDE;
 
   // ExtensionFunction:
   virtual void GetQuotaLimitHeuristics(
@@ -135,7 +136,7 @@ class StorageStorageAreaGetBytesInUseFunction : public SettingsFunction {
   virtual ~StorageStorageAreaGetBytesInUseFunction() {}
 
   // SettingsFunction:
-  virtual bool RunWithStorage(ValueStore* storage) OVERRIDE;
+  virtual ResponseValue RunWithStorage(ValueStore* storage) OVERRIDE;
 };
 
 }  // namespace extensions
diff --git a/extensions/browser/api/storage/storage_api_unittest.cc b/extensions/browser/api/storage/storage_api_unittest.cc
index caa17a3..a108b5d 100644
--- a/extensions/browser/api/storage/storage_api_unittest.cc
+++ b/extensions/browser/api/storage/storage_api_unittest.cc
@@ -64,6 +64,8 @@ class StorageApiUnittest : public ExtensionApiUnittest {
     scoped_ptr<base::Value> result = RunFunctionAndReturnValue(
         new StorageStorageAreaGetFunction(),
         base::StringPrintf("[\"local\", \"%s\"]", key.c_str()));
+    if (!result.get())
+      return testing::AssertionFailure() << "No result";
     base::DictionaryValue* dict = NULL;
     if (!result->GetAsDictionary(&dict))
       return testing::AssertionFailure() << result << " was not a dictionary.";
diff --git a/extensions/browser/extension_function.cc b/extensions/browser/extension_function.cc
index ed33a8f..accc12d 100644
--- a/extensions/browser/extension_function.cc
+++ b/extensions/browser/extension_function.cc
@@ -22,6 +22,74 @@ using content::WebContents;
 using extensions::ExtensionAPI;
 using extensions::Feature;
 
+namespace {
+
+class MultipleArgumentsResponseValue
+    : public ExtensionFunction::ResponseValueObject {
+ public:
+  MultipleArgumentsResponseValue(ExtensionFunction* function,
+                                 base::ListValue* result) {
+    if (function->GetResultList()) {
+      DCHECK_EQ(function->GetResultList(), result);
+    } else {
+      function->SetResultList(make_scoped_ptr(result));
+    }
+    DCHECK_EQ("", function->GetError());
+  }
+
+  virtual ~MultipleArgumentsResponseValue() {}
+
+  virtual bool Apply() OVERRIDE { return true; }
+};
+
+class ErrorResponseValue : public ExtensionFunction::ResponseValueObject {
+ public:
+  ErrorResponseValue(ExtensionFunction* function, const std::string& error) {
+    DCHECK_NE("", error);
+    function->SetError(error);
+  }
+
+  virtual ~ErrorResponseValue() {}
+
+  virtual bool Apply() OVERRIDE { return false; }
+};
+
+class BadMessageResponseValue : public ExtensionFunction::ResponseValueObject {
+ public:
+  explicit BadMessageResponseValue(ExtensionFunction* function) {
+    function->set_bad_message(true);
+    NOTREACHED() << function->name() << ": bad message";
+  }
+
+  virtual ~BadMessageResponseValue() {}
+
+  virtual bool Apply() OVERRIDE { return false; }
+};
+
+class RespondNowAction : public ExtensionFunction::ResponseActionObject {
+ public:
+  typedef base::Callback<void(bool)> SendResponseCallback;
+  RespondNowAction(ExtensionFunction::ResponseValue result,
+                   const SendResponseCallback& send_response)
+      : result_(result.Pass()), send_response_(send_response) {}
+  virtual ~RespondNowAction() {}
+
+  virtual void Execute() OVERRIDE { send_response_.Run(result_->Apply()); }
+
+ private:
+  ExtensionFunction::ResponseValue result_;
+  SendResponseCallback send_response_;
+};
+
+class RespondLaterAction : public ExtensionFunction::ResponseActionObject {
+ public:
+  virtual ~RespondLaterAction() {}
+
+  virtual void Execute() OVERRIDE {}
+};
+
+}  // namespace
+
 // static
 void ExtensionFunctionDeleteTraits::Destruct(const ExtensionFunction* x) {
   x->Destruct();
@@ -112,11 +180,15 @@ void ExtensionFunction::SetResult(base::Value* result) {
   results_->Append(result);
 }
 
-const base::ListValue* ExtensionFunction::GetResultList() {
+void ExtensionFunction::SetResultList(scoped_ptr<base::ListValue> results) {
+  results_ = results.Pass();
+}
+
+const base::ListValue* ExtensionFunction::GetResultList() const {
   return results_.get();
 }
 
-const std::string ExtensionFunction::GetError() {
+std::string ExtensionFunction::GetError() const {
   return error_;
 }
 
@@ -124,11 +196,62 @@ void ExtensionFunction::SetError(const std::string& error) {
   error_ = error;
 }
 
+ExtensionFunction::ResponseValue ExtensionFunction::NoArguments() {
+  return MultipleArguments(new base::ListValue());
+}
+
+ExtensionFunction::ResponseValue ExtensionFunction::SingleArgument(
+    base::Value* arg) {
+  base::ListValue* args = new base::ListValue();
+  args->Append(arg);
+  return MultipleArguments(args);
+}
+
+ExtensionFunction::ResponseValue ExtensionFunction::MultipleArguments(
+    base::ListValue* args) {
+  return scoped_ptr<ResponseValueObject>(
+      new MultipleArgumentsResponseValue(this, args));
+}
+
+ExtensionFunction::ResponseValue ExtensionFunction::Error(
+    const std::string& error) {
+  return scoped_ptr<ResponseValueObject>(new ErrorResponseValue(this, error));
+}
+
+ExtensionFunction::ResponseValue ExtensionFunction::BadMessage() {
+  return scoped_ptr<ResponseValueObject>(new BadMessageResponseValue(this));
+}
+
+ExtensionFunction::ResponseAction ExtensionFunction::RespondNow(
+    ResponseValue result) {
+  return scoped_ptr<ResponseActionObject>(new RespondNowAction(
+      result.Pass(), base::Bind(&ExtensionFunction::SendResponse, this)));
+}
+
+ExtensionFunction::ResponseAction ExtensionFunction::RespondLater() {
+  return scoped_ptr<ResponseActionObject>(new RespondLaterAction());
+}
+
 void ExtensionFunction::Run() {
   if (!RunImpl())
     SendResponse(false);
 }
 
+bool ExtensionFunction::RunImpl() {
+  RunImplTypesafe()->Execute();
+  return true;
+}
+
+ExtensionFunction::ResponseAction ExtensionFunction::RunImplTypesafe() {
+  NOTREACHED()
+      << "ExtensionFunctions must override either RunImpl or RunImplTypesafe";
+  return RespondNow(NoArguments());
+}
+
+void ExtensionFunction::SendResponseTypesafe(ResponseValue response) {
+  SendResponse(response->Apply());
+}
+
 bool ExtensionFunction::ShouldSkipQuotaLimiting() const {
   return false;
 }
diff --git a/extensions/browser/extension_function.h b/extensions/browser/extension_function.h
index 6090857..ee58cba 100644
--- a/extensions/browser/extension_function.h
+++ b/extensions/browser/extension_function.h
@@ -44,15 +44,22 @@ class ExtensionMessageFilter;
 class QuotaLimitHeuristic;
 }
 
+#define EXTENSION_FUNCTION_VALIDATE(test) \
+  EXTENSION_FUNCTION_VALIDATE_INTERNAL(test, false)
+
+#define EXTENSION_FUNCTION_VALIDATE_TYPESAFE(test) \
+  EXTENSION_FUNCTION_VALIDATE_INTERNAL(test, RespondNow(BadMessage()))
+
 #ifdef NDEBUG
-#define EXTENSION_FUNCTION_VALIDATE(test) do { \
-    if (!(test)) { \
-      bad_message_ = true; \
-      return false; \
-    } \
+#define EXTENSION_FUNCTION_VALIDATE_INTERNAL(test, failure) \
+  do {                                                      \
+    if (!(test)) {                                          \
+      bad_message_ = true;                                  \
+      return (failure);                                     \
+    }                                                       \
   } while (0)
 #else   // NDEBUG
-#define EXTENSION_FUNCTION_VALIDATE(test) CHECK(test)
+#define EXTENSION_FUNCTION_VALIDATE_INTERNAL(test, failure) CHECK(test)
 #endif  // NDEBUG
 
 #define EXTENSION_FUNCTION_ERROR(error) do { \
@@ -145,15 +152,21 @@ class ExtensionFunction
   // Sets a single Value as the results of the function.
   void SetResult(base::Value* result);
 
+  // Sets multiple Values as the results of the function.
+  void SetResultList(scoped_ptr<base::ListValue> results);
+
   // Retrieves the results of the function as a ListValue.
-  const base::ListValue* GetResultList();
+  const base::ListValue* GetResultList() const;
 
   // Retrieves any error string from the function.
-  virtual const std::string GetError();
+  virtual std::string GetError() const;
 
   // Sets the function's error string.
   virtual void SetError(const std::string& error);
 
+  // Sets the function's bad message state.
+  void set_bad_message(bool bad_message) { bad_message_ = bad_message; }
+
   // Specifies the name of the function.
   void set_name(const std::string& name) { name_ = name; }
   const std::string& name() const { return name_; }
@@ -195,19 +208,72 @@ class ExtensionFunction
   void set_source_tab_id(int source_tab_id) { source_tab_id_ = source_tab_id; }
   int source_tab_id() const { return source_tab_id_; }
 
+  // The result of a function call.
+  //
+  // Use NoArguments(), SingleArgument(), MultipleArguments(), or Error()
+  // rather than this class directly.
+  class ResponseValueObject {
+   public:
+    virtual ~ResponseValueObject() {}
+
+    // Returns true for success, false for failure.
+    virtual bool Apply() = 0;
+  };
+  typedef scoped_ptr<ResponseValueObject> ResponseValue;
+
+  // The action to use when returning from RunImpl.
+  //
+  // Use RespondNow() or RespondLater() rather than this class directly.
+  class ResponseActionObject {
+   public:
+    virtual ~ResponseActionObject() {}
+
+    virtual void Execute() = 0;
+  };
+  typedef scoped_ptr<ResponseActionObject> ResponseAction;
+
  protected:
   friend struct ExtensionFunctionDeleteTraits;
 
+  // ResponseValues.
+  //
+  // 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);
+  // 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().
+  ResponseValue BadMessage();
+
+  // ResponseActions.
+  //
+  // Respond to the extension immediately with |result|.
+  ResponseAction RespondNow(ResponseValue result);
+  // Don't respond now, but promise to call SendResponse later.
+  ResponseAction RespondLater();
+
   virtual ~ExtensionFunction();
 
   // Helper method for ExtensionFunctionDeleteTraits. Deletes this object.
   virtual void Destruct() const = 0;
 
-  // Derived classes should implement this method to do their work and return
-  // success/failure.
-  virtual bool RunImpl() = 0;
+  // Derived classes should implement one of these methods to do their work.
+  //
+  // Returns the action to take. DO NOT USE WITH SyncExtensionFunction.
+  virtual ResponseAction RunImplTypesafe();
+  // Deprecated. Returns true on success. SendResponse() must be called later.
+  // Return false to indicate an error and respond immediately.
+  virtual bool RunImpl();
 
   // Sends the result back to the extension.
+  //
+  // Responds with |response|.
+  void SendResponseTypesafe(ResponseValue response);
+  // Deprecated. Call with true to indicate success, false to indicate failure,
+  // in which case please set |error_|.
   virtual void SendResponse(bool success) = 0;
 
   // Common implementation for SendResponse.
diff --git a/extensions/browser/quota_service_unittest.cc b/extensions/browser/quota_service_unittest.cc
index 04798c2..57b2534 100644
--- a/extensions/browser/quota_service_unittest.cc
+++ b/extensions/browser/quota_service_unittest.cc
@@ -64,7 +64,7 @@ class MockFunction : public ExtensionFunction {
   explicit MockFunction(const std::string& name) { set_name(name); }
 
   virtual void SetArgs(const base::ListValue* args) OVERRIDE {}
-  virtual const std::string GetError() OVERRIDE { return std::string(); }
+  virtual std::string GetError() const OVERRIDE { return std::string(); }
   virtual void SetError(const std::string& error) OVERRIDE {}
   virtual void Run() OVERRIDE {}
   virtual void Destruct() const OVERRIDE { delete this; }