Viewport units should not dirty style just before we do layout
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Mar 2015 04:36:39 +0000 (04:36 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 1 Mar 2015 04:36:39 +0000 (04:36 +0000)
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]):

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/ScrollView.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebFrameView.mm

index 9bf3497..7712ad2 100644 (file)
@@ -1,3 +1,30 @@
+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>
 
         [Cocoa] Purge SQLite page cache when under memory pressure.
index be247eb..c1929dd 100644 (file)
@@ -1288,8 +1288,6 @@ void FrameView::layout(bool allowSubtree)
                         bodyRenderer->setChildNeedsLayout();
                     else if (rootRenderer && rootRenderer->stretchesToViewport())
                         rootRenderer->setChildNeedsLayout();
-
-                    document.updateViewportUnitsOnResize();
                 }
             }
 
@@ -2291,6 +2289,9 @@ bool FrameView::renderedCharactersExceed(unsigned threshold)
 
 void FrameView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
+    if (Document* document = frame().document())
+        document->updateViewportUnitsOnResize();
+
     setNeedsLayout();
     ScrollView::availableContentSizeChanged(reason);
 }
@@ -4726,8 +4727,11 @@ void FrameView::setViewportSizeForCSSViewportUnits(IntSize size)
     m_overrideViewportSize = size;
     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);
+    }
 }
     
 IntSize FrameView::viewportSizeForCSSViewportUnits() const
@@ -4735,6 +4739,8 @@ IntSize FrameView::viewportSizeForCSSViewportUnits() const
     if (m_hasOverrideViewportSize)
         return m_overrideViewportSize;
     
+    // FIXME: the value returned should take into account the value of the overflow
+    // property on the root element.
     return visibleContentRectIncludingScrollbars().size();
 }
     
index 969c009..3cd219e 100644 (file)
@@ -488,6 +488,8 @@ public:
     WEBCORE_EXPORT virtual void willStartLiveResize() override;
     WEBCORE_EXPORT virtual void willEndLiveResize() override;
 
+    WEBCORE_EXPORT virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
+
     void addPaintPendingMilestones(LayoutMilestones);
     void firePaintRelatedMilestonesIfNeeded();
     void fireLayoutRelatedMilestonesIfNeeded();
@@ -581,7 +583,6 @@ private:
 
     virtual void repaintContentRectangle(const IntRect&) override;
     virtual void updateContentsSize() override;
-    virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
     virtual void addedOrRemovedScrollbar() override;
 
     virtual void delegatesScrollingDidChange() override;
index 7fcd12b..6bdc90e 100644 (file)
@@ -366,6 +366,10 @@ void ScrollView::setUseFixedLayout(bool enable)
 void ScrollView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
     ScrollableArea::availableContentSizeChanged(reason);
+
+    if (platformWidget())
+        return;
+
     if (reason != AvailableSizeChangeReason::ScrollbarsChanged)
         updateScrollbars(scrollOffset());
 }
index 176d0f3..9c2bf46 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Rename DatabaseManager::manager() to DatabaseManager::singleton()
index a51d3e8..d06aec3 100644 (file)
@@ -351,7 +351,7 @@ static inline void addTypesFromClass(NSMutableDictionary *allTypes, Class objCCl
         [[self _scrollView] setDrawsBackground:YES];
     if (Frame* coreFrame = [self _web_frame]) {
         if (FrameView* coreFrameView = coreFrame->view())
-            coreFrameView->setNeedsLayout();
+            coreFrameView->availableContentSizeChanged(ScrollableArea::AvailableSizeChangeReason::AreaSizeChanged);
     }
 }