Bug 1389265 - Change contextual identity web extension APIs to reject instead of returning null. r?aswan draft
authorJonathan Kingston <jkt@mozilla.com>
Wed, 16 Aug 2017 03:09:48 +0100
changeset 647165 7d1c0f29a4a06a028cbc38bf2167818e27d9cd7b
parent 647164 6966f27380bf2e3e5ace3507f600852bd3ed9dbd
child 726418 2be4ebbb69556aa27ed4c6397cc93dc629bd93b3
push id74312
push userjkingston@mozilla.com
push dateWed, 16 Aug 2017 02:18:56 +0000
reviewersaswan
bugs1389265
milestone57.0a1
Bug 1389265 - Change contextual identity web extension APIs to reject instead of returning null. r?aswan MozReview-Commit-ID: 8htcRhzsj05
toolkit/components/extensions/ext-contextualIdentities.js
toolkit/components/extensions/test/xpcshell/test_ext_contextual_identities.js
--- a/toolkit/components/extensions/ext-contextualIdentities.js
+++ b/toolkit/components/extensions/ext-contextualIdentities.js
@@ -68,17 +68,19 @@ this.contextualIdentities = class extend
   }
 
   getAPI(context) {
     let self = {
       contextualIdentities: {
         get(cookieStoreId) {
           let containerId = getContainerForCookieStoreId(cookieStoreId);
           if (!containerId) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Invalid contextual identitiy: ${cookieStoreId}`,
+            });
           }
 
           let identity = ContextualIdentityService.getPublicIdentityFromId(containerId);
           return Promise.resolve(convertIdentity(identity));
         },
 
         query(details) {
           let identities = [];
@@ -99,22 +101,26 @@ this.contextualIdentities = class extend
                                                           details.icon,
                                                           details.color);
           return Promise.resolve(convertIdentity(identity));
         },
 
         update(cookieStoreId, details) {
           let containerId = getContainerForCookieStoreId(cookieStoreId);
           if (!containerId) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Invalid contextual identitiy: ${cookieStoreId}`,
+            });
           }
 
           let identity = ContextualIdentityService.getPublicIdentityFromId(containerId);
           if (!identity) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Invalid contextual identitiy: ${cookieStoreId}`,
+            });
           }
 
           if (details.name !== null) {
             identity.name = details.name;
           }
 
           if (details.color !== null) {
             identity.color = details.color;
@@ -122,38 +128,46 @@ this.contextualIdentities = class extend
 
           if (details.icon !== null) {
             identity.icon = details.icon;
           }
 
           if (!ContextualIdentityService.update(identity.userContextId,
                                                 identity.name, identity.icon,
                                                 identity.color)) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Contextual identitiy failed to update: ${cookieStoreId}`,
+            });
           }
 
           return Promise.resolve(convertIdentity(identity));
         },
 
         remove(cookieStoreId) {
           let containerId = getContainerForCookieStoreId(cookieStoreId);
           if (!containerId) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Invalid contextual identitiy: ${cookieStoreId}`,
+            });
           }
 
           let identity = ContextualIdentityService.getPublicIdentityFromId(containerId);
           if (!identity) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Invalid contextual identitiy: ${cookieStoreId}`,
+            });
           }
 
           // We have to create the identity object before removing it.
           let convertedIdentity = convertIdentity(identity);
 
           if (!ContextualIdentityService.remove(identity.userContextId)) {
-            return Promise.resolve(null);
+            return Promise.reject({
+              message: `Contextual identitiy failed to remove: ${cookieStoreId}`,
+            });
           }
 
           return Promise.resolve(convertedIdentity);
         },
 
         onCreated: new EventManager(context, "contextualIdentities.onCreated", fire => {
           let observer = (subject, topic) => {
             fire.async({contextualIdentity: convertIdentityFromObserver(subject)});
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contextual_identities.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contextual_identities.js
@@ -102,18 +102,20 @@ add_task(async function test_contextualI
 
   Services.prefs.clearUserPref(CONTAINERS_PREF);
 });
 
 add_task(async function test_contextualIdentity_with_permissions() {
   const CONTAINERS_PREF = "privacy.userContext.enabled";
   const initial = Services.prefs.getBoolPref(CONTAINERS_PREF);
   async function background(ver) {
-    let ci = await browser.contextualIdentities.get("foobar");
-    browser.test.assertEq(null, ci, "No identity should be returned here");
+    let ci;
+    await browser.test.assertRejects(browser.contextualIdentities.get("foobar"), "Invalid contextual identitiy: foobar", "API should reject here");
+    await browser.test.assertRejects(browser.contextualIdentities.update("foobar", {name: "testing"}), "Invalid contextual identitiy: foobar", "API should reject for unknown updates");
+    await browser.test.assertRejects(browser.contextualIdentities.remove("foobar"), "Invalid contextual identitiy: foobar", "API should reject for removing unknown containers");
 
     ci = await browser.contextualIdentities.get("firefox-container-1");
     browser.test.assertTrue(!!ci, "We have an identity");
     browser.test.assertTrue("name" in ci, "We have an identity.name");
     browser.test.assertTrue("color" in ci, "We have an identity.color");
     browser.test.assertTrue("icon" in ci, "We have an identity.icon");
     browser.test.assertEq("Personal", ci.name, "identity.name is correct");
     browser.test.assertEq("firefox-container-1", ci.cookieStoreId, "identity.cookieStoreId is correct");