Title: [156072] trunk
Revision
156072
Author
rn...@webkit.org
Date
2013-09-18 16:58:54 -0700 (Wed, 18 Sep 2013)

Log Message

Merge HTMLBodyElement::didNotifySubtreeInsertions into HTMLBodyElement::insertedInto
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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (156071 => 156072)


--- trunk/LayoutTests/ChangeLog	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/LayoutTests/ChangeLog	2013-09-18 23:58:54 UTC (rev 156072)
@@ -1,3 +1,14 @@
+2013-09-18  Ryosuke Niwa  <rn...@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  <fpi...@apple.com>
 
         Unreviewed check in a proper baseline.

Modified: trunk/LayoutTests/inspector/timeline/timeline-script-tag-1-expected.txt (156071 => 156072)


--- trunk/LayoutTests/inspector/timeline/timeline-script-tag-1-expected.txt	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/LayoutTests/inspector/timeline/timeline-script-tag-1-expected.txt	2013-09-18 23:58:54 UTC (rev 156072)
@@ -3,11 +3,9 @@
 
 
 ParseHTML
-----> InvalidateLayout
 ParseHTML
 ----> EvaluateScript
 --------> TimeStamp : SCRIPT TAG
-----> InvalidateLayout
 EvaluateScript Properties:
 {
     children : <object>

Modified: trunk/Source/WebCore/ChangeLog (156071 => 156072)


--- trunk/Source/WebCore/ChangeLog	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/Source/WebCore/ChangeLog	2013-09-18 23:58:54 UTC (rev 156072)
@@ -1,3 +1,31 @@
+2013-09-18  Ryosuke Niwa  <rn...@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.no...@apple.com>
 
         [MSE] Throw exception when setting timestampOffset while 'updating' state is set.

Modified: trunk/Source/WebCore/html/HTMLBodyElement.cpp (156071 => 156072)


--- trunk/Source/WebCore/html/HTMLBodyElement.cpp	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/Source/WebCore/html/HTMLBodyElement.cpp	2013-09-18 23:58:54 UTC (rev 156072)
@@ -159,18 +159,14 @@
 Node::InsertionNotificationRequest HTMLBodyElement::insertedInto(ContainerNode* insertionPoint)
 {
     HTMLElement::insertedInto(insertionPoint);
-    if (insertionPoint->inDocument())
-        return InsertionShouldCallDidNotifySubtreeInsertions;
-    return InsertionDone;
-}
+    if (!insertionPoint->inDocument())
+        return InsertionDone;
 
-void HTMLBodyElement::didNotifySubtreeInsertions(ContainerNode* insertionPoint)
-{
-    ASSERT_UNUSED(insertionPoint, 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>.
     // 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 @@
             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

Modified: trunk/Source/WebCore/html/HTMLBodyElement.h (156071 => 156072)


--- trunk/Source/WebCore/html/HTMLBodyElement.h	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/Source/WebCore/html/HTMLBodyElement.h	2013-09-18 23:58:54 UTC (rev 156072)
@@ -75,8 +75,7 @@
     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;

Modified: trunk/Source/WebCore/html/HTMLFrameElementBase.h (156071 => 156072)


--- trunk/Source/WebCore/html/HTMLFrameElementBase.h	2013-09-18 23:51:43 UTC (rev 156071)
+++ trunk/Source/WebCore/html/HTMLFrameElementBase.h	2013-09-18 23:58:54 UTC (rev 156072)
@@ -81,9 +81,19 @@
     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);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to