Bug 1291737 - handle empty API namespaces in schema injection helpers. draft
authorLuca Greco <lgreco@mozilla.com>
Fri, 02 Sep 2016 17:31:29 +0200
changeset 409948 5b4fec98e9f1051d3e402c82057ae2a48ddbadbc
parent 409947 fd52bba05cd714f2062769c7bf2654e403f0b944
child 409949 93b903fdcbff91b8ece6ae0f8c0b3aa8c7ec1b8c
push id28616
push userluca.greco@alcacoop.it
push dateMon, 05 Sep 2016 18:44:16 +0000
bugs1291737
milestone51.0a1
Bug 1291737 - handle empty API namespaces in schema injection helpers. MozReview-Commit-ID: GdjQn0QCgRv
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/test/xpcshell/test_ext_schemas_api_injection.js
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -285,34 +285,21 @@ class ProxyContext extends ExtensionCont
 
   get externallyVisible() {
     return false;
   }
 }
 
 function findPathInObject(obj, path) {
   for (let elt of path.split(".")) {
-    // If we get a null object before reaching the requested path
-    // (e.g. the API object is returned only on particular kind of contexts instead
-    // of based on WebExtensions permissions, like it happens for the devtools APIs),
-    // stop searching and return undefined.
-    // TODO(robwu): This should never be reached. If an API is not available for
-    // a context, it should be declared as such in the schema and enforced by
-    // `shouldInject`, for instance using the same logic that is used to opt-in
-    // to APIs in content scripts.
-    // If this check is kept, then there is a discrepancy between APIs depending
-    // on whether it is generated locally or remotely: Non-existing local APIs
-    // are excluded in `shouldInject` by this check, but remote APIs do not have
-    // this information and will therefore cause the schema API generator to
-    // create an API that proxies to a non-existing API implementation.
-    if (!obj || !(elt in obj)) {
-      return null;
-    }
-
-    obj = obj[elt];
+    // NOTE: some of the defined namespaces in the schema (e.g. manifest,
+    // events, extensionTypes) doesn't have an API implementation (and
+    // none of them is currently a nested namespace), if any of these
+    // namespace is nested, then this check will not be enough.
+    obj = obj[elt] || undefined;
   }
 
   return obj;
 }
 
 let ParentAPIManager = {
   proxyContexts: new Map(),
 
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas_api_injection.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas_api_injection.js
@@ -3,101 +3,82 @@
 Components.utils.import("resource://gre/modules/Schemas.jsm");
 
 let {
   Management,
 } = Components.utils.import("resource://gre/modules/Extension.jsm", {});
 
 let nestedNamespaceJson = [
   {
-    "namespace": "backgroundAPI.testnamespace",
+    "namespace": "fakeAPI.testnamespace",
     "functions": [
       {
         "name": "create",
         "type": "function",
         "parameters": [
           {
             "name": "title",
             "type": "string",
           },
         ],
         "returns": {
           "type": "string",
         },
       },
     ],
   },
+
+  // NOTE: some of the defined namespaces in the schema (e.g. manifest, events, extensionTypes)
+  // doesn't have an API implementation (and none of them is currently a nested namespace)
   {
-    "namespace": "noBackgroundAPI.testnamespace",
-    "functions": [
+    "namespace": "fakeNS_without_an_API_implemention",
+    "types": [
       {
-        "name": "create",
-        "type": "function",
-        "parameters": [
-          {
-            "name": "title",
+        "id": "FakeType",
+        "type": "object",
+        "properties": {
+          "fakeProp": {
             "type": "string",
           },
-        ],
+        },
       },
     ],
   },
 ];
 
 add_task(function* testSchemaAPIInjection() {
   let url = "data:," + JSON.stringify(nestedNamespaceJson);
 
   // Load the schema of the fake APIs.
   yield Schemas.load(url);
 
-  // Register an API that will skip the background page.
-  Management.registerSchemaAPI("noBackgroundAPI.testnamespace", "addon_child", context => {
-    if (context.type !== "background") {
-      return {
-        noBackgroundAPI: {
-          testnamespace: {
-            create(title) {},
+  // Register an API that will skip any but the background page.
+  Management.registerSchemaAPI("fakeAPI.testnamespace", "addon_parent", context => {
+    return {
+      fakeAPI: {
+        testnamespace: {
+          create(title) {
+            return title;
           },
         },
-      };
-    }
-
-    // This API should not be available in this context, return null so that
-    // the schema wrapper is removed as well.
-    return null;
-  });
-
-  // Register an API that will skip any but the background page.
-  Management.registerSchemaAPI("backgroundAPI.testnamespace", "addon_child", context => {
-    if (context.type === "background") {
-      return {
-        backgroundAPI: {
-          testnamespace: {
-            create(title) {
-              return title;
-            },
-          },
-        },
-      };
-    }
-
-    // This API should not be available in this context, return null so that
-    // the schema wrapper is removed as well.
-    return null;
+      },
+    };
   });
 
   let extension = ExtensionTestUtils.loadExtension({
     background() {
-      if (browser.noBackgroundAPI) {
-        browser.test.notifyFail("skipAPIonNull.done");
+      if (browser.fakeNS) {
+        browser.test.notifyFail("schema_api_injection.done");
       } else {
-        const res = browser.backgroundAPI.testnamespace.create("param-value");
+        const res = browser.fakeAPI.testnamespace.create("param-value");
         browser.test.assertEq("param-value", res,
                               "Got the expected result from the fake API method");
-        browser.test.notifyPass("skipAPIonNull.done");
+        browser.test.notifyPass("schema_api_injection.done");
       }
     },
   });
 
   yield extension.startup();
-  yield extension.awaitFinish("skipAPIonNull.done");
+
+  yield extension.awaitFinish("schema_api_injection.done");
+
   yield extension.unload();
 });