Make sure that extension callbacks are called only once

customCallback implementations must now explicitly call the (optional) callback.

BUG=446035
R=kalman@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#317778}

5 files changed, 49 insertions, 41 deletions

diff --git a/extensions/renderer/resources/app_window_custom_bindings.js b/extensions/renderer/resources/app_window_custom_bindings.js
index 6cd762d..99c1a3d 100644
--- a/extensions/renderer/resources/app_window_custom_bindings.js
+++ b/extensions/renderer/resources/app_window_custom_bindings.js
@@ -114,7 +114,7 @@ appWindow.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
 
   apiFunctions.setCustomCallback('create',
-                                 function(name, request, windowParams) {
+      function(name, request, callback, windowParams) {
     var view = null;
 
     // When window creation fails, |windowParams| will be undefined.
@@ -126,29 +126,23 @@ appWindow.registerCustomHook(function(bindingsAPI) {
     if (!view) {
       // No route to created window. If given a callback, trigger it with an
       // undefined object.
-      if (request.callback) {
-        request.callback();
-        delete request.callback;
-      }
+      if (callback)
+        callback();
       return;
     }
 
     if (windowParams.existingWindow) {
       // Not creating a new window, but activating an existing one, so trigger
       // callback with existing window and don't do anything else.
-      if (request.callback) {
-        request.callback(view.chrome.app.window.current());
-        delete request.callback;
-      }
+      if (callback)
+        callback(view.chrome.app.window.current());
       return;
     }
 
     // Initialize appWindowData in the newly created JS context
     view.chrome.app.window.initializeAppWindow(windowParams);
 
-    var callback = request.callback;
     if (callback) {
-      delete request.callback;
       if (!view) {
         callback(undefined);
         return;
diff --git a/extensions/renderer/resources/context_menus_custom_bindings.js b/extensions/renderer/resources/context_menus_custom_bindings.js
index 71a97a4..0e82711b5 100644
--- a/extensions/renderer/resources/context_menus_custom_bindings.js
+++ b/extensions/renderer/resources/context_menus_custom_bindings.js
@@ -9,6 +9,7 @@ var binding = require('binding').Binding.create('contextMenus');
 var contextMenuNatives = requireNative('context_menus');
 var sendRequest = require('sendRequest').sendRequest;
 var Event = require('event_bindings').Event;
+var lastError = require('lastError');
 
 binding.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
@@ -55,8 +56,11 @@ binding.registerCustomHook(function(bindingsAPI) {
     return contextMenus.getIdFromCreateProperties(args[0]);
   });
 
-  apiFunctions.setCustomCallback('create', function(name, request, response) {
-    if (chrome.runtime.lastError) {
+  apiFunctions.setCustomCallback('create',
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
 
@@ -68,33 +72,49 @@ binding.registerCustomHook(function(bindingsAPI) {
       contextMenus.ensureListenerSetup();
       contextMenus.handlersForId(id)[id] = onclick;
     }
+    if (callback)
+      callback();
   });
 
-  apiFunctions.setCustomCallback('remove', function(name, request, response) {
-    if (chrome.runtime.lastError) {
+  apiFunctions.setCustomCallback('remove',
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     var id = request.args[0];
     delete contextMenus.handlersForId(id)[id];
+    if (callback)
+      callback();
   });
 
-  apiFunctions.setCustomCallback('update', function(name, request, response) {
-    if (chrome.runtime.lastError) {
+  apiFunctions.setCustomCallback('update',
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     var id = request.args[0];
     if (request.args[1].onclick) {
       contextMenus.handlersForId(id)[id] = request.args[1].onclick;
     }
+    if (callback)
+      callback();
   });
 
   apiFunctions.setCustomCallback('removeAll',
-                                 function(name, request, response) {
-    if (chrome.runtime.lastError) {
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     contextMenus.generatedIdHandlers = {};
     contextMenus.stringIdHandlers = {};
+    if (callback)
+      callback();
   });
 });
 
diff --git a/extensions/renderer/resources/permissions_custom_bindings.js b/extensions/renderer/resources/permissions_custom_bindings.js
index 60edfaa..43b436e 100644
--- a/extensions/renderer/resources/permissions_custom_bindings.js
+++ b/extensions/renderer/resources/permissions_custom_bindings.js
@@ -62,7 +62,7 @@ binding.registerCustomHook(function(api) {
 
   // Convert complex permissions back to objects
   apiFunctions.setCustomCallback('getAll',
-      function(name, request, response) {
+      function(name, request, callback, response) {
         for (var i = 0; i < response.permissions.length; i += 1) {
           response.permissions[i] =
               maybeConvertToObject(response.permissions[i]);
@@ -70,14 +70,9 @@ binding.registerCustomHook(function(api) {
 
         // Since the schema says Permissions.permissions contains strings and
         // not objects, validation will fail after the for-loop above.  This
-        // skips validation and calls the callback directly, then clears it so
-        // that handleResponse doesn't call it again.
-        try {
-          if (request.callback)
-            $Function.apply(request.callback, request, [response]);
-        } finally {
-          delete request.callback;
-        }
+        // skips validation and calls the callback directly.
+        if (callback)
+          callback(response);
       });
 
   // Also convert complex permissions back to objects for events.  The
diff --git a/extensions/renderer/resources/runtime_custom_bindings.js b/extensions/renderer/resources/runtime_custom_bindings.js
index 2f0cb49..60c3bd0 100644
--- a/extensions/renderer/resources/runtime_custom_bindings.js
+++ b/extensions/renderer/resources/runtime_custom_bindings.js
@@ -41,11 +41,12 @@ if (window == backgroundPage) {
   var GetIsolatedFileSystem = fileSystemNatives.GetIsolatedFileSystem;
   var bindDirectoryEntryCallback = function(functionName, apiFunctions) {
     apiFunctions.setCustomCallback(functionName,
-        function(name, request, response) {
-      if (request.callback && response) {
-        var callback = request.callback;
-        request.callback = null;
-
+        function(name, request, callback, response) {
+      if (callback) {
+        if (!response) {
+          callback();
+          return;
+        }
         var fileSystemId = response.fileSystemId;
         var baseName = response.baseName;
         var fs = GetIsolatedFileSystem(fileSystemId);
@@ -191,12 +192,11 @@ binding.registerCustomHook(function(binding, id, contextType) {
   });
 
   apiFunctions.setCustomCallback('getBackgroundPage',
-                                 function(name, request, response) {
-    if (request.callback) {
+                                 function(name, request, callback, response) {
+    if (callback) {
       var bg = runtimeNatives.GetExtensionViews(-1, 'BACKGROUND')[0] || null;
-      request.callback(bg);
+      callback(bg);
     }
-    request.callback = null;
   });
 
   bindDirectoryEntryCallback('getPackageDirectoryEntry', apiFunctions);
diff --git a/extensions/renderer/resources/send_request.js b/extensions/renderer/resources/send_request.js
index bdf0324..e042b01 100644
--- a/extensions/renderer/resources/send_request.js
+++ b/extensions/renderer/resources/send_request.js
@@ -57,10 +57,9 @@ function handleResponse(requestId, name, success, responseList, error) {
       safeCallbackApply(name,
                         request,
                         request.customCallback,
-                        $Array.concat([name, request], responseList));
-    }
-
-    if (request.callback) {
+                        $Array.concat([name, request, request.callback],
+                                      responseList));
+    } else if (request.callback) {
       // Validate callback in debug only -- and only when the
       // caller has provided a callback. Implementations of api
       // calls may not return data if they observe the caller