Bug 1069518 - XPTCall should refuse to implement interfaces with [notxpcom] methods, r=froydnj/bholley

js/xpconnect/src/XPCWrappedJS.cpp

@@ -347,6 +347,7 @@ nsXPCWrappedJS::GetNewOrUsed(JS::HandleObject jsObj,
     if (!rootJSObj)
         return NS_ERROR_FAILURE;
 
+    nsresult rv = NS_ERROR_FAILURE;
     nsRefPtr<nsXPCWrappedJS> root = map->Find(rootJSObj);
     if (root) {
         nsRefPtr<nsXPCWrappedJS> wrapper = root->FindOrFindInherited(aIID);
@@ -363,10 +364,16 @@ nsXPCWrappedJS::GetNewOrUsed(JS::HandleObject jsObj,
         if (!rootClasp)
             return NS_ERROR_FAILURE;
 
-        root = new nsXPCWrappedJS(cx, rootJSObj, rootClasp, nullptr);
+        root = new nsXPCWrappedJS(cx, rootJSObj, rootClasp, nullptr, &rv);
+        if (NS_FAILED(rv)) {
+            return rv;
+        }
     }
 
-    nsRefPtr<nsXPCWrappedJS> wrapper = new nsXPCWrappedJS(cx, jsObj, clasp, root);
+    nsRefPtr<nsXPCWrappedJS> wrapper = new nsXPCWrappedJS(cx, jsObj, clasp, root, &rv);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
     wrapper.forget(wrapperResult);
     return NS_OK;
 }
@@ -374,13 +381,16 @@ nsXPCWrappedJS::GetNewOrUsed(JS::HandleObject jsObj,
 nsXPCWrappedJS::nsXPCWrappedJS(JSContext* cx,
                                JSObject* aJSObj,
                                nsXPCWrappedJSClass* aClass,
-                               nsXPCWrappedJS* root)
+                               nsXPCWrappedJS* root,
+                               nsresult *rv)
     : mJSObj(aJSObj),
       mClass(aClass),
       mRoot(root ? root : MOZ_THIS_IN_INITIALIZER_LIST()),
       mNext(nullptr)
 {
-    InitStub(GetClass()->GetIID());
+    *rv = InitStub(GetClass()->GetIID());
+    // Continue even in the failure case, so that our refcounting/Destroy
+    // behavior works correctly.
 
     // There is an extra AddRef to support weak references to wrappers
     // that are subject to finalization. See the top of the file for more

js/xpconnect/src/xpcprivate.h

@@ -2493,7 +2493,8 @@ protected:
     nsXPCWrappedJS(JSContext* cx,
                    JSObject* aJSObj,
                    nsXPCWrappedJSClass* aClass,
-                   nsXPCWrappedJS* root);
+                   nsXPCWrappedJS* root,
+                   nsresult* rv);
 
     bool CanSkip();
     void Destroy();

xpcom/reflect/xptcall/xptcall.cpp

@@ -8,6 +8,7 @@
 #include "xptcprivate.h"
 #include "xptiprivate.h"
 #include "mozilla/XPTInterfaceInfoManager.h"
+#include "nsPrintfCString.h"
 
 using namespace mozilla;
 
@@ -52,6 +53,14 @@ NS_GetXPTCallStub(REFNSIID aIID, nsIXPTCProxy* aOuter,
     if (!iie || !iie->EnsureResolved() || iie->GetBuiltinClassFlag())
         return NS_ERROR_FAILURE;
 
+    if (iie->GetHasNotXPCOMFlag()) {
+#ifdef DEBUG
+        nsPrintfCString msg("XPTCall will not implement interface %s because of [notxpcom] members.", iie->GetTheName());
+        NS_WARNING(msg.get());
+#endif
+        return NS_ERROR_FAILURE;
+    }
+
     nsXPTCStubBase* newbase = new nsXPTCStubBase(aOuter, iie);
     if (!newbase)
         return NS_ERROR_OUT_OF_MEMORY;

xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp

@@ -107,6 +107,19 @@ xptiInterfaceEntry::ResolveLocked()
         }
 
         mParent = parent;
+        if (parent->GetHasNotXPCOMFlag()) {
+            SetHasNotXPCOMFlag();
+        } else {
+            for (uint16_t idx = 0; idx < mDescriptor->num_methods; ++idx) {
+                nsXPTMethodInfo* method = reinterpret_cast<nsXPTMethodInfo*>(
+                    mDescriptor->method_descriptors + idx);
+                if (method->IsNotXPCOM()) {
+                    SetHasNotXPCOMFlag();
+                    break;
+                }
+            }
+        }
+
 
         mMethodBaseIndex =
             parent->mMethodBaseIndex + 

xpcom/reflect/xptinfo/xptiprivate.h

@@ -180,7 +180,7 @@ public:
     };
     
     // Additional bit flags...
-    enum {SCRIPTABLE = 4, BUILTINCLASS = 8};
+    enum {SCRIPTABLE = 4, BUILTINCLASS = 8, HASNOTXPCOM = 16};
 
     uint8_t GetResolveState() const {return mFlags.GetState();}
     
@@ -196,6 +196,17 @@ public:
     bool GetBuiltinClassFlag() const
                 {return mFlags.GetFlagBit(uint8_t(BUILTINCLASS));}
 
+
+    // AddRef/Release are special and are not considered for the NOTXPCOM flag.
+    void SetHasNotXPCOMFlag()
+    {
+        mFlags.SetFlagBit(HASNOTXPCOM, true);
+    }
+    bool GetHasNotXPCOMFlag() const
+    {
+        return mFlags.GetFlagBit(HASNOTXPCOM);
+    }
+
     const nsID* GetTheIID()  const {return &mIID;}
     const char* GetTheName() const {return mName;}
 

xpcom/tests/Makefile.in

@@ -5,6 +5,9 @@
 # Make sure we have symbols in case we need to debug these.
 MOZ_DEBUG_SYMBOLS = 1
 
+# Don't add our test-only .xpt files to the normal manifests
+NO_INTERFACES_MANIFEST = 1
+
 include $(topsrcdir)/config/rules.mk
 
 ifneq (,$(SIMPLE_PROGRAMS))
@@ -12,6 +15,9 @@ libs::
 	$(INSTALL) $(SIMPLE_PROGRAMS) $(DEPTH)/_tests/xpcshell/$(relativesrcdir)/unit
 endif
 
+libs::
+	$(INSTALL) $(DIST)/bin/components/xpcomtest.xpt $(DEPTH)/_tests/xpcshell/$(relativesrcdir)/unit
+
 ifeq (,$(filter-out WINNT, $(HOST_OS_ARCH)))
 getnativepath = $(call normalizepath,$(1))
 else

xpcom/tests/NotXPCOMTest.idl

@@ -0,0 +1,23 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsISupports.idl"
+
+[scriptable, uuid(93142a4f-e4cf-424a-b833-e638f87d2607)]
+interface ScriptableOK : nsISupports
+{
+  void method1();
+};
+
+[scriptable, uuid(237d01a3-771e-4c6e-adf9-c97f9aab2950)]
+interface ScriptableWithNotXPCOM : nsISupports
+{
+  [notxpcom] void method2();
+};
+
+[scriptable, uuid(4f06ec60-3bb3-4712-ab18-b2b595285558)]
+interface ScriptableWithNotXPCOMBase : ScriptableWithNotXPCOM
+{
+  void method3();
+};

xpcom/tests/TestScriptable.idl

@@ -1,21 +0,0 @@
-
-[scriptable, uuid(76d74662-0eae-404c-9d1f-697c0e321c0a)]
-interface testScriptableInterface {
-  readonly attribute long scriptable_attr1;
-  attribute long scriptable_attr2;
-  [noscript] readonly attribute long notscriptable_attr1;
-  [noscript] attribute long notscriptable_attr2;
-
-  void scriptable_method1();
-  [noscript] void notscriptable_method1();
-  [notxpcom] void notscriptable_method2();
-  [noscript, notxpcom] void notscriptable_method3();
-};
-
-[uuid(76d74662-0eae-404c-9d1f-697c0e321c0a)]
-interface testNotscriptableInterface {
-  readonly attribute long notscriptable_attr1;
-  attribute long notscriptable_attr2;
-
-  void notscriptable_method1();
-};

xpcom/tests/moz.build

@@ -110,6 +110,11 @@ if CONFIG['MOZ_DEBUG'] and CONFIG['OS_ARCH'] not in ('WINNT'):
         'TestDeadlockDetectorScalability',
     ])
 
+XPIDL_MODULE = 'xpcomtest'
+XPIDL_SOURCES += [
+    'NotXPCOMTest.idl',
+]
+
 LOCAL_INCLUDES += [
     '../ds',
 ]

xpcom/tests/unit/test_notxpcom_scriptable.js

@@ -0,0 +1,86 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+const kCID = Components.ID("{1f9f7181-e6c5-4f4c-8f71-08005cec8468}");
+const kContract = "@testing/notxpcomtest";
+
+function run_test()
+{
+  let manifest = do_get_file("xpcomtest.manifest");
+  let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
+  registrar.autoRegister(manifest);
+
+  ok(Ci.ScriptableWithNotXPCOM);
+
+  let method1Called = false;
+
+  let testObject = {
+    QueryInterface: XPCOMUtils.generateQI([Ci.ScriptableOK,
+                                           Ci.ScriptableWithNotXPCOM,
+                                           Ci.ScriptableWithNotXPCOMBase]),
+
+    method1: function() {
+      method1Called = true;
+    },
+
+    method2: function() {
+      ok(false, "method2 should not have been called!");
+    },
+
+    method3: function() {
+      ok(false, "mehod3 should not have been called!");
+    },
+
+    jsonly: true,
+  };
+
+  let factory = {
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIFactory]),
+
+    createInstance: function(outer, iid) {
+      if (outer) {
+        throw Cr.NS_ERROR_NO_AGGREGATION;
+      }
+      return testObject.QueryInterface(iid);
+    },
+  };
+
+  registrar.registerFactory(kCID, null, kContract, factory);
+
+  let xpcomObject = Cc[kContract].createInstance();
+  ok(xpcomObject);
+  strictEqual(xpcomObject.jsonly, undefined);
+
+  xpcomObject.QueryInterface(Ci.ScriptableOK);
+
+  xpcomObject.method1();
+  ok(method1Called);
+
+  try {
+    xpcomObject.QueryInterface(Ci.ScriptableWithNotXPCOM);
+    ok(false, "Should not have implemented ScriptableWithNotXPCOM");
+  }
+  catch(e) {
+    ok(true, "Should not have implemented ScriptableWithNotXPCOM. Correctly threw error: " + e);
+  }
+  strictEqual(xpcomObject.method2, undefined);
+
+  try {
+    xpcomObject.QueryInterface(Ci.ScriptableWithNotXPCOMBase);
+    ok(false, "Should not have implemented ScriptableWithNotXPCOMBase");
+  }
+  catch (e) {
+    ok(true, "Should not have implemented ScriptableWithNotXPCOMBase. Correctly threw error: " + e);
+  }
+  strictEqual(xpcomObject.method3, undefined);
+}
+

xpcom/tests/unit/xpcomtest.manifest

@@ -0,0 +1 @@
+interfaces xpcomtest.xpt

xpcom/tests/unit/xpcshell.ini

@@ -5,6 +5,10 @@ support-files =
   bug725015.manifest
   compmgr_warnings.manifest
   data/**
+  xpcomtest.xpt
+  xpcomtest.manifest
+generated-files =
+  xpcomtest.xpt
 
 [test_bug121341.js]
 [test_bug325418.js]
@@ -64,3 +68,4 @@ skip-if = os == "win"
 # Bug 676998: test fails consistently on Android
 fail-if = os == "android"
 [test_file_renameTo.js]
+[test_notxpcom_scriptable.js]