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}

13 files changed, 126 insertions, 109 deletions

diff --git a/chrome/renderer/resources/extensions/browser_action_custom_bindings.js b/chrome/renderer/resources/extensions/browser_action_custom_bindings.js
index 5704305..cc22bef 100644
--- a/chrome/renderer/resources/extensions/browser_action_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/browser_action_custom_bindings.js
@@ -9,6 +9,7 @@ var binding = require('binding').Binding.create('browserAction');
 var setIcon = require('setIcon').setIcon;
 var getExtensionViews = requireNative('runtime').GetExtensionViews;
 var sendRequest = require('sendRequest').sendRequest;
+var lastError = require('lastError');
 
 binding.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
@@ -20,17 +21,16 @@ binding.registerCustomHook(function(bindingsAPI) {
   });
 
   apiFunctions.setCustomCallback('openPopup',
-                                 function(name, request, response) {
-    if (!request.callback)
+      function(name, request, callback, response) {
+    if (!callback)
       return;
 
-    if (chrome.runtime.lastError) {
-      request.callback();
+    if (lastError.hasError(chrome)) {
+      callback();
     } else {
       var views = getExtensionViews(-1, 'POPUP');
-      request.callback(views.length > 0 ? views[0] : null);
+      callback(views.length > 0 ? views[0] : null);
     }
-    request.callback = null;
   });
 });
 
diff --git a/chrome/renderer/resources/extensions/chrome_web_view_internal_custom_bindings.js b/chrome/renderer/resources/extensions/chrome_web_view_internal_custom_bindings.js
index c55d644..9346c11 100644
--- a/chrome/renderer/resources/extensions/chrome_web_view_internal_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/chrome_web_view_internal_custom_bindings.js
@@ -14,6 +14,7 @@ var EventBindings = require('event_bindings');
 var binding = require('binding').Binding.create('chromeWebViewInternal');
 var contextMenuNatives = requireNative('context_menus');
 var sendRequest = require('sendRequest').sendRequest;
+var lastError = require('lastError');
 
 binding.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
@@ -78,8 +79,10 @@ binding.registerCustomHook(function(bindingsAPI) {
   });
 
   apiFunctions.setCustomCallback('contextMenusCreate',
-                                 function(name, request, response) {
-    if (chrome.runtime.lastError) {
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
 
@@ -90,11 +93,15 @@ binding.registerCustomHook(function(bindingsAPI) {
       webviewContextMenus.ensureListenerSetup();
       webviewContextMenus.handlersForId(instanceId, id)[id] = onclick;
     }
+    if (callback)
+      callback();
   });
 
   apiFunctions.setCustomCallback('contextMenusUpdate',
-                                 function(name, request, response) {
-    if (chrome.runtime.lastError) {
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     var instanceId = request.args[0];
@@ -103,26 +110,36 @@ binding.registerCustomHook(function(bindingsAPI) {
       webviewContextMenus.handlersForId(instanceId, id)[id] =
           request.args[2].onclick;
     }
+    if (callback)
+      callback();
   });
 
   apiFunctions.setCustomCallback('contextMenusRemove',
-                                  function(name, request, response) {
-    if (chrome.runtime.lastError) {
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     var instanceId = request.args[0];
     var id = request.args[1];
     delete webviewContextMenus.handlersForId(instanceId, id)[id];
+    if (callback)
+      callback();
   });
 
   apiFunctions.setCustomCallback('contextMenusRemoveAll',
-                                 function(name, request, response) {
-    if (chrome.runtime.lastError) {
+      function(name, request, callback, response) {
+    if (lastError.hasError(chrome)) {
+      if (callback)
+        callback();
       return;
     }
     var instanceId = request.args[0];
     webviewContextMenus.stringIdHandlers[instanceId] = {};
     webviewContextMenus.generatedIdHandlers[instanceId] = {};
+    if (callback)
+      callback();
   });
 
 });
diff --git a/chrome/renderer/resources/extensions/file_entry_binding_util.js b/chrome/renderer/resources/extensions/file_entry_binding_util.js
index 3e4216b..1789726 100644
--- a/chrome/renderer/resources/extensions/file_entry_binding_util.js
+++ b/chrome/renderer/resources/extensions/file_entry_binding_util.js
@@ -28,10 +28,12 @@ function getFileBindingsForApi(apiName) {
   if (window == backgroundPage) {
     var bindFileEntryCallback = 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 entries = [];
           var hasError = false;
diff --git a/chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js b/chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js
index 9d374ae..7347e23 100644
--- a/chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js
@@ -24,34 +24,32 @@ binding.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
 
   apiFunctions.setCustomCallback('requestFileSystem',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var fs = null;
     if (response && !response.error)
       fs = GetFileSystem(response.name, response.root_url);
-    if (request.callback)
-      request.callback(fs);
-    request.callback = null;
+    if (callback)
+      callback(fs);
   });
 
   apiFunctions.setCustomCallback('searchDrive',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     if (response && !response.error && response.entries) {
       response.entries = response.entries.map(function(entry) {
         return GetExternalFileEntry(entry);
       });
     }
 
-    // So |request.callback| doesn't break if response is not defined.
+    // So |callback| doesn't break if response is not defined.
     if (!response)
       response = {};
 
-    if (request.callback)
-      request.callback(response.entries, response.nextFeed);
-    request.callback = null;
+    if (callback)
+      callback(response.entries, response.nextFeed);
   });
 
   apiFunctions.setCustomCallback('searchDriveMetadata',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     if (response && !response.error) {
       for (var i = 0; i < response.length; i++) {
         response[i].entry =
@@ -59,13 +57,12 @@ binding.registerCustomHook(function(bindingsAPI) {
       }
     }
 
-    // So |request.callback| doesn't break if response is not defined.
+    // So |callback| doesn't break if response is not defined.
     if (!response)
       response = {};
 
-    if (request.callback)
-      request.callback(response);
-    request.callback = null;
+    if (callback)
+      callback(response);
   });
 
   apiFunctions.setHandleRequest('resolveIsolatedEntries',
diff --git a/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js b/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
index bbe740a..7c52a38 100644
--- a/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
@@ -34,23 +34,21 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
   // custom callback so that they can instantiate and return an array of file
   // system objects.
   apiFunctions.setCustomCallback('getMediaFileSystems',
-                                 function(name, request, response) {
+                                 function(name, request, callback, response) {
     var result = createFileSystemObjectsAndUpdateMetadata(response);
-    if (request.callback)
-      request.callback(result);
-    request.callback = null;
+    if (callback)
+      callback(result);
   });
 
   apiFunctions.setCustomCallback('addScanResults',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var result = createFileSystemObjectsAndUpdateMetadata(response);
-    if (request.callback)
-      request.callback(result);
-    request.callback = null;
+    if (callback)
+      callback(result);
   });
 
   apiFunctions.setCustomCallback('addUserSelectedFolder',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var fileSystems = [];
     var selectedFileSystemName = "";
     if (response && 'mediaFileSystems' in response &&
@@ -62,13 +60,12 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
         selectedFileSystemName = fileSystems[selectedFileSystemIndex].name;
       }
     }
-    if (request.callback)
-      request.callback(fileSystems, selectedFileSystemName);
-    request.callback = null;
+    if (callback)
+      callback(fileSystems, selectedFileSystemName);
   });
 
   apiFunctions.setCustomCallback('dropPermissionForMediaFileSystem',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var galleryId = response;
 
     if (galleryId) {
@@ -79,9 +76,8 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
         }
       }
     }
-    if (request.callback)
-      request.callback();
-    request.callback = null;
+    if (callback)
+      callback();
   });
 
   apiFunctions.setHandleRequest('getMediaFileSystemMetadata',
@@ -110,7 +106,7 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
   });
 
   apiFunctions.setCustomCallback('getMetadata',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     if (response.attachedImagesBlobInfo) {
       for (var i = 0; i < response.attachedImagesBlobInfo.length; i++) {
         var blobInfo = response.attachedImagesBlobInfo[i];
@@ -120,9 +116,8 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
       }
     }
 
-    if (request.callback)
-      request.callback(response.metadata);
-    request.callback = null;
+    if (callback)
+      callback(response.metadata);
 
     // The UUID was in position 0 in the setUpdateArgumentsPostValidate
     // function.
diff --git a/chrome/renderer/resources/extensions/page_capture_custom_bindings.js b/chrome/renderer/resources/extensions/page_capture_custom_bindings.js
index 1848297..fa1a544 100644
--- a/chrome/renderer/resources/extensions/page_capture_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/page_capture_custom_bindings.js
@@ -15,9 +15,7 @@ binding.registerCustomHook(function(bindingsAPI) {
   var apiFunctions = bindingsAPI.apiFunctions;
 
   apiFunctions.setCustomCallback('saveAsMHTML',
-                                 function(name, request, response) {
-    var callback = request.callback;
-    request.callback = null;
+      function(name, request, callback, response) {
     if (response)
       response = CreateBlob(response.mhtmlFilePath, response.mhtmlFileLength);
 
diff --git a/chrome/renderer/resources/extensions/sync_file_system_custom_bindings.js b/chrome/renderer/resources/extensions/sync_file_system_custom_bindings.js
index 352b0a7..21d4d47 100644
--- a/chrome/renderer/resources/extensions/sync_file_system_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/sync_file_system_custom_bindings.js
@@ -48,21 +48,20 @@ binding.registerCustomHook(function(bindingsAPI) {
 
   // Functions which return an [instanceOf=DOMFileSystem].
   apiFunctions.setCustomCallback('requestFileSystem',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var result = null;
     if (response) {
       result = syncFileSystemNatives.GetSyncFileSystemObject(
           response.name, response.root);
     }
-    if (request.callback)
-      request.callback(result);
-    request.callback = null;
+    if (callback)
+      callback(result);
   });
 
   // Functions which return an array of FileStatusInfo object
   // which has [instanceOf=FileEntry].
   apiFunctions.setCustomCallback('getFileStatuses',
-                                 function(name, request, response) {
+      function(name, request, callback, response) {
     var results = [];
     if (response) {
       for (var i = 0; i < response.length; i++) {
@@ -79,9 +78,8 @@ binding.registerCustomHook(function(bindingsAPI) {
         $Array.push(results, result);
       }
     }
-    if (request.callback)
-      request.callback(results);
-    request.callback = null;
+    if (callback)
+      callback(results);
   });
 });
 
diff --git a/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js b/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
index 0e285d2..93a6054 100644
--- a/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
@@ -9,9 +9,12 @@ var binding = require('binding').Binding.create('tabCapture');
 binding.registerCustomHook(function(bindingsAPI, extensionId) {
   var apiFunctions = bindingsAPI.apiFunctions;
 
-  apiFunctions.setCustomCallback('capture', function(name, request, response) {
-    if (response && request.callback) {
-      var callback = request.callback;
+  apiFunctions.setCustomCallback('capture',
+      function(name, request, callback, response) {
+    if (!callback)
+      return;
+
+    if (response) {
       var options = {};
       if (response.audioConstraints)
         options.audio = response.audioConstraints;
@@ -26,9 +29,8 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
         callback(null);
       }
     } else {
-      request.callback(null);
+      callback(null);
     }
-    request.callback = null;
   });
 });
 
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