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 141102 f5ba9622f687c9d52b9adffe5fc7b1cc3d81ac82
parent 141101 48595e0d85d53fea285e206c61d4c4ec9f3fecf7
child 141103 39615ca8ab61c0ef1d23a0666492a4884eb72b11
push id350
push userbbajaj@mozilla.com
push dateMon, 29 Jul 2013 23:00:49 +0000
treeherdermozilla-release@064965b37dbd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs862092
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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>