Bug 862092 - "Assertion failure: target->isNative() == obj->isNative()" adopting a <select>. r=bz.
authorPeter Van der Beken <peterv@propagandism.org>
Tue, 16 Apr 2013 19:02:57 +0200
changeset 129321 f5ba9622f687c9d52b9adffe5fc7b1cc3d81ac82
parent 129320 48595e0d85d53fea285e206c61d4c4ec9f3fecf7
child 129322 39615ca8ab61c0ef1d23a0666492a4884eb72b11
push idunknown
push userunknown
push dateunknown
reviewersbz
bugs862092
milestone23.0a1
Bug 862092 - "Assertion failure: target->isNative() == obj->isNative()" adopting a <select>. r=bz.
dom/bindings/BindingUtils.cpp
dom/bindings/DOMJSProxyHandler.cpp
dom/bindings/DOMJSProxyHandler.h
dom/bindings/crashtests/862092.html
dom/bindings/crashtests/crashtests.list
dom/bindings/test/Makefile.in
dom/bindings/test/test_bug862092.html
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1464,16 +1464,22 @@ ReparentWrapper(JSContext* aCx, JS::Hand
   // Before proceeding, eagerly create any same-compartment security wrappers
   // that the object might have. This forces us to take the 'WithWrapper' path
   // while transplanting that handles this stuff correctly.
   JSObject* ww = xpc::WrapperFactory::WrapForSameCompartment(aCx, aObj);
   if (!ww) {
     return NS_ERROR_FAILURE;
   }
 
+  bool isProxy = js::IsProxy(aObj);
+  JSObject* expandoObject;
+  if (isProxy) {
+    expandoObject = DOMProxyHandler::GetAndClearExpandoObject(aObj);
+  }
+
   JSAutoCompartment newAc(aCx, newParent);
 
   // First we clone the reflector. We get a copy of its properties and clone its
   // expando chain. The only part that is dangerous here is that if we have to
   // return early we must avoid ending up with two reflectors pointing to the
   // same native. Other than that, the objects we create will just go away.
 
   JSObject *proto =
@@ -1492,28 +1498,33 @@ ReparentWrapper(JSContext* aCx, JS::Hand
                       js::GetReservedSlot(aObj, DOM_OBJECT_SLOT));
 
   // At this point, both |aObj| and |newobj| point to the same native
   // which is bad, because one of them will end up being finalized with a
   // native it does not own. |cloneGuard| ensures that if we exit before
   // clearing |aObj|'s reserved slot the reserved slot of |newobj| will be
   // set to null. |aObj| will go away soon, because we swap it with
   // another object during the transplant and let that object die.
-  JSObject *propertyHolder;
+  JSObject* propertyHolder;
   {
     AutoCloneDOMObjectSlotGuard cloneGuard(aObj, newobj);
 
-    propertyHolder = JS_NewObjectWithGivenProto(aCx, nullptr, nullptr,
-                                                newParent);
-    if (!propertyHolder) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
+    JSObject* copyFrom = isProxy ? expandoObject : aObj;
+    if (copyFrom) {
+      propertyHolder = JS_NewObjectWithGivenProto(aCx, nullptr, nullptr,
+                                                  newParent);
+      if (!propertyHolder) {
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
 
-    if (!JS_CopyPropertiesFrom(aCx, propertyHolder, aObj)) {
-      return NS_ERROR_FAILURE;
+      if (!JS_CopyPropertiesFrom(aCx, propertyHolder, copyFrom)) {
+        return NS_ERROR_FAILURE;
+      }
+    } else {
+      propertyHolder = nullptr;
     }
 
     // Expandos from other compartments are attached to the target JS object.
     // Copy them over, and let the old ones die a natural death.
     SetXrayExpandoChain(newobj, nullptr);
     if (!xpc::XrayUtils::CloneExpandoChain(aCx, newobj, aObj)) {
       return NS_ERROR_FAILURE;
     }
@@ -1552,18 +1563,28 @@ ReparentWrapper(JSContext* aCx, JS::Hand
       MOZ_CRASH();
     }
   }
 
   bool preserving = cache->PreservingWrapper();
   cache->SetPreservingWrapper(false);
   cache->SetWrapper(aObj);
   cache->SetPreservingWrapper(preserving);
-  if (!JS_CopyPropertiesFrom(aCx, aObj, propertyHolder)) {
-    MOZ_CRASH();
+
+  if (propertyHolder) {
+    JSObject* copyTo;
+    if (isProxy) {
+      copyTo = DOMProxyHandler::EnsureExpandoObject(aCx, aObj);
+    } else {
+      copyTo = aObj;
+    }
+
+    if (!copyTo || !JS_CopyPropertiesFrom(aCx, copyTo, propertyHolder)) {
+      MOZ_CRASH();
+    }
   }
 
   nsObjectLoadingContent* htmlobject;
   nsresult rv = UnwrapObject<HTMLObjectElement>(aCx, aObj, htmlobject);
   if (NS_FAILED(rv)) {
     rv = UnwrapObject<prototypes::id::HTMLEmbedElement,
                       HTMLSharedObjectElement>(aCx, aObj, htmlobject);
     if (NS_FAILED(rv)) {
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -42,16 +42,26 @@ struct SetListBaseInformation
 {
   SetListBaseInformation() {
     js::SetListBaseInformation((void*) &HandlerFamily, js::JSSLOT_PROXY_EXTRA + JSPROXYSLOT_EXPANDO);
   }
 };
 
 SetListBaseInformation gSetListBaseInformation;
 
+// static
+JSObject*
+DOMProxyHandler::GetAndClearExpandoObject(JSObject* obj)
+{
+  JSObject* expando = GetExpandoObject(obj);
+  XPCWrappedNativeScope* scope = xpc::GetObjectScope(obj);
+  scope->RemoveDOMExpandoObject(obj);
+  js::SetProxyExtra(obj, JSPROXYSLOT_EXPANDO, UndefinedValue());
+  return expando;
+}
 
 // static
 JSObject*
 DOMProxyHandler::EnsureExpandoObject(JSContext* cx, JSObject* obj)
 {
   NS_ASSERTION(IsDOMProxy(obj), "expected a DOM proxy object");
   JSObject* expando = GetExpandoObject(obj);
   if (!expando) {
--- a/dom/bindings/DOMJSProxyHandler.h
+++ b/dom/bindings/DOMJSProxyHandler.h
@@ -49,16 +49,17 @@ public:
   using js::BaseProxyHandler::obj_toString;
 
   static JSObject* GetExpandoObject(JSObject* obj)
   {
     MOZ_ASSERT(IsDOMProxy(obj), "expected a DOM proxy object");
     JS::Value v = js::GetProxyExtra(obj, JSPROXYSLOT_EXPANDO);
     return v.isUndefined() ? NULL : v.toObjectOrNull();
   }
+  static JSObject* GetAndClearExpandoObject(JSObject* obj);
   static JSObject* EnsureExpandoObject(JSContext* cx, JSObject* obj);
 
   const DOMClass& mClass;
 
 protected:
   static JSString* obj_toString(JSContext* cx, const char* className);
 
   // Append the property names in "names" that don't live on our proto
new file mode 100644
--- /dev/null
+++ b/dom/bindings/crashtests/862092.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<script>
+
+function boom()
+{
+  var frameDoc = document.getElementById("f").contentDocument;
+  frameDoc.adoptNode(document.createElement("select"));
+}
+
+</script>
+</head>
+
+<body onload="boom();">
+<iframe id="f"></iframe>
+</body>
+</html>
--- a/dom/bindings/crashtests/crashtests.list
+++ b/dom/bindings/crashtests/crashtests.list
@@ -1,7 +1,8 @@
 asserts-if(cocoaWidget,0-1) load 769464.html
 load 822340-1.html
 load 822340-2.html
 load 832899.html
 load 860591.html
 load 860551.html
 load 862610.html
+load 862092.html
--- a/dom/bindings/test/Makefile.in
+++ b/dom/bindings/test/Makefile.in
@@ -67,16 +67,17 @@ MOCHITEST_FILES := \
   file_bug775543.html \
   test_bug788369.html \
   test_bug742191.html \
   test_namedNoIndexed.html \
   test_bug759621.html \
   test_queryInterface.html \
   test_exceptionThrowing.html \
   test_bug852846.html \
+  test_bug862092.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
   test_bug775543.html \
   $(NULL)
 
 ifdef GNU_CC
 CXXFLAGS += -Wno-uninitialized
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_bug862092.html
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=862092
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 862092</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 862092 **/
+
+  SimpleTest.waitForExplicitFinish();
+  function runTest()
+  {
+    var frameDoc = document.getElementById("f").contentDocument;
+    var a = document.createElement("select");
+    a.expando = "test";
+    a = frameDoc.adoptNode(a)
+    is(a.expando, "test", "adoptNode needs to preserve expandos");
+    SimpleTest.finish();
+  }
+
+  </script>
+</head>
+<body onload="runTest();">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=862092">Mozilla Bug 862092</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="f"></iframe>
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>