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

--HG--
extra : rebase_source : 998de7388ee03fdec0a0949b4e43bd9169dbb592
extra : histedit_source : 414d0ed1842b2270146d37b2788a56c682d3d695

dom/base/ChromeUtils.cpp

@@ -421,6 +421,149 @@ ChromeUtils::Import(const GlobalObject& aGlobal,
   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,

dom/base/ChromeUtils.h

@@ -160,6 +160,12 @@ public:
                      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

dom/webidl/ChromeUtils.webidl

@@ -261,6 +261,39 @@ partial namespace ChromeUtils {
    */
   [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);
 };
 
 /**

js/xpconnect/loader/XPCOMUtils.jsm

@@ -321,6 +321,10 @@ this.XPCOMUtils = {
                                    aObject, aName, aResource, aSymbol,
                                    aPreLambda, aPostLambda, aProxy)
   {
+    if (arguments.length == 3) {
+      return ChromeUtils.defineModuleGetter(aObject, aName, aResource);
+    }
+
     let proxy = aProxy || {};
 
     if (typeof(aPreLambda) === "function") {
@@ -358,7 +362,7 @@ this.XPCOMUtils = {
                                    aObject, aModules)
   {
     for (let [name, module] of Object.entries(aModules)) {
-      this.defineLazyModuleGetter(aObject, name, module);
+      ChromeUtils.defineModuleGetter(aObject, name, module);
     }
   },
 

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");
+});

js/xpconnect/tests/unit/xpcshell.ini

@@ -67,6 +67,7 @@ support-files =
 [test_classesByID_instanceof.js]
 [test_compileScript.js]
 [test_deepFreezeClone.js]
+[test_defineModuleGetter.js]
 [test_file.js]
 [test_blob.js]
 [test_blob2.js]