Bug 1234020: Part 2a - [webext] Return promises from the background page APIs. r?rpl draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 02 Feb 2016 19:14:34 -0800
changeset 328972 67fc3bdf0f225f0fc4192af6623fa5b0cf205ac1
parent 327814 25f761534d9b415f08789bff02a4480a9669f48d
child 328973 4ed79019ea977795987b1ab33d0b4729ec83b5d9
push id10445
push usermaglione.k@gmail.com
push dateThu, 04 Feb 2016 21:38:16 +0000
reviewersrpl
bugs1234020
milestone47.0a1
Bug 1234020: Part 2a - [webext] Return promises from the background page APIs. r?rpl
browser/components/extensions/test/browser/browser_ext_getViews.js
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/ext-backgroundPage.js
toolkit/components/extensions/schemas/runtime.json
--- a/browser/components/extensions/test/browser/browser_ext_getViews.js
+++ b/browser/components/extensions/test/browser/browser_ext_getViews.js
@@ -15,26 +15,35 @@ function genericChecker() {
   browser.test.onMessage.addListener((msg, ...args) => {
     if (msg == kind + "-check-views") {
       let views = browser.extension.getViews();
       let counts = {
         "background": 0,
         "tab": 0,
         "popup": 0,
       };
+      let background;
       for (let i = 0; i < views.length; i++) {
         let view = views[i];
         browser.test.assertTrue(view.kind in counts, "view type is valid");
         counts[view.kind]++;
         if (view.kind == "background") {
           browser.test.assertTrue(view === browser.extension.getBackgroundPage(),
                                   "background page is correct");
+          background = view;
         }
       }
-      browser.test.sendMessage("counts", counts);
+      if (background) {
+        browser.runtime.getBackgroundPage().then(view => {
+          browser.test.assertEq(background, view, "runtime.getBackgroundPage() is correct");
+          browser.test.sendMessage("counts", counts);
+        });
+      } else {
+        browser.test.sendMessage("counts", counts);
+      }
     } else if (msg == kind + "-open-tab") {
       browser.tabs.create({windowId: args[0], url: browser.runtime.getURL("tab.html")});
     } else if (msg == kind + "-close-tab") {
       browser.tabs.query({
         windowId: args[0],
       }, tabs => {
         let tab = tabs.find(tab => tab.url.indexOf("tab.html") != -1);
         browser.tabs.remove(tab.id, () => {
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -207,40 +207,51 @@ class BaseContext {
    * callback funciton must check `browser.runtime.lastError` or
    * `extension.runtime.lastError` in order to prevent it being reported
    * to the console.
    *
    * @param {Promise} promise The promise with which to wrap the
    *     callback. May resolve to a `SpreadArgs` instance, in which case
    *     each element will be used as a separate argument.
    *
+   *     Unless the promise object belongs to the cloneScope global, its
+   *     resolution value is cloned into cloneScope prior to calling the
+   *     `callback` function or resolving the wrapped promise.
+   *
    * @param {function} [callback] The callback function to wrap
    *
    * @returns {Promise|undefined} If callback is null, a promise object
    *     belonging to the target scope. Otherwise, undefined.
    */
   wrapPromise(promise, callback = null) {
+    // Note: `promise instanceof this.cloneScope.Promise` returns true
+    // here even for promises that do not belong to the content scope.
+    let runSafe = runSafeSync.bind(null, this);
+    if (promise.constructor === this.cloneScope.Promise) {
+      runSafe = runSafeSyncWithoutClone;
+    }
+
     if (callback) {
       promise.then(
         args => {
           if (args instanceof SpreadArgs) {
-            runSafeSync(this, callback, ...args);
+            runSafe(callback, ...args);
           } else {
-            runSafeSync(this, callback, args);
+            runSafe(callback, args);
           }
         },
         error => {
           this.withLastError(error, () => {
-            runSafeSync(this, callback);
+            runSafeSyncWithoutClone(callback);
           });
         });
     } else {
       return new this.cloneScope.Promise((resolve, reject) => {
         promise.then(
-          value => { runSafeSync(this, resolve, value); },
+          value => { runSafe(resolve, value); },
           value => {
             if (!(value instanceof this.cloneScope.Error)) {
               value = new this.cloneScope.Error(value.message);
             }
             runSafeSyncWithoutClone(reject, value);
           });
       });
     }
--- a/toolkit/components/extensions/ext-backgroundPage.js
+++ b/toolkit/components/extensions/ext-backgroundPage.js
@@ -128,14 +128,14 @@ extensions.registerSchemaAPI("extension"
   return {
     extension: {
       getBackgroundPage: function() {
         return backgroundPagesMap.get(extension).contentWindow;
       },
     },
 
     runtime: {
-      getBackgroundPage: function(callback) {
-        runSafe(context, callback, backgroundPagesMap.get(extension).contentWindow);
+      getBackgroundPage() {
+        return context.cloneScope.Promise.resolve(backgroundPagesMap.get(extension).contentWindow);
       },
     },
   };
 });
--- a/toolkit/components/extensions/schemas/runtime.json
+++ b/toolkit/components/extensions/schemas/runtime.json
@@ -109,16 +109,17 @@
         "description": "The ID of the extension/app."
       }
     },
     "functions": [
       {
         "name": "getBackgroundPage",
         "type": "function",
         "description": "Retrieves the JavaScript 'window' object for the background page running inside the current extension/app. If the background page is an event page, the system will ensure it is loaded before calling the callback. If there is no background page, an error is set.",
+        "async": "callback",
         "parameters": [
           {
             "type": "function",
             "name": "callback",
             "parameters": [
               {
                 "name": "backgroundPage",
                 "optional": true,