# HG changeset patch
# User Boris Zbarsky <bzbarsky@mit.edu>
# Date 1515784974 18000
#      Fri Jan 12 14:22:54 2018 -0500
# Node ID 5afb960ffb089f3964ef13ef192c73ccb9b5a46f
# Parent  99f4647a241e0d20b5cefe34f8fed063f3852280
Bug 1430164.  Stop doing weird sandbox-callable-wrapping for DOM constructors. r=bholley

This fixes DOM constructor identity in web extension content scripts to work as expected.

MozReview-Commit-ID: Ab4sFWiMU6c

diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -2612,16 +2612,30 @@ inline bool
 UseDOMXray(JSObject* obj)
 {
   const js::Class* clasp = js::GetObjectClass(obj);
   return IsDOMClass(clasp) ||
          JS_IsNativeFunction(obj, Constructor) ||
          IsDOMIfaceAndProtoClass(clasp);
 }
 
+inline bool
+IsDOMConstructor(JSObject* obj)
+{
+  if (JS_IsNativeFunction(obj, dom::Constructor)) {
+    // NamedConstructor, like Image
+    return true;
+  }
+
+  const js::Class* clasp = js::GetObjectClass(obj);
+  // Check for a DOM interface object.
+  return dom::IsDOMIfaceAndProtoClass(clasp) &&
+         dom::DOMIfaceAndProtoJSClass::FromJSClass(clasp)->mType == dom::eInterface;
+}
+
 #ifdef DEBUG
 inline bool
 HasConstructor(JSObject* obj)
 {
   return JS_IsNativeFunction(obj, Constructor) ||
          js::GetObjectClass(obj)->getConstruct();
 }
 #endif
diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -751,16 +751,29 @@ bool WrapAccessorFunction(JSContext* cx,
     RootedObject func(cx, JS_FUNC_TO_DATA_PTR(JSObject*, op));
     func = WrapCallable(cx, func, sandboxProtoProxy);
     if (!func)
         return false;
     op = JS_DATA_TO_FUNC_PTR(Op, func.get());
     return true;
 }
 
+static bool
+IsMaybeWrappedDOMConstructor(JSObject* obj)
+{
+    // We really care about the underlying object here, which might be wrapped
+    // in cross-compartment wrappers.
+    obj = js::CheckedUnwrap(obj);
+    if (!obj) {
+        return false;
+    }
+
+    return dom::IsDOMConstructor(obj);
+}
+
 bool
 xpc::SandboxProxyHandler::getPropertyDescriptor(JSContext* cx,
                                                 JS::Handle<JSObject*> proxy,
                                                 JS::Handle<jsid> id,
                                                 JS::MutableHandle<PropertyDescriptor> desc) const
 {
     JS::RootedObject obj(cx, wrappedObject(proxy));
 
@@ -775,17 +788,21 @@ xpc::SandboxProxyHandler::getPropertyDes
     if (!WrapAccessorFunction(cx, desc.getter(), desc.address(),
                               JSPROP_GETTER, proxy))
         return false;
     if (!WrapAccessorFunction(cx, desc.setter(), desc.address(),
                               JSPROP_SETTER, proxy))
         return false;
     if (desc.value().isObject()) {
         RootedObject val (cx, &desc.value().toObject());
-        if (JS::IsCallable(val)) {
+        if (JS::IsCallable(val) &&
+            // Don't wrap DOM constructors: they don't care about the "this"
+            // they're invoked with anyway, being constructors.  And if we wrap
+            // them here we break invariants like Node == Node and whatnot.
+            !IsMaybeWrappedDOMConstructor(val)) {
             val = WrapCallable(cx, val, proxy);
             if (!val)
                 return false;
             desc.value().setObject(*val);
         }
     }
 
     return true;
diff --git a/js/xpconnect/tests/chrome/chrome.ini b/js/xpconnect/tests/chrome/chrome.ini
--- a/js/xpconnect/tests/chrome/chrome.ini
+++ b/js/xpconnect/tests/chrome/chrome.ini
@@ -84,16 +84,17 @@ skip-if = os == 'win' || os == 'mac' || 
 [test_bug1050049.html]
 [test_bug1065185.html]
 [test_bug1074863.html]
 [test_bug1092477.xul]
 [test_bug1124898.html]
 [test_bug1126911.html]
 [test_bug1281071.xul]
 [test_bug1390159.xul]
+[test_bug1430164.html]
 [test_chrometoSource.xul]
 [test_cloneInto.xul]
 [test_cows.xul]
 [test_discardSystemSource.xul]
 [test_documentdomain.xul]
 [test_doublewrappedcompartments.xul]
 [test_evalInSandbox.xul]
 [test_evalInWindow.xul]
diff --git a/js/xpconnect/tests/chrome/test_bug1430164.html b/js/xpconnect/tests/chrome/test_bug1430164.html
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug1430164.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1430164
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1430164</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://global/skin"/>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+  /** Test for Bug 1430164 **/
+      var Cu = Components.utils;
+      var s = new Cu.Sandbox(window, { sandboxPrototype: window } );
+      var t;
+      var desc;
+      try {
+          t = Cu.evalInSandbox('Node === Node', s);
+          ok(t, "Object identity should be sane");
+
+          t = Cu.evalInSandbox('Node === this.Node', s);
+          ok(t, "Node should match this.Node in toplevel");
+
+          t = Cu.evalInSandbox('Node === window.Node', s);
+          ok(t, "Node should match window.Node");
+      } catch (e) {
+          ok(false, "Should not get an exception: " + e);
+      }
+  </script>
+</head>
+</html>