Title: [180848] trunk/Source
Revision
180848
Author
simon.fra...@apple.com
Date
2015-02-28 20:36:39 -0800 (Sat, 28 Feb 2015)

Log 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]):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (180847 => 180848)


--- trunk/Source/WebCore/ChangeLog	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebCore/ChangeLog	2015-03-01 04:36:39 UTC (rev 180848)
@@ -1,3 +1,30 @@
+2015-02-28  Simon Fraser  <simon.fra...@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  <akl...@apple.com>
 
         [Cocoa] Purge SQLite page cache when under memory pressure.

Modified: trunk/Source/WebCore/page/FrameView.cpp (180847 => 180848)


--- trunk/Source/WebCore/page/FrameView.cpp	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebCore/page/FrameView.cpp	2015-03-01 04:36:39 UTC (rev 180848)
@@ -1288,8 +1288,6 @@
                         bodyRenderer->setChildNeedsLayout();
                     else if (rootRenderer && rootRenderer->stretchesToViewport())
                         rootRenderer->setChildNeedsLayout();
-
-                    document.updateViewportUnitsOnResize();
                 }
             }
 
@@ -2291,6 +2289,9 @@
 
 void FrameView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
+    if (Document* document = frame().document())
+        document->updateViewportUnitsOnResize();
+
     setNeedsLayout();
     ScrollView::availableContentSizeChanged(reason);
 }
@@ -4726,8 +4727,11 @@
     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 @@
     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();
 }
     

Modified: trunk/Source/WebCore/page/FrameView.h (180847 => 180848)


--- trunk/Source/WebCore/page/FrameView.h	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebCore/page/FrameView.h	2015-03-01 04:36:39 UTC (rev 180848)
@@ -488,6 +488,8 @@
     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 @@
 
     virtual void repaintContentRectangle(const IntRect&) override;
     virtual void updateContentsSize() override;
-    virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
     virtual void addedOrRemovedScrollbar() override;
 
     virtual void delegatesScrollingDidChange() override;

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (180847 => 180848)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2015-03-01 04:36:39 UTC (rev 180848)
@@ -366,6 +366,10 @@
 void ScrollView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
     ScrollableArea::availableContentSizeChanged(reason);
+
+    if (platformWidget())
+        return;
+
     if (reason != AvailableSizeChangeReason::ScrollbarsChanged)
         updateScrollbars(scrollOffset());
 }

Modified: trunk/Source/WebKit/mac/ChangeLog (180847 => 180848)


--- trunk/Source/WebKit/mac/ChangeLog	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebKit/mac/ChangeLog	2015-03-01 04:36:39 UTC (rev 180848)
@@ -1,3 +1,13 @@
+2015-02-28  Simon Fraser  <simon.fra...@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  <cdu...@apple.com>
 
         Rename DatabaseManager::manager() to DatabaseManager::singleton()

Modified: trunk/Source/WebKit/mac/WebView/WebFrameView.mm (180847 => 180848)


--- trunk/Source/WebKit/mac/WebView/WebFrameView.mm	2015-03-01 02:31:16 UTC (rev 180847)
+++ trunk/Source/WebKit/mac/WebView/WebFrameView.mm	2015-03-01 04:36:39 UTC (rev 180848)
@@ -351,7 +351,7 @@
         [[self _scrollView] setDrawsBackground:YES];
     if (Frame* coreFrame = [self _web_frame]) {
         if (FrameView* coreFrameView = coreFrame->view())
-            coreFrameView->setNeedsLayout();
+            coreFrameView->availableContentSizeChanged(ScrollableArea::AvailableSizeChangeReason::AreaSizeChanged);
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to