Issue #2135 - Bug 1393806/Part 3: Change dom::ReparentWrapper to take an ErrorResult

* Bug 1393806 (Part 4) landed as part of Issue #1118.
* Account for ReparentWrappersInSubtree, introduced in 1466991

dom/base/crashtests/1393806.html

@@ -0,0 +1,17 @@
+<html>
+  <head>
+    <script>
+      function fun_0() {
+        document.implementation.createDocument('', '', null).adoptNode(o2);
+      }
+
+      o1 = document.createElement('map');
+      o2 = document.createElement('iframe');
+      document.documentElement.appendChild(o1);
+      document.documentElement.appendChild(o2);
+      o1.textContent = 'x';
+      document.addEventListener('DOMNodeRemoved', fun_0, false);
+      o1.innerText = 'x';
+    </script>
+  </head>
+</html>

dom/base/crashtests/crashtests.list

@@ -209,6 +209,7 @@ load 1230422.html
 load 1251361.html
 load 1304437.html
 pref(clipboard.autocopy,true) load 1385272-1.html
+load 1393806.html
 pref(dom.webcomponents.enabled,true) load 1341693.html
 pref(dom.webcomponents.enabled,true) load 1419799.html
 pref(dom.webcomponents.enabled,false) load 1422931.html

dom/base/nsINode.cpp

@@ -1555,28 +1555,25 @@ nsINode::SetExplicitBaseURI(nsIURI* aURI)
   return rv;
 }
 
-static nsresult
+static void
-AdoptNodeIntoOwnerDoc(nsINode *aParent, nsINode *aNode)
+AdoptNodeIntoOwnerDoc(nsINode *aParent, nsINode *aNode, ErrorResult& aError)
 {
   NS_ASSERTION(!aNode->GetParentNode(),
                "Should have removed from parent already");
 
   nsIDocument *doc = aParent->OwnerDoc();
 
-  ErrorResult rv;
+  DebugOnly<nsINode*> adoptedNode = doc->AdoptNode(*aNode, aError);
-  nsINode* adoptedNode = doc->AdoptNode(*aNode, rv);
-  rv.WouldReportJSException();
-  if (NS_WARN_IF(rv.Failed())) {
-    return rv.StealNSResult();
-  }
 
-  NS_ASSERTION(aParent->OwnerDoc() == doc,
+#ifdef DEBUG
-               "ownerDoc chainged while adopting");
+  if (!aError.Failed()) {
-  NS_ASSERTION(adoptedNode == aNode, "Uh, adopt node changed nodes?");
+    MOZ_ASSERT(aParent->OwnerDoc() == doc,
-  NS_ASSERTION(aParent->OwnerDoc() == aNode->OwnerDoc(),
+               "ownerDoc changed while adopting");
+    MOZ_ASSERT(adoptedNode == aNode, "Uh, adopt node changed nodes?");
+    MOZ_ASSERT(aParent->OwnerDoc() == aNode->OwnerDoc(),
                "ownerDocument changed again after adopting!");
-
+  }
-  return NS_OK;
+#endif // DEBUG
 }
 
 static nsresult
@@ -1601,19 +1598,20 @@ ReparentWrappersInSubtree(nsIContent* aRoot)
 
   rootedGlobal = xpc::GetXBLScope(cx, rootedGlobal);
 
-  nsresult rv;
+  ErrorResult rv;
   JS::Rooted<JSObject*> reflector(cx);
   for (nsIContent* cur = aRoot; cur; cur = cur->GetNextNode(aRoot)) {
     if ((reflector = cur->GetWrapper())) {
       JSAutoCompartment ac(cx, reflector);
-      rv = ReparentWrapper(cx, reflector);
+      ReparentWrapper(cx, reflector, rv);
-      if NS_FAILED(rv) {
+      rv.WouldReportJSException();
+      if (rv.Failed()) {
         // We _could_ consider BlastSubtreeToPieces here, but it's not really
         // needed.  Having some nodes in here accessible to content while others
         // are not is probably OK.  We just need to fail out of the actual
         // insertion, so they're not in the DOM.  Returning a failure here will
         // do that.
-        return rv;
+        return rv.StealNSResult();
       }
     }
   }
@@ -1627,7 +1625,6 @@ nsINode::doInsertChildAt(nsIContent* aKid, uint32_t aIndex,
 {
   NS_PRECONDITION(!aKid->GetParentNode(),
                   "Inserting node that already has parent");
-  nsresult rv;
 
   // The id-handling code, and in the future possibly other code, need to
   // react to unexpected attribute changes.
@@ -1638,15 +1635,23 @@ nsINode::doInsertChildAt(nsIContent* aKid, uint32_t aIndex,
   mozAutoDocUpdate updateBatch(GetComposedDoc(), UPDATE_CONTENT_MODEL, aNotify);
 
   if (OwnerDoc() != aKid->OwnerDoc()) {
-    rv = AdoptNodeIntoOwnerDoc(this, aKid);
+    ErrorResult error;
-    NS_ENSURE_SUCCESS(rv, rv);
+    AdoptNodeIntoOwnerDoc(this, aKid, error);
+
+    // Need to WouldReportJSException() if our callee can throw a JS
+    // exception (which it can) and we're neither propagating the
+    // error out nor unconditionally suppressing it.
+    error.WouldReportJSException();
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
   }
 
   uint32_t childCount = aChildArray.ChildCount();
   NS_ENSURE_TRUE(aIndex <= childCount, NS_ERROR_ILLEGAL_VALUE);
   bool isAppend = (aIndex == childCount);
 
-  rv = aChildArray.InsertChildAt(aKid, aIndex);
+  nsresult rv = aChildArray.InsertChildAt(aKid, aIndex);
   NS_ENSURE_SUCCESS(rv, rv);
   if (aIndex == 0) {
     mFirstChild = aKid;
@@ -2483,7 +2488,7 @@ nsINode::ReplaceOrInsertBefore(bool aReplace, nsINode* aNewChild,
   // inserting them w/o calling AdoptNode().
   nsIDocument* doc = OwnerDoc();
   if (doc != newContent->OwnerDoc()) {
-    aError = AdoptNodeIntoOwnerDoc(this, aNewChild);
+    AdoptNodeIntoOwnerDoc(this, aNewChild, aError);
     if (aError.Failed()) {
       return nullptr;
     }

dom/base/nsNodeUtils.cpp

@@ -614,8 +614,8 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep,
       if ((wrapper = aNode->GetWrapper())) {
         MOZ_ASSERT(IsDOMObject(wrapper));
         JSAutoCompartment ac(cx, wrapper);
-        nsresult rv = ReparentWrapper(cx, wrapper);
+        ReparentWrapper(cx, wrapper, aError);
-        if (NS_FAILED(rv)) {
+        if (aError.Failed()) {
           if (wasRegistered) {
             aNode->OwnerDoc()->UnregisterActivityObserver(aNode->AsElement());
           }
@@ -623,7 +623,6 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep,
           if (wasRegistered) {
             aNode->OwnerDoc()->RegisterActivityObserver(aNode->AsElement());
           }
-          aError.Throw(rv);
           return nullptr;
         }
       }

dom/bindings/BindingUtils.cpp

@@ -2106,16 +2106,19 @@ DictionaryBase::AppendJSONToString(const char16_t* aJSONData,
   return true;
 }
 
-nsresult
+void
-ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
+ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg, ErrorResult& aError)
 {
   js::AssertSameCompartment(aCx, aObjArg);
 
+  aError.MightThrowJSException();
+
   // Check if we're anywhere near the stack limit before we reach the
   // transplanting code, since it has no good way to handle errors. This uses
   // the untrusted script limit, which is not strictly necessary since no
   // actual script should run.
-  JS_CHECK_RECURSION_CONSERVATIVE(aCx, return NS_ERROR_FAILURE);
+  // TODO: Make sure to retain 'onerror' if bug 1342439 lands.
+  JS_CHECK_RECURSION_CONSERVATIVE(aCx, aError.StealExceptionFromJSContext(aCx); return);
 
   JS::Rooted<JSObject*> aObj(aCx, aObjArg);
   const DOMJSClass* domClass = GetDOMClass(aObj);
@@ -2135,12 +2138,12 @@ ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
   JSCompartment* newCompartment = js::GetObjectCompartment(newParent);
   if (oldCompartment == newCompartment) {
     MOZ_ASSERT(oldParent == newParent);
-    return NS_OK;
+    return;
   }
 
   nsISupports* native = UnwrapDOMObjectToISupports(aObj);
   if (!native) {
-    return NS_OK;
+    return;
   }
 
   bool isProxy = js::IsProxy(aObj);
@@ -2156,12 +2159,14 @@ ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
 
   JS::Handle<JSObject*> proto = (domClass->mGetProto)(aCx);
   if (!proto) {
-    return NS_ERROR_FAILURE;
+    aError.StealExceptionFromJSContext(aCx);
+    return;
   }
 
   JS::Rooted<JSObject*> newobj(aCx, JS_CloneObject(aCx, aObj, proto));
   if (!newobj) {
-    return NS_ERROR_FAILURE;
+    aError.StealExceptionFromJSContext(aCx);
+    return;
   }
 
   JS::Rooted<JSObject*> propertyHolder(aCx);
@@ -2169,11 +2174,13 @@ ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
   if (copyFrom) {
     propertyHolder = JS_NewObjectWithGivenProto(aCx, nullptr, nullptr);
     if (!propertyHolder) {
-      return NS_ERROR_OUT_OF_MEMORY;
+      aError.StealExceptionFromJSContext(aCx);
+      return;
     }
 
     if (!JS_CopyPropertiesFrom(aCx, propertyHolder, copyFrom)) {
-      return NS_ERROR_FAILURE;
+      aError.StealExceptionFromJSContext(aCx);
+      return;
     }
   } else {
     propertyHolder = nullptr;
@@ -2189,7 +2196,8 @@ ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
   // if expandos are present then the wrapper will already have been preserved
   // for this native.
   if (!xpc::XrayUtils::CloneExpandoChain(aCx, newobj, aObj)) {
-    return NS_ERROR_FAILURE;
+    aError.StealExceptionFromJSContext(aCx);
+    return;
   }
 
   // We've set up |newobj|, so we make it own the native by setting its reserved
@@ -2244,9 +2252,6 @@ ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
   if (htmlobject) {
     htmlobject->SetupProtoChain(aCx, aObj);
   }
-
-  // Now we can just return the wrapper
-  return NS_OK;
 }
 
 GlobalObject::GlobalObject(JSContext* aCx, JSObject* aObject)

dom/bindings/BindingUtils.h

@@ -2758,8 +2758,8 @@ const nsAString& NonNullHelper(const binding_detail::FakeString& aArg)
 
 // Reparent the wrapper of aObj to whatever its native now thinks its
 // parent should be.
-nsresult
+void
-ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObj);
+ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObj, ErrorResult& aError);
 
 /**
  * Used to implement the Symbol.hasInstance property of an interface object.

dom/xml/XMLDocument.cpp

@@ -155,6 +155,8 @@ NS_NewDOMDocument(nsIDOMDocument** aInstancePtrResult,
   if (aDoctype) {
     nsCOMPtr<nsIDOMNode> tmpNode;
     rv = doc->AppendChild(aDoctype, getter_AddRefs(tmpNode));
+    // TODO: if we choose to land bug 1318479, make sure to call
+    // result.WouldReportJSException() before stealing the NSResult.
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -167,6 +169,8 @@ NS_NewDOMDocument(nsIDOMDocument** aInstancePtrResult,
     nsCOMPtr<nsIDOMNode> tmpNode;
 
     rv = doc->AppendChild(root, getter_AddRefs(tmpNode));
+    // TODO: if we choose to land bug 1318479, make sure to call
+    // result.WouldReportJSException() before stealing the NSResult.
     NS_ENSURE_SUCCESS(rv, rv);
   }