Bug 1477432 - Part 2: Avoid using nsIJSID in GenerateQI, and produce better diagnostics, r=kmag

This is the first part of hiding the implementation of nsIJSID behind the
interface added in Part 1, such that we can substitute that implementation out.

I had to make a couple of changes to fix the errors caused by the new behaviour
in GenerateQI.

Differential Revision: https://phabricator.services.mozilla.com/D2279

browser/modules/Sanitizer.jsm

@@ -293,7 +293,7 @@ var Sanitizer = {
   },
 
   QueryInterface: ChromeUtils.generateQI([
-    Ci.nsiObserver,
+    Ci.nsIObserver,
     Ci.nsISupportsWeakReference,
   ]),
 

dom/base/ChromeUtils.h

@@ -126,7 +126,7 @@ public:
                 ErrorResult& aRv);
 
   static MozQueryInterface*
-  GenerateQI(const GlobalObject& global, const Sequence<OwningStringOrIID>& interfaces,
+  GenerateQI(const GlobalObject& global, const Sequence<JS::Value>& interfaces,
              ErrorResult& aRv);
 
   static void WaiveXrays(GlobalObject& aGlobal,

dom/base/MozQueryInterface.cpp

@@ -6,6 +6,7 @@
 
 #include "ChromeUtils.h"
 #include "MozQueryInterface.h"
+#include "nsIException.h"
 
 #include <string.h>
 
@@ -30,57 +31,37 @@ CompareIIDs(const nsIID& aA, const nsIID &aB)
 
 /* static */
 MozQueryInterface*
-ChromeUtils::GenerateQI(const GlobalObject& aGlobal, const Sequence<OwningStringOrIID>& aInterfaces, ErrorResult& aRv)
+ChromeUtils::GenerateQI(const GlobalObject& aGlobal,
+                        const Sequence<JS::Value>& aInterfaces,
+                        ErrorResult& aRv)
 {
   JSContext* cx = aGlobal.Context();
-  JS::RootedObject xpcIfaces(cx);
 
   nsTArray<nsIID> ifaces;
 
-  JS::RootedValue val(cx);
-  for (auto& iface : aInterfaces) {
-    if (iface.IsIID()) {
-      ifaces.AppendElement(*iface.GetAsIID()->GetID());
+  JS::RootedValue iface(cx);
+  for (uint32_t idx = 0; idx < aInterfaces.Length(); ++idx) {
+    iface = aInterfaces[idx];
+
+    // Handle ID objects
+    if (Maybe<nsID> id = xpc::JSValue2ID(cx, iface)) {
+      ifaces.AppendElement(*id);
       continue;
     }
 
-    // If we have a string value, we need to look up the interface name. The
-    // simplest and most efficient way to do this is to just grab the "Ci"
-    // object from the global scope.
-    if (!xpcIfaces) {
-      JS::RootedObject global(cx, aGlobal.Get());
-      if (!JS_GetProperty(cx, global, "Ci", &val)) {
-        aRv.NoteJSContextException(cx);
-        return nullptr;
+    // Accept string valued names
+    if (iface.isString()) {
+      JS::UniqueChars name = JS_EncodeStringToLatin1(cx, iface.toString());
+
+      const nsXPTInterfaceInfo* iinfo = nsXPTInterfaceInfo::ByName(name.get());
+      if (iinfo) {
+        ifaces.AppendElement(iinfo->IID());
+        continue;
       }
-      if (!val.isObject()) {
-        aRv.Throw(NS_ERROR_UNEXPECTED);
-        return nullptr;
-      }
-      xpcIfaces = &val.toObject();
     }
 
-    auto& name = iface.GetAsString();
-    if (!JS_GetUCProperty(cx, xpcIfaces, name.get(), name.Length(), &val)) {
-      aRv.NoteJSContextException(cx);
-      return nullptr;
-    }
-
-    if (val.isNullOrUndefined()) {
-      continue;
-    }
-    if (!val.isObject()) {
-      aRv.Throw(NS_ERROR_INVALID_ARG);
-      return nullptr;
-    }
-
-    nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(&val.toObject());
-    nsCOMPtr<nsIJSID> iid = do_QueryInterface(base);
-    if (!iid) {
-      aRv.Throw(NS_ERROR_INVALID_ARG);
-      return nullptr;
-    }
-    ifaces.AppendElement(*iid->GetID());
+    // NOTE: We ignore unknown interfaces here because in some cases we try to
+    // pass them in to support multiple platforms.
   }
 
   MOZ_ASSERT(!ifaces.Contains(NS_GET_IID(nsISupports), CompareIIDs));

dom/chrome-webidl/ChromeUtils.webidl

@@ -215,17 +215,14 @@ partial namespace ChromeUtils {
    * JavaScript, acts as an ordinary QueryInterface function call, and when
    * called from XPConnect, circumvents JSAPI entirely.
    *
-   * The list of interfaces may include a mix of nsIJSID objects and interface
-   * name strings. Strings for nonexistent interface names are silently
-   * ignored, as long as they don't refer to any non-IID property of the Ci
-   * global. Any non-IID value is implicitly coerced to a string, and treated
-   * as an interface name.
+   * The list of interfaces may include a mix of JS ID objects and interface
+   * name strings.
    *
    * nsISupports is implicitly supported, and must not be included in the
    * interface list.
    */
   [Affects=Nothing, NewObject, Throws]
-  MozQueryInterface generateQI(sequence<(DOMString or IID)> interfaces);
+  MozQueryInterface generateQI(sequence<any> interfaces);
 
   /**
    * Waive Xray on a given value. Identity op for primitives.

editor/composer/test/test_bug1266815.html

@@ -8,6 +8,8 @@
 <body>
 <p id="display"></p>
 <script type="text/javascript">
+// XXX(nika): Why are we using SpecialPowers here? If we're a chrome mochitest
+// can't we avoid using the SpecialPowers wrappers?
 const Cc = SpecialPowers.Cc;
 const Ci = SpecialPowers.Ci;
 const Cu = SpecialPowers.Cu;
@@ -38,7 +40,7 @@ var helperAppDlgPromise = new Promise(function(resolve) {
       registrar.unregisterFactory(MOCK_HELPERAPP_DIALOG_CID,
                                   mockHelperAppService);
     },
-    QueryInterface: ChromeUtils.generateQI([Ci.nsIHelperAppLauncherDialog]),
+    QueryInterface: ChromeUtils.generateQI(["nsIHelperAppLauncherDialog"]),
   };
 
   mockHelperAppService = XPCOMUtils._getFactory(HelperAppLauncherDialog);

js/xpconnect/tests/unit/test_generateQI.js

@@ -24,9 +24,6 @@ add_task(async function test_generateQI() {
   ChromeUtils.generateQI([])(Ci.nsISupports);
 
   // Test failure scenarios.
-  Assert.throws(() => ChromeUtils.generateQI(["toString"]),
-                e => e.result == Cr.NS_ERROR_INVALID_ARG);
-
   Assert.throws(() => checkQI([], Ci.nsIPropertyBag),
                 e => e.result == Cr.NS_ERROR_NO_INTERFACE);
 });

toolkit/components/satchel/FormHistoryStartup.js

@@ -16,7 +16,6 @@ FormHistoryStartup.prototype = {
   QueryInterface: ChromeUtils.generateQI([
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference,
-    Ci.nsIFrameMessageListener,
   ]),
 
   observe(subject, topic, data) {