Fix DCHECK in ~ExtensionFunction().

If permission is denied to call an extension function, the function
is currently destroyed without responding.  This causes a DCHECK
failure in its destructor.

Instead, ask the function to respond with an error in this case, which
makes did_respond() return true, which avoids the DCHECK.

This showed up during some trybot runs of an unrelated change, so this
fixes some test flakiness.

Bug: none
Change-Id: I6cc5481157161b00a493504b6a403c0e492bef09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116537
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#752655}

extensions/browser/extension_function.cc

@@ -407,6 +407,10 @@ bool ExtensionFunction::HasPermission() const {
   return availability.is_available();
 }
 
+void ExtensionFunction::RespondWithError(const std::string& error) {
+  Respond(Error(error));
+}
+
 bool ExtensionFunction::PreRunValidation(std::string* error) {
   // TODO(crbug.com/625646) This is a partial fix to avoid crashes when certain
   // extension functions run during shutdown. Browser or Notification creation
@@ -443,8 +447,7 @@ bool ExtensionFunction::ShouldSkipQuotaLimiting() const {
 }
 
 void ExtensionFunction::OnQuotaExceeded(const std::string& violation_error) {
-  error_ = violation_error;
-  SendResponseImpl(false);
+  RespondWithError(violation_error);
 }
 
 void ExtensionFunction::SetArgs(base::Value args) {

extensions/browser/extension_function.h

@@ -112,6 +112,9 @@ class ExtensionFunction : public base::RefCountedThreadSafe<
   // checks in Run(), such as for specific host permissions or user gestures.
   bool HasPermission() const;
 
+  // Sends |error| as an error response.
+  void RespondWithError(const std::string& error);
+
   // The result of a function call.
   //
   // Use NoArguments(), OneArgument(), ArgumentList(), or Error()

extensions/browser/extension_function_dispatcher.cc

@@ -51,6 +51,8 @@ using content::BrowserThread;
 namespace extensions {
 namespace {
 
+constexpr char kCreationFailed[] = "Access to extension API denied.";
+
 // Notifies the ApiActivityMonitor that an extension API function has been
 // called. May be called from any thread.
 void NotifyApiFunctionCalled(const std::string& extension_id,
@@ -460,7 +462,7 @@ bool ExtensionFunctionDispatcher::CheckPermissions(
     const ExtensionFunction::ResponseCallback& callback) {
   if (!function->HasPermission()) {
     LOG(ERROR) << "Permission denied for " << params.name;
-    SendAccessDenied(callback);
+    function->RespondWithError(kCreationFailed);
     return false;
   }
   return true;
@@ -481,7 +483,7 @@ ExtensionFunctionDispatcher::CreateExtensionFunction(
       ExtensionFunctionRegistry::GetInstance().NewFunction(params.name);
   if (!function) {
     LOG(ERROR) << "Unknown Extension API - " << params.name;
-    SendAccessDenied(callback);
+    callback.Run(ExtensionFunction::FAILED, base::ListValue(), kCreationFailed);
     return nullptr;
   }
 
@@ -499,13 +501,4 @@ ExtensionFunctionDispatcher::CreateExtensionFunction(
 
   return function;
 }
-
-// static
-void ExtensionFunctionDispatcher::SendAccessDenied(
-    const ExtensionFunction::ResponseCallback& callback) {
-  base::ListValue empty_list;
-  callback.Run(ExtensionFunction::FAILED, empty_list,
-               "Access to extension API denied.");
-}
-
 }  // namespace extensions

extensions/browser/extension_function_dispatcher.h

@@ -135,11 +135,6 @@ class ExtensionFunctionDispatcher
       void* profile_id,
       const ExtensionFunction::ResponseCallback& callback);
 
-  // Helper to run the response callback with an access denied error. Can be
-  // called on any thread.
-  static void SendAccessDenied(
-      const ExtensionFunction::ResponseCallback& callback);
-
   void DispatchWithCallbackInternal(
       const ExtensionHostMsg_Request_Params& params,
       content::RenderFrameHost* render_frame_host,