Changeset 180848 in webkit

Timestamp:

Feb 28, 2015 8:36:39 PM (9 years ago)

Author:

Simon Fraser

Message:

Viewport units should not dirty style just before we do layout
​https://bugs.webkit.org/show_bug.cgi?id=141682

Reviewed by Zalan Bujtas.
Source/WebCore:

In documents using viewport units, we dirtied style every time layout changed
the size of the document. This is nonsensical, because viewport units depend on the
viewport size, not the document size.

Move the style dirtying from layout() into availableContentSizeChanged(). Hook
this up for WebKit1 by calling from -[WebFrameView _frameSizeChanged], and,
since that causes availableContentSizeChanged() to be called for WK1 for the first
time, protect the call to updateScrollbars() with a !platformWidget check.

Covered by existing viewport unit tests.

• page/FrameView.cpp:

(WebCore::FrameView::layout):
(WebCore::FrameView::availableContentSizeChanged):
(WebCore::FrameView::viewportSizeForCSSViewportUnits): Add a FIXME comment. Whether
scrollbars are ignored depends on the value of the overflow property on the root element.

• page/FrameView.h:
• platform/ScrollView.cpp:

(WebCore::ScrollView::availableContentSizeChanged):

Source/WebKit/mac:

• WebView/WebFrameView.mm:

(-[WebFrameView _frameSizeChanged]):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (4 diffs)
• WebCore/page/FrameView.h (modified) (2 diffs)
• WebCore/platform/ScrollView.cpp (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebView/WebFrameView.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-02-28  Simon Fraser  <simon.fraser@apple.com>
+
+        Viewport units should not dirty style just before we do layout
+        https://bugs.webkit.org/show_bug.cgi?id=141682
+
+        Reviewed by Zalan Bujtas.
+        
+        In documents using viewport units, we dirtied style every time layout changed
+        the size of the document. This is nonsensical, because viewport units depend on the
+        viewport size, not the document size.
+        
+        Move the style dirtying from layout() into availableContentSizeChanged(). Hook
+        this up for WebKit1 by calling from -[WebFrameView _frameSizeChanged], and,
+        since that causes availableContentSizeChanged() to be called for WK1 for the first
+        time, protect the call to updateScrollbars() with a !platformWidget check.
+
+        Covered by existing viewport unit tests.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout):
+        (WebCore::FrameView::availableContentSizeChanged):
+        (WebCore::FrameView::viewportSizeForCSSViewportUnits): Add a FIXME comment. Whether
+        scrollbars are ignored depends on the value of the overflow property on the root element.
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::availableContentSizeChanged):
+
 2015-02-28  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -1289 +1289 @@
                     else if (rootRenderer && rootRenderer->stretchesToViewport())
                         rootRenderer->setChildNeedsLayout();
-
-                    document.updateViewportUnitsOnResize();
                 }
             }
@@ -2292 +2290 @@
 void FrameView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
+    if (Document* document = frame().document())
+        document->updateViewportUnitsOnResize();
+
     setNeedsLayout();
     ScrollView::availableContentSizeChanged(reason);
@@ -4727 +4728 @@
     m_hasOverrideViewportSize = true;
     
-    if (Document* document = m_frame->document())
+    if (Document* document = m_frame->document()) {
+        // FIXME: this should probably be updateViewportUnitsOnResize(), but synchronously
+        // dirtying style here causes assertions on iOS (rdar://problem/19998166).
         document->styleResolverChanged(DeferRecalcStyle);
+    }
 }
     
@@ -4736 +4740 @@
         return m_overrideViewportSize;
     
+    // FIXME: the value returned should take into account the value of the overflow
+    // property on the root element.
     return visibleContentRectIncludingScrollbars().size();
 }

trunk/Source/WebCore/page/FrameView.h

@@ -489 +489 @@
     WEBCORE_EXPORT virtual void willEndLiveResize() override;
 
+    WEBCORE_EXPORT virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
+
     void addPaintPendingMilestones(LayoutMilestones);
     void firePaintRelatedMilestonesIfNeeded();
@@ -582 +584 @@
     virtual void repaintContentRectangle(const IntRect&) override;
     virtual void updateContentsSize() override;
-    virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
     virtual void addedOrRemovedScrollbar() override;
 

trunk/Source/WebCore/platform/ScrollView.cpp

@@ -367 +367 @@
 {
     ScrollableArea::availableContentSizeChanged(reason);
+
+    if (platformWidget())
+        return;
+
     if (reason != AvailableSizeChangeReason::ScrollbarsChanged)
         updateScrollbars(scrollOffset());

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2015-02-28  Simon Fraser  <simon.fraser@apple.com>
+
+        Viewport units should not dirty style just before we do layout
+        https://bugs.webkit.org/show_bug.cgi?id=141682
+
+        Reviewed by Zalan Bujtas.
+
+        * WebView/WebFrameView.mm:
+        (-[WebFrameView _frameSizeChanged]):
+
 2015-02-26  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/mac/WebView/WebFrameView.mm

@@ -352 +352 @@
     if (Frame* coreFrame = [self _web_frame]) {
         if (FrameView* coreFrameView = coreFrame->view())
-            coreFrameView->setNeedsLayout();
+            coreFrameView->availableContentSizeChanged(ScrollableArea::AvailableSizeChangeReason::AreaSizeChanged);
     }
 }