Merge HTMLBodyElement::didNotifySubtreeInsertions into HTMLBodyElement::insertedInto
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Sep 2013 23:58:54 +0000 (23:58 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Sep 2013 23:58:54 +0000 (23:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121576

Reviewed by Andreas Kling.

Source/WebCore:

Merge https://chromium.googlesource.com/chromium/blink/+/2a9cac908f4eceadfcf9d21bdf5b3e598075aa1f

The logic in didNotifySubtreeInsertions to set the marginwidth and marginheight attributes
on the <body> of elements inside <iframe> and <frame> doesn't need to run after inserting
all the children of the frame. In fact this means that when you have those attributes
and then the script in the iframe touches offsetLeft or any layout dependent property
we'll layout with the wrong values and then have to do another layout after these margin
attributes are set.

I also remove the scheduleRelayout() call that was inside didNotifySubtreeInsertions. This
call doesn't make any sense, inserting a <body> will always trigger a style recalc and
a subsequent layout. The code is 9 years old: https://trac.webkit.org/changeset/8122
and all tests run fine without it.

* html/HTMLBodyElement.cpp:
(WebCore::HTMLBodyElement::insertedInto):
* html/HTMLBodyElement.h:
* html/HTMLFrameElementBase.h:
(WebCore::isHTMLFrameElementBase):
(WebCore::toHTMLFrameElementBase):

LayoutTests:

Rebaseline a test now that we don't do an extra layout.

* inspector/timeline/timeline-script-tag-1-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@156072 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/timeline/timeline-script-tag-1-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLBodyElement.cpp
Source/WebCore/html/HTMLBodyElement.h
Source/WebCore/html/HTMLFrameElementBase.h

index 6f515decca166d56e9dfe2b70de0745cfeefcd8f..82d722e2900303aaae54635748c159933b24b398 100644 (file)
@@ -1,3 +1,14 @@
+2013-09-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Merge HTMLBodyElement::didNotifySubtreeInsertions into HTMLBodyElement::insertedInto
+        https://bugs.webkit.org/show_bug.cgi?id=121576
+
+        Reviewed by Andreas Kling.
+
+        Rebaseline a test now that we don't do an extra layout.
+
+        * inspector/timeline/timeline-script-tag-1-expected.txt:
+
 2013-09-18  Filip Pizlo  <fpizlo@apple.com>
 
         Unreviewed check in a proper baseline.
index 717ebf563d00dd4f0070f30d7d19f1e99088bda1..11e455702a3fdf5811bf13da48ac84df382e13f1 100644 (file)
@@ -3,11 +3,9 @@ Tests the Timeline API instrumentation of an HTML script tag.
 
 
 ParseHTML
-----> InvalidateLayout
 ParseHTML
 ----> EvaluateScript
 --------> TimeStamp : SCRIPT TAG
-----> InvalidateLayout
 EvaluateScript Properties:
 {
     children : <object>
index bb295659dfe955ee1660129430b8a84797c85e04..05440503a5d9904bcf34861875303614b37e67cc 100644 (file)
@@ -1,3 +1,31 @@
+2013-09-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Merge HTMLBodyElement::didNotifySubtreeInsertions into HTMLBodyElement::insertedInto
+        https://bugs.webkit.org/show_bug.cgi?id=121576
+
+        Reviewed by Andreas Kling.
+
+        Merge https://chromium.googlesource.com/chromium/blink/+/2a9cac908f4eceadfcf9d21bdf5b3e598075aa1f
+
+        The logic in didNotifySubtreeInsertions to set the marginwidth and marginheight attributes
+        on the <body> of elements inside <iframe> and <frame> doesn't need to run after inserting
+        all the children of the frame. In fact this means that when you have those attributes
+        and then the script in the iframe touches offsetLeft or any layout dependent property
+        we'll layout with the wrong values and then have to do another layout after these margin
+        attributes are set.
+
+        I also remove the scheduleRelayout() call that was inside didNotifySubtreeInsertions. This
+        call doesn't make any sense, inserting a <body> will always trigger a style recalc and
+        a subsequent layout. The code is 9 years old: https://trac.webkit.org/changeset/8122
+        and all tests run fine without it.
+
+        * html/HTMLBodyElement.cpp:
+        (WebCore::HTMLBodyElement::insertedInto):
+        * html/HTMLBodyElement.h:
+        * html/HTMLFrameElementBase.h:
+        (WebCore::isHTMLFrameElementBase):
+        (WebCore::toHTMLFrameElementBase):
+
 2013-09-18  Jer Noble  <jer.noble@apple.com>
 
         [MSE] Throw exception when setting timestampOffset while 'updating' state is set.
index 45706304ad0fe5fb83036332c43a52e5ca3dbc0b..c2ce03f4723eae6b2fcfe2f3ac3b0409ccb7d134 100644 (file)
@@ -159,18 +159,14 @@ void HTMLBodyElement::parseAttribute(const QualifiedName& name, const AtomicStri
 Node::InsertionNotificationRequest HTMLBodyElement::insertedInto(ContainerNode* insertionPoint)
 {
     HTMLElement::insertedInto(insertionPoint);
-    if (insertionPoint->inDocument())
-        return InsertionShouldCallDidNotifySubtreeInsertions;
-    return InsertionDone;
-}
-
-void HTMLBodyElement::didNotifySubtreeInsertions(ContainerNode* insertionPoint)
-{
-    ASSERT_UNUSED(insertionPoint, insertionPoint->inDocument());
+    if (!insertionPoint->inDocument())
+        return InsertionDone;
 
+    // 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>.
     // FIXME: Perhaps this code should be in attach() instead of here.
     Element* ownerElement = document().ownerElement();
-    if (ownerElement && (ownerElement->hasTagName(frameTag) || ownerElement->hasTagName(iframeTag))) {
+    if (ownerElement && isHTMLFrameElementBase(ownerElement)) {
         HTMLFrameElementBase* ownerFrameElement = toHTMLFrameElementBase(ownerElement);
         int marginWidth = ownerFrameElement->marginWidth();
         if (marginWidth != -1)
@@ -180,10 +176,7 @@ void HTMLBodyElement::didNotifySubtreeInsertions(ContainerNode* insertionPoint)
             setAttribute(marginheightAttr, String::number(marginHeight));
     }
 
-    // FIXME: This call to scheduleRelayout should not be needed here.
-    // But without it we hang during WebKit tests; need to fix that and remove this.
-    if (FrameView* view = document().view())
-        view->scheduleRelayout();
+    return InsertionDone;
 }
 
 bool HTMLBodyElement::isURLAttribute(const Attribute& attribute) const
index 0f58effffca6a6480ae2470d36a3e23ce65192b8..830f916aba77ebf480b5ca3466a78672c88c2a7b 100644 (file)
@@ -75,8 +75,7 @@ private:
     virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStylePropertySet*) OVERRIDE;
 
     virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
-    virtual void didNotifySubtreeInsertions(ContainerNode*) OVERRIDE;
-    
+
     virtual bool isURLAttribute(const Attribute&) const OVERRIDE;
     
     virtual bool supportsFocus() const OVERRIDE;
index dab43340601b70df467a5213bca9de34dff48b69..31a54f715b960becde49822565ac408ab026e829 100644 (file)
@@ -81,9 +81,19 @@ private:
     bool m_viewSource;
 };
 
+inline bool isHTMLFrameElementBase(const Node* node)
+{
+    return isHTMLFrameElement(node) || isHTMLIFrameElement(node);
+}
+
+inline bool isHTMLFrameElementBase(const Element* element)
+{
+    return isHTMLFrameElement(element) || isHTMLIFrameElement(element);
+}
+
 inline HTMLFrameElementBase* toHTMLFrameElementBase(Node* node)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(!node || isHTMLFrameElement(node) || isHTMLIFrameElement(node));
+    ASSERT_WITH_SECURITY_IMPLICATION(!node || isHTMLFrameElementBase(node));
     return static_cast<HTMLFrameElementBase*>(node);
 }