Bug 1234020: Part 2j - [webext] Return promises from the tabs API. r=rpl draft
authorKris Maglione <maglione.k@gmail.com>
Mon, 01 Feb 2016 20:25:35 -0800
changeset 328982 d550750d04b7fe517d03232745925fa10ee50eba
parent 328981 1c959d09485300c54717cb1e846c62a53d4af4fb
child 328983 1c167b8bb915f45ac1ced1eef69c2708e3ee2fd5
push id10445
push usermaglione.k@gmail.com
push dateThu, 04 Feb 2016 21:38:16 +0000
reviewersrpl
bugs1234020
milestone47.0a1
Bug 1234020: Part 2j - [webext] Return promises from the tabs API. r=rpl
browser/components/extensions/ext-tabs.js
browser/components/extensions/schemas/tabs.json
browser/components/extensions/test/browser/browser_ext_tabs_captureVisibleTab.js
browser/components/extensions/test/browser/browser_ext_tabs_create.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -12,17 +12,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
   EventManager,
   ignoreEvent,
-  runSafe,
 } = ExtensionUtils;
 
 // This function is pretty tightly tied to Extension.jsm.
 // Its job is to fill in the |tab| property of the sender.
 function getSender(context, target, sender) {
   // The message was sent from a content script to a <browser> element.
   // We can just get the |tab| from |target|.
   if (target instanceof Ci.nsIDOMXULElement) {
@@ -263,77 +262,75 @@ extensions.registerSchemaAPI("tabs", nul
         WindowListManager.addCloseListener(windowListener);
         AllWindowEvents.addListener("TabClose", tabListener);
         return () => {
           WindowListManager.removeCloseListener(windowListener);
           AllWindowEvents.removeListener("TabClose", tabListener);
         };
       }).api(),
 
-      create: function(createProperties, callback) {
+      create: function(createProperties) {
         let url = createProperties.url || aboutNewTabService.newTabURL;
         url = context.uri.resolve(url);
 
-        function createInWindow(window) {
-          let tab = window.gBrowser.addTab(url);
+        return new Promise(resolve => {
+          function createInWindow(window) {
+            let tab = window.gBrowser.addTab(url);
 
-          let active = true;
-          if (createProperties.active !== null) {
-            active = createProperties.active;
-          }
-          if (active) {
-            window.gBrowser.selectedTab = tab;
-          }
+            let active = true;
+            if (createProperties.active !== null) {
+              active = createProperties.active;
+            }
+            if (active) {
+              window.gBrowser.selectedTab = tab;
+            }
 
-          if (createProperties.index !== null) {
-            window.gBrowser.moveTabTo(tab, createProperties.index);
-          }
+            if (createProperties.index !== null) {
+              window.gBrowser.moveTabTo(tab, createProperties.index);
+            }
 
-          if (createProperties.pinned) {
-            window.gBrowser.pinTab(tab);
+            if (createProperties.pinned) {
+              window.gBrowser.pinTab(tab);
+            }
+
+            resolve(TabManager.convert(extension, tab));
           }
 
-          if (callback) {
-            runSafe(context, callback, TabManager.convert(extension, tab));
+          let window = createProperties.windowId !== null ?
+            WindowManager.getWindow(createProperties.windowId) :
+            WindowManager.topWindow;
+          if (!window.gBrowser) {
+            let obs = (finishedWindow, topic, data) => {
+              if (finishedWindow != window) {
+                return;
+              }
+              Services.obs.removeObserver(obs, "browser-delayed-startup-finished");
+              createInWindow(window);
+            };
+            Services.obs.addObserver(obs, "browser-delayed-startup-finished", false);
+          } else {
+            createInWindow(window);
           }
-        }
-
-        let window = createProperties.windowId !== null ?
-          WindowManager.getWindow(createProperties.windowId) :
-          WindowManager.topWindow;
-        if (!window.gBrowser) {
-          let obs = (finishedWindow, topic, data) => {
-            if (finishedWindow != window) {
-              return;
-            }
-            Services.obs.removeObserver(obs, "browser-delayed-startup-finished");
-            createInWindow(window);
-          };
-          Services.obs.addObserver(obs, "browser-delayed-startup-finished", false);
-        } else {
-          createInWindow(window);
-        }
+        });
       },
 
-      remove: function(tabs, callback) {
+      remove: function(tabs) {
         if (!Array.isArray(tabs)) {
           tabs = [tabs];
         }
 
         for (let tabId of tabs) {
           let tab = TabManager.getTab(tabId);
           tab.ownerDocument.defaultView.gBrowser.removeTab(tab);
         }
 
-        if (callback) {
-          runSafe(context, callback);
-        }
+        return Promise.resolve();
       },
 
-      update: function(tabId, updateProperties, callback) {
+      update: function(tabId, updateProperties) {
         let tab = tabId !== null ? TabManager.getTab(tabId) : TabManager.activeTab;
         let tabbrowser = tab.ownerDocument.defaultView.gBrowser;
         if (updateProperties.url !== null) {
           tab.linkedBrowser.loadURI(updateProperties.url);
         }
         if (updateProperties.active !== null) {
           if (updateProperties.active) {
             tabbrowser.selectedTab = tab;
@@ -350,56 +347,52 @@ extensions.registerSchemaAPI("tabs", nul
           if (updateProperties.pinned) {
             tabbrowser.pinTab(tab);
           } else {
             tabbrowser.unpinTab(tab);
           }
         }
         // FIXME: highlighted/selected, openerTabId
 
-        if (callback) {
-          runSafe(context, callback, TabManager.convert(extension, tab));
-        }
+        return Promise.resolve(TabManager.convert(extension, tab));
       },
 
-      reload: function(tabId, reloadProperties, callback) {
+      reload: function(tabId, reloadProperties) {
         let tab = tabId !== null ? TabManager.getTab(tabId) : TabManager.activeTab;
         let flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
         if (reloadProperties && reloadProperties.bypassCache) {
           flags |= Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE;
         }
         tab.linkedBrowser.reloadWithFlags(flags);
 
-        if (callback) {
-          runSafe(context, callback);
-        }
+        return Promise.resolve();
       },
 
-      get: function(tabId, callback) {
+      get: function(tabId) {
         let tab = TabManager.getTab(tabId);
-        runSafe(context, callback, TabManager.convert(extension, tab));
+        return Promise.resolve(TabManager.convert(extension, tab));
       },
 
-      getCurrent(callback) {
+      getCurrent() {
         let tab;
         if (context.tabId) {
           tab = TabManager.convert(extension, TabManager.getTab(context.tabId));
         }
-        runSafe(context, callback, tab);
+        return Promise.resolve(tab);
       },
 
-      getAllInWindow: function(windowId, callback) {
+      getAllInWindow: function(windowId) {
         if (windowId === null) {
           windowId = WindowManager.topWindow.windowId;
         }
 
-        return self.tabs.query({windowId}, callback);
+        return self.tabs.query({windowId});
       },
 
-      query: function(queryInfo, callback) {
+      query: function(queryInfo) {
         let pattern = null;
         if (queryInfo.url !== null) {
           pattern = new MatchPattern(queryInfo.url);
         }
 
         function matches(window, tab) {
           let props = ["active", "pinned", "highlighted", "status", "title", "index"];
           for (let prop of props) {
@@ -458,17 +451,17 @@ extensions.registerSchemaAPI("tabs", nul
         for (let window of WindowListManager.browserWindows()) {
           let tabs = TabManager.for(extension).getTabs(window);
           for (let tab of tabs) {
             if (matches(window, tab)) {
               result.push(tab);
             }
           }
         }
-        runSafe(context, callback, result);
+        return Promise.resolve(result);
       },
 
       captureVisibleTab: function(windowId, options) {
         if (!extension.hasPermission("<all_urls>")) {
           throw new context.contentWindow.Error("The <all_urls> permission is required to use the captureVisibleTab API");
         }
 
         let window = windowId == null ?
@@ -577,17 +570,17 @@ extensions.registerSchemaAPI("tabs", nul
 
         let recipient = {extensionId: extension.id};
         if (options && options.frameId !== null) {
           recipient.frameId = options.frameId;
         }
         return context.messenger.sendMessage(mm, message, recipient, responseCallback);
       },
 
-      move: function(tabIds, moveProperties, callback) {
+      move: function(tabIds, moveProperties) {
         let index = moveProperties.index;
         let tabsMoved = [];
         if (!Array.isArray(tabIds)) {
           tabIds = [tabIds];
         }
 
         let destinationWindow = null;
         if (moveProperties.windowId !== null) {
@@ -649,16 +642,14 @@ extensions.registerSchemaAPI("tabs", nul
             gBrowser.swapBrowsersAndCloseOther(newTab, tab);
           } else {
             // If the window we are moving is the same, just move the tab.
             gBrowser.moveTabTo(tab, getInsertionPoint());
           }
           tabsMoved.push(tab);
         }
 
-        if (callback) {
-          runSafe(context, callback, tabsMoved.map(tab => TabManager.convert(extension, tab)));
-        }
+        return Promise.resolve(tabsMoved.map(tab => TabManager.convert(extension, tab)));
       },
     },
   };
   return self;
 });
--- a/browser/components/extensions/schemas/tabs.json
+++ b/browser/components/extensions/schemas/tabs.json
@@ -151,16 +151,17 @@
         "description": "An ID which represents the absence of a browser tab."
       }
     },
     "functions": [
       {
         "name": "get",
         "type": "function",
         "description": "Retrieves details about the specified tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0
           },
           {
             "type": "function",
@@ -170,16 +171,17 @@
             ]
           }
         ]
       },
       {
         "name": "getCurrent",
         "type": "function",
         "description": "Gets the tab that this script call is being made from. May be undefined if called from a non-tab context (for example: a background page or popup view).",
+        "async": "callback",
         "parameters": [
           {
             "type": "function",
             "name": "callback",
             "parameters": [
               {
                 "name": "tab",
                 "$ref": "Tab",
@@ -291,16 +293,17 @@
         ]
       },
       {
         "name": "getSelected",
         "deprecated": "Please use $(ref:tabs.query) <code>{active: true}</code>.",
         "unsupported": true,
         "type": "function",
         "description": "Gets the tab that is selected in the specified window.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "windowId",
             "minimum": -2,
             "optional": true,
             "description": "Defaults to the $(topic:current-window)[current window]."
           },
@@ -313,16 +316,17 @@
           }
         ]
       },
       {
         "name": "getAllInWindow",
         "type": "function",
         "deprecated": "Please use $(ref:tabs.query) <code>{windowId: windowId}</code>.",
         "description": "Gets details about all tabs in the specified window.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "windowId",
             "minimum": -2,
             "optional": true,
             "description": "Defaults to the $(topic:current-window)[current window]."
             },
@@ -334,16 +338,17 @@
             ]
           }
         ]
       },
       {
         "name": "create",
         "type": "function",
         "description": "Creates a new tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "object",
             "name": "createProperties",
             "properties": {
               "windowId": {
                 "type": "integer",
                 "minimum": -2,
@@ -400,16 +405,17 @@
             ]
           }
         ]
       },
       {
         "name": "duplicate",
         "type": "function",
         "description": "Duplicates a tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0,
             "description": "The ID of the tab which is to be duplicated."
           },
           {
@@ -426,16 +432,17 @@
             ]
           }
         ]
       },
       {
         "name": "query",
         "type": "function",
         "description": "Gets all tabs that have the specified properties, or all tabs if no properties are specified.",
+        "async": "callback",
         "parameters": [
           {
             "type": "object",
             "name": "queryInfo",
             "properties": {
               "active": {
                 "type": "boolean",
                 "optional": true,
@@ -522,16 +529,17 @@
             ]
           }
         ]
       },
       {
         "name": "highlight",
         "type": "function",
         "description": "Highlights the given tabs.",
+        "async": "callback",
         "parameters": [
           {
             "type": "object",
             "name": "highlightInfo",
             "properties": {
                "windowId": {
                  "type": "integer",
                  "optional": true,
@@ -560,16 +568,17 @@
              ]
            }
         ]
       },
       {
         "name": "update",
         "type": "function",
         "description": "Modifies the properties of a tab. Properties that are not specified in <var>updateProperties</var> are not modified.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0,
             "optional": true,
             "description": "Defaults to the selected tab of the $(topic:current-window)[current window]."
           },
@@ -633,16 +642,17 @@
             ]
           }
         ]
       },
       {
         "name": "move",
         "type": "function",
         "description": "Moves one or more tabs to a new position within its window, or to a new window. Note that tabs can only be moved to and from normal (window.type === \"normal\") windows.",
+        "async": "callback",
         "parameters": [
           {
             "name": "tabIds",
             "description": "The tab or list of tabs to move.",
             "choices": [
               {"type": "integer", "minimum": 0},
               {"type": "array", "items": {"type": "integer", "minimum": 0}}
             ]
@@ -680,53 +690,72 @@
             ]
           }
         ]
       },
       {
         "name": "reload",
         "type": "function",
         "description": "Reload a tab.",
+        "async": "callback",
         "parameters": [
-          {"type": "integer", "name": "tabId", "minimum": 0, "optional": true, "description": "The ID of the tab to reload; defaults to the selected tab of the current window."},
+          {
+            "type": "integer",
+            "name": "tabId",
+            "minimum": 0,
+            "optional": true,
+            "description": "The ID of the tab to reload; defaults to the selected tab of the current window."
+          },
           {
             "type": "object",
             "name": "reloadProperties",
             "optional": true,
             "properties": {
               "bypassCache": {
                 "type": "boolean",
                 "optional": true,
                 "description": "Whether using any local cache. Default is false."
               }
             }
           },
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": []
+          }
         ]
       },
       {
         "name": "remove",
         "type": "function",
         "description": "Closes one or more tabs.",
+        "async": "callback",
         "parameters": [
           {
             "name": "tabIds",
             "description": "The tab or list of tabs to close.",
             "choices": [
               {"type": "integer", "minimum": 0},
               {"type": "array", "items": {"type": "integer", "minimum": 0}}
             ]
           },
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": []
+          }
         ]
       },
       {
         "name": "detectLanguage",
         "type": "function",
         "description": "Detects the primary language of the content in a tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0,
             "optional": true,
             "description": "Defaults to the active tab of the $(topic:current-window)[current window]."
           },
@@ -757,29 +786,41 @@
             "description": "The target window. Defaults to the $(topic:current-window)[current window]."
           },
           {
             "$ref": "extensionTypes.ImageDetails",
             "name": "options",
             "optional": true
           },
           {
-            "type": "function", "name": "callback", "parameters": [
-              {"type": "string", "name": "dataUrl", "description": "A data URL which encodes an image of the visible area of the captured tab. May be assigned to the 'src' property of an HTML Image element for display."}
+            "type": "function",
+            "name": "callback",
+            "parameters": [
+              {
+                "type": "string",
+                "name": "dataUrl",
+                "description": "A data URL which encodes an image of the visible area of the captured tab. May be assigned to the 'src' property of an HTML Image element for display."
+              }
             ]
           }
         ]
       },
       {
         "name": "executeScript",
         "type": "function",
         "description": "Injects JavaScript code into a page. For details, see the $(topic:content_scripts)[programmatic injection] section of the content scripts doc.",
         "async": "callback",
         "parameters": [
-          {"type": "integer", "name": "tabId", "minimum": 0, "optional": true, "description": "The ID of the tab in which to run the script; defaults to the active tab of the current window."},
+          {
+            "type": "integer",
+            "name": "tabId",
+            "minimum": 0,
+            "optional": true,
+            "description": "The ID of the tab in which to run the script; defaults to the active tab of the current window."
+          },
           {
             "$ref": "extensionTypes.InjectDetails",
             "name": "details",
             "description": "Details of the script to run."
           },
           {
             "type": "function",
             "name": "callback",
@@ -798,17 +839,23 @@
         ]
       },
       {
         "name": "insertCSS",
         "type": "function",
         "description": "Injects CSS into a page. For details, see the $(topic:content_scripts)[programmatic injection] section of the content scripts doc.",
         "async": "callback",
         "parameters": [
-          {"type": "integer", "name": "tabId", "minimum": 0, "optional": true, "description": "The ID of the tab in which to insert the CSS; defaults to the active tab of the current window."},
+          {
+            "type": "integer",
+            "name": "tabId",
+            "minimum": 0,
+            "optional": true,
+            "description": "The ID of the tab in which to insert the CSS; defaults to the active tab of the current window."
+          },
           {
             "$ref": "extensionTypes.InjectDetails",
             "name": "details",
             "description": "Details of the CSS text to insert."
           },
           {
             "type": "function",
             "name": "callback",
@@ -817,16 +864,17 @@
             "parameters": []
           }
         ]
       },
       {
         "name": "setZoom",
         "type": "function",
         "description": "Zooms a specified tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0,
             "optional": true,
             "description": "The ID of the tab to zoom; defaults to the active tab of the current window."
           },
@@ -843,16 +891,17 @@
             "parameters": []
           }
         ]
       },
       {
         "name": "getZoom",
         "type": "function",
         "description": "Gets the current zoom factor of a specified tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0,
             "optional": true,
             "description": "The ID of the tab to get the current zoom factor from; defaults to the active tab of the current window."
           },
@@ -869,16 +918,17 @@
             ]
           }
         ]
       },
       {
         "name": "setZoomSettings",
         "type": "function",
         "description": "Sets the zoom settings for a specified tab, which define how zoom changes are handled. These settings are reset to defaults upon navigating the tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "optional": true,
             "minimum": 0,
             "description": "The ID of the tab to change the zoom settings for; defaults to the active tab of the current window."
           },
@@ -895,16 +945,17 @@
             "parameters": []
           }
         ]
       },
       {
         "name": "getZoomSettings",
         "type": "function",
         "description": "Gets the current zoom settings of a specified tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "optional": true,
             "minimum": 0,
             "description": "The ID of the tab to get the current zoom settings from; defaults to the active tab of the current window."
           },
--- a/browser/components/extensions/test/browser/browser_ext_tabs_captureVisibleTab.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_captureVisibleTab.js
@@ -145,25 +145,25 @@ add_task(function* testCaptureVisibleTab
 add_task(function* testCaptureVisibleTabPermissions() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],
     },
 
     background: function(x) {
       browser.tabs.query({ currentWindow: true, active: true }, tab => {
-        try {
-          browser.tabs.captureVisibleTab(tab.windowId, () => {});
-        } catch (e) {
-          if (e.message == "The <all_urls> permission is required to use the captureVisibleTab API") {
+        browser.tabs.captureVisibleTab(tab.windowId).then(
+          () => {
+            browser.test.notifyFail("captureVisibleTabPermissions");
+          },
+          (e) => {
+            browser.test.assertEq("The <all_urls> permission is required to use the captureVisibleTab API",
+                                  e.message, "Expected permissions error message");
             browser.test.notifyPass("captureVisibleTabPermissions");
-            return;
-          }
-        }
-        browser.test.notifyFail("captureVisibleTabPermissions");
+          });
       });
     },
   });
 
   yield extension.startup();
 
   yield extension.awaitFinish("captureVisibleTabPermissions");
 
--- a/browser/components/extensions/test/browser/browser_ext_tabs_create.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_create.js
@@ -26,27 +26,16 @@ add_task(function* () {
       "bg/blank.html": `<html><head><meta charset="utf-8"></head></html>`,
 
       "bg/background.html": `<html><head>
         <meta charset="utf-8">
         <script src="background.js"></script>
       </head></html>`,
 
       "bg/background.js": function() {
-        // Wrap API methods in promise-based variants.
-        let promiseTabs = {};
-        Object.keys(browser.tabs).forEach(method => {
-          promiseTabs[method] = (...args) => {
-            return new Promise(resolve => {
-              browser.tabs[method](...args, resolve);
-            });
-          };
-        });
-
-
         let activeTab;
         let activeWindow;
 
         function runTests() {
           const DEFAULTS = {
             index: 2,
             windowId: activeWindow,
             active: true,
@@ -127,17 +116,17 @@ add_task(function* () {
               let onCreated = tab => {
                 browser.test.assertTrue("id" in tab, `Expected tabs.onCreated callback to receive tab object`);
                 resolve();
               };
               browser.tabs.onCreated.addListener(onCreated);
             });
 
             Promise.all([
-              promiseTabs.create(test.create),
+              browser.tabs.create(test.create),
               createdPromise,
             ]).then(([tab]) => {
               tabId = tab.id;
 
               for (let key of Object.keys(expected)) {
                 if (key === "url") {
                   // FIXME: This doesn't get updated until later in the load cycle.
                   continue;
@@ -145,19 +134,19 @@ add_task(function* () {
 
                 browser.test.assertEq(expected[key], tab[key], `Expected value for tab.${key}`);
               }
 
               return updatedPromise;
             }).then(url => {
               browser.test.assertEq(expected.url, url, `Expected value for tab.url`);
 
-              return promiseTabs.remove(tabId);
+              return browser.tabs.remove(tabId);
             }).then(() => {
-              return promiseTabs.update(activeTab, { active: true });
+              return browser.tabs.update(activeTab, { active: true });
             }).then(() => {
               nextTest();
             });
           }
 
           nextTest();
         }