Bug 1431533: Part 2 - Add ChromeUtils.defineModuleGetter helper. r?bz draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 17 Jan 2018 19:20:16 -0800
changeset 723881 9d054457dd2fed77b506fbbd98f1666887713ae8
parent 723880 1931e27261da728225533f66ce052eed1d5836b2
child 723882 f01a298948f94b79447ef0b525c91774ee84b9f3
push id96567
push usermaglione.k@gmail.com
push dateWed, 24 Jan 2018 00:47:30 +0000
reviewersbz
bugs1431533
milestone59.0a1
Bug 1431533: Part 2 - Add ChromeUtils.defineModuleGetter helper. r?bz This helper makes it easy to lazily import a JavaScript module the first time one of its exports is required. It is intended to replace XPCOMUtils.defineLazyModuleGetter, which has similar functionality but is much less efficient. MozReview-Commit-ID: 2zxXYwrn3Dr
dom/base/ChromeUtils.cpp
dom/base/ChromeUtils.h
dom/webidl/ChromeUtils.webidl
js/xpconnect/loader/XPCOMUtils.jsm
js/xpconnect/tests/unit/test_defineModuleGetter.js
js/xpconnect/tests/unit/xpcshell.ini
--- a/dom/base/ChromeUtils.cpp
+++ b/dom/base/ChromeUtils.cpp
@@ -416,16 +416,159 @@ ChromeUtils::Import(const GlobalObject& 
     return;
   }
 
   // Now we better have an object.
   MOZ_ASSERT(retval.isObject());
   aRetval.set(&retval.toObject());
 }
 
+namespace module_getter {
+  static const size_t SLOT_ID = 0;
+  static const size_t SLOT_URI = 1;
+
+  static bool
+  ExtractArgs(JSContext* aCx, JS::CallArgs& aArgs,
+              JS::MutableHandle<JSObject*> aCallee,
+              JS::MutableHandle<JSObject*> aThisObj,
+              JS::MutableHandle<jsid> aId)
+  {
+    aCallee.set(&aArgs.callee());
+
+    JS::Handle<JS::Value> thisv = aArgs.thisv();
+    if (!thisv.isObject()) {
+      JS_ReportErrorASCII(aCx, "Invalid target object");
+      return false;
+    }
+
+    aThisObj.set(&thisv.toObject());
+
+    JS::Rooted<JS::Value> id(aCx, js::GetFunctionNativeReserved(aCallee, SLOT_ID));
+    MOZ_ALWAYS_TRUE(JS_ValueToId(aCx, id, aId));
+    return true;
+  }
+
+  static bool
+  ModuleGetter(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
+  {
+    JS::CallArgs args = JS::CallArgsFromVp(aArgc, aVp);
+
+    JS::Rooted<JSObject*> callee(aCx);
+    JS::Rooted<JSObject*> thisObj(aCx);
+    JS::Rooted<jsid> id(aCx);
+    if (!ExtractArgs(aCx, args, &callee, &thisObj, &id)) {
+      return false;
+    }
+
+    JS::Rooted<JSString*> moduleURI(
+      aCx, js::GetFunctionNativeReserved(callee, SLOT_URI).toString());
+    JSAutoByteString bytes;
+    if (!bytes.encodeUtf8(aCx, moduleURI)) {
+      return false;
+    }
+    nsDependentCString uri(bytes.ptr());
+
+    RefPtr<mozJSComponentLoader> moduleloader = mozJSComponentLoader::Get();
+    MOZ_ASSERT(moduleloader);
+
+    JS::Rooted<JSObject*> moduleGlobal(aCx);
+    JS::Rooted<JSObject*> moduleExports(aCx);
+    nsresult rv = moduleloader->Import(aCx, uri, &moduleGlobal, &moduleExports);
+    if (NS_FAILED(rv)) {
+      Throw(aCx, rv);
+      return false;
+    }
+
+    JS::RootedValue value(aCx);
+    {
+      JSAutoCompartment ac(aCx, moduleExports);
+
+      if (!JS_GetPropertyById(aCx, moduleExports, id, &value)) {
+        return false;
+      }
+    }
+
+    if (!JS_WrapValue(aCx, &value) ||
+        !JS_DefinePropertyById(aCx, thisObj, id, value,
+                               JSPROP_ENUMERATE)) {
+      return false;
+    }
+
+    args.rval().set(value);
+    return true;
+  }
+
+  static bool
+  ModuleSetter(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
+  {
+    JS::CallArgs args = JS::CallArgsFromVp(aArgc, aVp);
+
+    JS::Rooted<JSObject*> callee(aCx);
+    JS::Rooted<JSObject*> thisObj(aCx);
+    JS::Rooted<jsid> id(aCx);
+    if (!ExtractArgs(aCx, args, &callee, &thisObj, &id)) {
+      return false;
+    }
+
+    return JS_DefinePropertyById(aCx, thisObj, id, args.get(0),
+                                 JSPROP_ENUMERATE);
+  }
+
+  static bool
+  DefineGetter(JSContext* aCx,
+               JS::Handle<JSObject*> aTarget,
+               const nsAString& aId,
+               const nsAString& aResourceURI)
+  {
+    JS::RootedValue uri(aCx);
+    JS::RootedValue idValue(aCx);
+    JS::Rooted<jsid> id(aCx);
+    if (!xpc::NonVoidStringToJsval(aCx, aResourceURI, &uri) ||
+        !xpc::NonVoidStringToJsval(aCx, aId, &idValue) ||
+        !JS_ValueToId(aCx, idValue, &id)) {
+      return false;
+    }
+    idValue = js::IdToValue(id);
+
+
+    JS::Rooted<JSObject*> getter(aCx, JS_GetFunctionObject(
+      js::NewFunctionByIdWithReserved(aCx, ModuleGetter, 0, 0, id)));
+
+    JS::Rooted<JSObject*> setter(aCx, JS_GetFunctionObject(
+      js::NewFunctionByIdWithReserved(aCx, ModuleSetter, 0, 0, id)));
+
+    if (!getter || !setter) {
+      JS_ReportOutOfMemory(aCx);
+      return false;
+    }
+
+    js::SetFunctionNativeReserved(getter, SLOT_ID, idValue);
+    js::SetFunctionNativeReserved(setter, SLOT_ID, idValue);
+
+    js::SetFunctionNativeReserved(getter, SLOT_URI, uri);
+
+    return JS_DefinePropertyById(aCx, aTarget, id,
+                                 JS_DATA_TO_FUNC_PTR(JSNative, getter.get()),
+                                 JS_DATA_TO_FUNC_PTR(JSNative, setter.get()),
+                                 JSPROP_GETTER | JSPROP_SETTER | JSPROP_ENUMERATE);
+  }
+} // namespace module_getter
+
+/* static */ void
+ChromeUtils::DefineModuleGetter(const GlobalObject& global,
+                                JS::Handle<JSObject*> target,
+                                const nsAString& id,
+                                const nsAString& resourceURI,
+                                ErrorResult& aRv)
+{
+  if (!module_getter::DefineGetter(global.Context(), target, id, resourceURI)) {
+    aRv.NoteJSContextException(global.Context());
+  }
+}
+
 /* static */ void
 ChromeUtils::OriginAttributesToSuffix(dom::GlobalObject& aGlobal,
                                       const dom::OriginAttributesDictionary& aAttrs,
                                       nsCString& aSuffix)
 
 {
   OriginAttributes attrs(aAttrs);
   attrs.CreateSuffix(aSuffix);
--- a/dom/base/ChromeUtils.h
+++ b/dom/base/ChromeUtils.h
@@ -155,14 +155,20 @@ public:
 
   static void ClearRecentJSDevError(GlobalObject& aGlobal);
 
   static void Import(const GlobalObject& aGlobal,
                      const nsAString& aResourceURI,
                      const Optional<JS::Handle<JSObject*>>& aTargetObj,
                      JS::MutableHandle<JSObject*> aRetval,
                      ErrorResult& aRv);
+
+  static void DefineModuleGetter(const GlobalObject& global,
+                                 JS::Handle<JSObject*> target,
+                                 const nsAString& id,
+                                 const nsAString& resourceURI,
+                                 ErrorResult& aRv);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_ChromeUtils__
--- a/dom/webidl/ChromeUtils.webidl
+++ b/dom/webidl/ChromeUtils.webidl
@@ -256,16 +256,49 @@ partial namespace ChromeUtils {
    * The implementation maintains a hash of aResourceURI->global obj.
    * Subsequent invocations of import with 'aResourceURI' pointing to
    * the same file will not cause the module to be re-evaluated, but
    * the symbols in EXPORTED_SYMBOLS will be exported into the
    * specified target object and the global object returned as above.
    */
   [Throws]
   object import(DOMString aResourceURI, optional object? aTargetObj);
+
+  /**
+   * Defines a property on the given target which lazily imports a JavaScript
+   * module when accessed.
+   *
+   * The first time the property is accessed, the module at the given URL is
+   * imported, and the property is replaced with the module's exported symbol
+   * of the same name.
+   *
+   * Some points to note when using this utility:
+   *
+   * - The cached module export is always stored on the `this` object that was
+   *   used to access the getter. This means that if you define the lazy
+   *   getter on a prototype, the module will be re-imported every time the
+   *   property is accessed on a new instance.
+   *
+   * - The getter property may be overwritten by simple assignment, but as
+   *   with imports, the new property value is always defined on the `this`
+   *   object that the setter is called with.
+   *
+   * - If the module import fails, the getter will throw an error, and the
+   *   property will not be replaced. Each subsequent attempt to access the
+   *   getter will attempt to re-import the object, which will likely continue
+   *   to result in errors.
+   *
+   * @param target The target object on which to define the property.
+   * @param id The name of the property to define, and of the symbol to
+   *           import.
+   * @param resourceURI The resource URI of the module, as passed to
+   *                    ChromeUtils.import.
+   */
+  [Throws]
+  void defineModuleGetter(object target, DOMString id, DOMString resourceURI);
 };
 
 /**
  * Used by principals and the script security manager to represent origin
  * attributes. The first dictionary is designed to contain the full set of
  * OriginAttributes, the second is used for pattern-matching (i.e. does this
  * OriginAttributesDictionary match the non-empty attributes in this pattern).
  *
--- a/js/xpconnect/loader/XPCOMUtils.jsm
+++ b/js/xpconnect/loader/XPCOMUtils.jsm
@@ -316,16 +316,20 @@ this.XPCOMUtils = {
    * @param aProxy
    *        An object which acts on behalf of the module to be imported until
    *        the module has been imported.
    */
   defineLazyModuleGetter: function XPCU_defineLazyModuleGetter(
                                    aObject, aName, aResource, aSymbol,
                                    aPreLambda, aPostLambda, aProxy)
   {
+    if (arguments.length == 3) {
+      return ChromeUtils.defineModuleGetter(aObject, aName, aResource);
+    }
+
     let proxy = aProxy || {};
 
     if (typeof(aPreLambda) === "function") {
       aPreLambda.apply(proxy);
     }
 
     this.defineLazyGetter(aObject, aName, function XPCU_moduleLambda() {
       var temp = {};
@@ -353,17 +357,17 @@ this.XPCOMUtils = {
    *        An object with a property for each module property to be
    *        imported, where the property name is the name of the
    *        imported symbol and the value is the module URI.
    */
   defineLazyModuleGetters: function XPCU_defineLazyModuleGetters(
                                    aObject, aModules)
   {
     for (let [name, module] of Object.entries(aModules)) {
-      this.defineLazyModuleGetter(aObject, name, module);
+      ChromeUtils.defineModuleGetter(aObject, name, module);
     }
   },
 
   /**
    * Defines a getter on a specified object for preference value. The
    * preference is read the first time that the property is accessed,
    * and is thereafter kept up-to-date using a preference observer.
    *
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_defineModuleGetter.js
@@ -0,0 +1,115 @@
+"use strict";
+
+function assertIsGetter(obj, prop) {
+  let desc = Object.getOwnPropertyDescriptor(obj, prop);
+
+  ok(desc, `Property ${prop} exists on object`);
+  equal(typeof desc.get, "function", `Getter function exists for property ${prop}`);
+  equal(typeof desc.set, "function", `Setter function exists for property ${prop}`);
+  equal(desc.enumerable, true, `Property ${prop} is enumerable`);
+  equal(desc.configurable, true, `Property ${prop} is configurable`);
+}
+
+function assertIsValue(obj, prop, value) {
+  let desc = Object.getOwnPropertyDescriptor(obj, prop);
+
+  ok(desc, `Property ${prop} exists on object`);
+
+  ok("value" in desc, `${prop} is a data property`);
+  equal(desc.value, value, `${prop} has the expected value`);
+
+  equal(desc.enumerable, true, `Property ${prop} is enumerable`);
+  equal(desc.configurable, true, `Property ${prop} is configurable`);
+  equal(desc.writable, true, `Property ${prop} is writable`);
+}
+
+add_task(async function() {
+  let temp = {};
+  ChromeUtils.import("resource://gre/modules/Services.jsm", temp);
+
+  let obj = {};
+  let child = Object.create(obj);
+  let sealed = Object.seal(Object.create(obj));
+
+
+  // Test valid import
+
+  ChromeUtils.defineModuleGetter(obj, "Services",
+                                 "resource://gre/modules/Services.jsm");
+
+  assertIsGetter(obj, "Services");
+  equal(child.Services, temp.Services, "Getter works on descendent object");
+  assertIsValue(child, "Services", temp.Services);
+  assertIsGetter(obj, "Services");
+
+  Assert.throws(() => sealed.Services, /Object is not extensible/,
+                "Cannot access lazy getter from sealed object");
+  Assert.throws(() => sealed.Services = null, /Object is not extensible/,
+                "Cannot access lazy setter from sealed object");
+  assertIsGetter(obj, "Services");
+
+  equal(obj.Services, temp.Services, "Getter works on object");
+  assertIsValue(obj, "Services", temp.Services);
+
+
+  // Test overwriting via setter
+
+  child = Object.create(obj);
+
+  ChromeUtils.defineModuleGetter(obj, "Services",
+                                 "resource://gre/modules/Services.jsm");
+
+  assertIsGetter(obj, "Services");
+
+  child.Services = "foo";
+  assertIsValue(child, "Services", "foo");
+  assertIsGetter(obj, "Services");
+
+  obj.Services = "foo";
+  assertIsValue(obj, "Services", "foo");
+
+
+  // Test import missing property
+
+  ChromeUtils.defineModuleGetter(obj, "meh",
+                                 "resource://gre/modules/Services.jsm");
+  assertIsGetter(obj, "meh");
+  equal(obj.meh, undefined, "Missing property returns undefined");
+  assertIsValue(obj, "meh", undefined);
+
+
+  // Test import broken module
+
+  ChromeUtils.defineModuleGetter(obj, "broken",
+                                 "resource://test/bogus_exports_type.jsm");
+  assertIsGetter(obj, "broken");
+
+  let errorPattern = /EXPORTED_SYMBOLS is not an array/;
+  Assert.throws(() => child.broken, errorPattern,
+                "Broken import throws on child");
+  Assert.throws(() => child.broken, errorPattern,
+                "Broken import throws on child again");
+  Assert.throws(() => sealed.broken, errorPattern,
+                "Broken import throws on sealed child");
+  Assert.throws(() => obj.broken, errorPattern,
+                "Broken import throws on object");
+  assertIsGetter(obj, "broken");
+
+
+  // Test import missing module
+
+  ChromeUtils.defineModuleGetter(obj, "missing",
+                                 "resource://test/does_not_exist.jsm");
+  assertIsGetter(obj, "missing");
+
+  Assert.throws(() => obj.missing, /NS_ERROR_FILE_NOT_FOUND/,
+                "missing import throws on object");
+  assertIsGetter(obj, "missing");
+
+
+  // Test overwriting broken import via setter
+
+  assertIsGetter(obj, "broken");
+  obj.broken = "foo";
+  assertIsValue(obj, "broken", "foo");
+});
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -62,16 +62,17 @@ support-files =
 [test_bug1151385.js]
 [test_bug1170311.js]
 [test_bug1244222.js]
 [test_bug_442086.js]
 [test_callFunctionWithAsyncStack.js]
 [test_classesByID_instanceof.js]
 [test_compileScript.js]
 [test_deepFreezeClone.js]
+[test_defineModuleGetter.js]
 [test_file.js]
 [test_blob.js]
 [test_blob2.js]
 [test_file2.js]
 [test_import.js]
 [test_import_fail.js]
 [test_interposition.js]
 [test_isModuleLoaded.js]