Do not update attributes in HTMLBodyElement::insertedInto.

Use didNotifySubtreeInsertionsToDocument instead. BUG=356095 TEST=automated [email protected] Review URL: https://codereview.chromium.org/212793007 git-svn-id: svn://svn.chromium.org/blink/trunk@170216 bbb929c8-8fbe-4397-9dbb-9b2b20218538
yoavweiss · Mar 27, 2014 · b72d4e3 · b72d4e3
1 parent 11b4697
commit b72d4e3
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 16 deletions.
diff --git a/LayoutTests/fast/dom/HTMLBodyElement/body-inserting-iframe-crash-expected.txt b/LayoutTests/fast/dom/HTMLBodyElement/body-inserting-iframe-crash-expected.txt
@@ -0,0 +1 @@
+PASS if not crashed.
diff --git a/LayoutTests/fast/dom/HTMLBodyElement/body-inserting-iframe-crash.html b/LayoutTests/fast/dom/HTMLBodyElement/body-inserting-iframe-crash.html
@@ -0,0 +1,25 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var iframe = null;
+
+function start() {
+    iframe = document.createElement('iframe');
+    iframe.setAttribute('marginwidth', 1);
+    document.body.appendChild(iframe);
+
+    var bodyInFrame = document.createElement('body');
+    bodyInFrame.addEventListener('DOMSubtreeModified', removeIframe);
+    iframe.contentDocument.documentElement.appendChild(bodyInFrame);
+    document.body.innerHTML = 'PASS if not crashed.';
+}
+
+function removeIframe() {
+    iframe.parentNode.removeChild(iframe);
+    iframe = null;
+    gc();
+}
+</script>
+<body onload="start()">
+</body>
diff --git a/Source/core/html/HTMLBodyElement.cpp b/Source/core/html/HTMLBodyElement.cpp
@@ -155,22 +155,24 @@ void HTMLBodyElement::parseAttribute(const QualifiedName& name, const AtomicStri
 Node::InsertionNotificationRequest HTMLBodyElement::insertedInto(ContainerNode* insertionPoint)
 {
     HTMLElement::insertedInto(insertionPoint);
-    if (insertionPoint->inDocument()) {
-        // FIXME: It's surprising this is web compatible since it means a marginwidth
-        // and marginheight attribute can magically appear on the <body> of all documents
-        // embedded through <iframe> or <frame>.
-        Element* ownerElement = document().ownerElement();
-        if (isHTMLFrameElementBase(ownerElement)) {
-            HTMLFrameElementBase& ownerFrameElement = toHTMLFrameElementBase(*ownerElement);
-            int marginWidth = ownerFrameElement.marginWidth();
-            if (marginWidth != -1)
-                setIntegralAttribute(marginwidthAttr, marginWidth);
-            int marginHeight = ownerFrameElement.marginHeight();
-            if (marginHeight != -1)
-                setIntegralAttribute(marginheightAttr, marginHeight);
-        }
-    }
-    return InsertionDone;
+    return InsertionShouldCallDidNotifySubtreeInsertions;
+}
+
+void HTMLBodyElement::didNotifySubtreeInsertionsToDocument()
+{
+    // FIXME: It's surprising this is web compatible since it means a
+    // marginwidth and marginheight attribute can magically appear on the <body>
+    // of all documents embedded through <iframe> or <frame>.
+    Element* ownerElement = document().ownerElement();
+    if (!isHTMLFrameElementBase(ownerElement))
+        return;
+    HTMLFrameElementBase& ownerFrameElement = toHTMLFrameElementBase(*ownerElement);
+    int marginWidth = ownerFrameElement.marginWidth();
+    int marginHeight = ownerFrameElement.marginHeight();
+    if (marginWidth != -1)
+        setIntegralAttribute(marginwidthAttr, marginWidth);
+    if (marginHeight != -1)
+        setIntegralAttribute(marginheightAttr, marginHeight);
 }
 
 bool HTMLBodyElement::isURLAttribute(const Attribute& attribute) const

diff --git a/Source/core/html/HTMLBodyElement.h b/Source/core/html/HTMLBodyElement.h
@@ -51,6 +51,7 @@ class HTMLBodyElement FINAL : public HTMLElement {
     virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStylePropertySet*) OVERRIDE;
 
     virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
+    virtual void didNotifySubtreeInsertionsToDocument() OVERRIDE;
 
     virtual bool isURLAttribute(const Attribute&) const OVERRIDE;
     virtual bool hasLegalLinkAttribute(const QualifiedName&) const OVERRIDE;