Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove update...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 May 2016 22:19:29 +0000 (22:19 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 May 2016 22:19:29 +0000 (22:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157277

Reviewed by Dean Jackson, Tim Horton.

Source/WebCore:

shouldUpdateScrollLayerPositionSynchronously() gave an answer for the main frame even if
called for a subframe. This could have caused scroll snapping, and isRubberBandInProgress()
to give wrong answers sometimes. Fix by passing in the FrameView.

I was unable to easily come up with a testcase to detect the incorrect behavior.

Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.

* page/FrameView.cpp:
(WebCore::FrameView::isScrollSnapInProgress):
(WebCore::FrameView::shouldUpdateCompositingLayersAfterScrolling):
(WebCore::FrameView::isRubberBandInProgress):
(WebCore::FrameView::updatesScrollLayerPositionOnMainThread): Deleted.
* page/FrameView.h:
* page/scrolling/ScrollingCoordinator.cpp:
(WebCore::ScrollingCoordinator::updateSynchronousScrollingReasons):
(WebCore::ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously):
* page/scrolling/ScrollingCoordinator.h:
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::updateTiledScrollingIndicator):
* platform/ScrollableArea.h:
* platform/win/PopupMenuWin.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::setupFontSubpixelQuantization):
* rendering/RenderLayer.h:
* rendering/RenderListBox.h:

Source/WebKit2:

Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.

* WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h:

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

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Source/WebCore/page/scrolling/ScrollingCoordinator.h
Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
Source/WebCore/platform/ScrollableArea.h
Source/WebCore/platform/win/PopupMenuWin.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderListBox.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h

index cdb36a3..7d8d352 100644 (file)
@@ -1,5 +1,39 @@
 2016-05-02  Simon Fraser  <simon.fraser@apple.com>
 
+        Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove updatesScrollLayerPositionOnMainThread()
+        https://bugs.webkit.org/show_bug.cgi?id=157277
+
+        Reviewed by Dean Jackson, Tim Horton.
+
+        shouldUpdateScrollLayerPositionSynchronously() gave an answer for the main frame even if
+        called for a subframe. This could have caused scroll snapping, and isRubberBandInProgress()
+        to give wrong answers sometimes. Fix by passing in the FrameView.
+
+        I was unable to easily come up with a testcase to detect the incorrect behavior.
+
+        Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::isScrollSnapInProgress):
+        (WebCore::FrameView::shouldUpdateCompositingLayersAfterScrolling):
+        (WebCore::FrameView::isRubberBandInProgress):
+        (WebCore::FrameView::updatesScrollLayerPositionOnMainThread): Deleted.
+        * page/FrameView.h:
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::updateSynchronousScrollingReasons):
+        (WebCore::ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously):
+        * page/scrolling/ScrollingCoordinator.h:
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::updateTiledScrollingIndicator):
+        * platform/ScrollableArea.h:
+        * platform/win/PopupMenuWin.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::setupFontSubpixelQuantization):
+        * rendering/RenderLayer.h:
+        * rendering/RenderListBox.h:
+
+2016-05-02  Simon Fraser  <simon.fraser@apple.com>
+
         Add to the Animations log channel output about which properties are being blended
         https://bugs.webkit.org/show_bug.cgi?id=157271
 
index 9e08972..edfc3cf 100644 (file)
@@ -970,7 +970,7 @@ bool FrameView::isScrollSnapInProgress() const
     // ScrollingCoordinator::isScrollSnapInProgress().
     if (Page* page = frame().page()) {
         if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
-            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
+            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
                 return scrollingCoordinator->isScrollSnapInProgress();
         }
     }
@@ -2307,7 +2307,7 @@ bool FrameView::shouldUpdateCompositingLayersAfterScrolling() const
     if (!scrollingCoordinator->supportsFixedPositionLayers())
         return true;
 
-    if (scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
+    if (scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
         return true;
 
     if (inProgrammaticScroll())
@@ -2340,7 +2340,7 @@ bool FrameView::isRubberBandInProgress() const
     // ScrollingCoordinator::isRubberBandInProgress().
     if (Page* page = frame().page()) {
         if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
-            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
+            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
                 return scrollingCoordinator->isRubberBandInProgress();
         }
     }
@@ -3527,16 +3527,6 @@ bool FrameView::isActive() const
     return page && page->focusController().isActive();
 }
 
-bool FrameView::updatesScrollLayerPositionOnMainThread() const
-{
-    if (Page* page = frame().page()) {
-        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-            return scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously();
-    }
-
-    return true;
-}
-
 bool FrameView::forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const
 {
     Page* page = frame().page();
index ac373b1..290c9ee 100644 (file)
@@ -486,7 +486,6 @@ public:
 #endif
 
     bool isActive() const override;
-    bool updatesScrollLayerPositionOnMainThread() const override;
     bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
 
 #if ENABLE(RUBBER_BANDING)
index 53b98c1..6cba262 100644 (file)
@@ -347,7 +347,7 @@ SynchronousScrollingReasons ScrollingCoordinator::synchronousScrollingReasons(co
     return synchronousScrollingReasons;
 }
 
-void ScrollingCoordinator::updateSynchronousScrollingReasons(FrameView& frameView)
+void ScrollingCoordinator::updateSynchronousScrollingReasons(const FrameView& frameView)
 {
     // FIXME: Once we support async scrolling of iframes, we'll have to track the synchronous scrolling
     // reasons per frame (maybe on scrolling tree nodes).
@@ -367,10 +367,11 @@ void ScrollingCoordinator::setForceSynchronousScrollLayerPositionUpdates(bool fo
         updateSynchronousScrollingReasons(*frameView);
 }
 
-bool ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously() const
+bool ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously(const FrameView& frameView) const
 {
-    if (FrameView* frameView = m_page->mainFrame().view())
-        return synchronousScrollingReasons(*frameView);
+    if (&frameView == m_page->mainFrame().view())
+        return synchronousScrollingReasons(frameView);
+    
     return true;
 }
 
index c91c32c..a02fe7b 100644 (file)
@@ -195,7 +195,7 @@ public:
     };
 
     SynchronousScrollingReasons synchronousScrollingReasons(const FrameView&) const;
-    bool shouldUpdateScrollLayerPositionSynchronously() const;
+    bool shouldUpdateScrollLayerPositionSynchronously(const FrameView&) const;
 
     virtual void willDestroyScrollableArea(ScrollableArea&) { }
     virtual void scrollableAreaScrollLayerDidChange(ScrollableArea&) { }
@@ -227,7 +227,7 @@ private:
     virtual void setSynchronousScrollingReasons(SynchronousScrollingReasons) { }
 
     virtual bool hasVisibleSlowRepaintViewportConstrainedObjects(const FrameView&) const;
-    void updateSynchronousScrollingReasons(FrameView&);
+    void updateSynchronousScrollingReasons(const FrameView&);
 
     Region absoluteNonFastScrollableRegionForFrame(const Frame&) const;
     
index ddf4fb2..034a711 100644 (file)
@@ -135,7 +135,7 @@ void ScrollingCoordinatorMac::updateTiledScrollingIndicator()
         return;
 
     ScrollingModeIndication indicatorMode;
-    if (shouldUpdateScrollLayerPositionSynchronously())
+    if (shouldUpdateScrollLayerPositionSynchronously(*frameView))
         indicatorMode = SynchronousScrollingBecauseOfStyleIndication;
     else
         indicatorMode = AsyncScrollingIndication;
index 099a351..8909715 100644 (file)
@@ -155,8 +155,6 @@ public:
     virtual IntRect scrollCornerRect() const = 0;
     WEBCORE_EXPORT virtual void invalidateScrollCorner(const IntRect&);
 
-    virtual bool updatesScrollLayerPositionOnMainThread() const = 0;
-
     virtual bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const = 0;
 
     // Convert points and rects between the scrollbar and its containing view.
index b0fd6c3..7087a39 100644 (file)
@@ -106,7 +106,6 @@ private:
     IntSize visibleSize() const override;
     IntSize contentsSize() const override;
     IntRect scrollableAreaBoundingBox(bool* = nullptr) const override;
-    bool updatesScrollLayerPositionOnMainThread() const override { return true; }
     bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override { return false; }
     bool shouldPlaceBlockDirectionScrollbarOnLeft() const final { return false; }
 
index 8a7f2e5..8257d57 100644 (file)
@@ -4024,7 +4024,7 @@ bool RenderLayer::setupFontSubpixelQuantization(GraphicsContext& context, bool&
 #if ENABLE(ASYNC_SCROLLING)
     if (Page* page = renderer().frame().page()) {
         if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-            scrollingOnMainThread = scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously();
+            scrollingOnMainThread = scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(renderer().view().frameView());
     }
 #endif
 
index 18db665..ef8ecff 100644 (file)
@@ -883,7 +883,6 @@ private:
     bool shouldSuspendScrollAnimations() const override;
     IntRect scrollableAreaBoundingBox(bool* isInsideFixed = nullptr) const override;
     bool isRubberBandInProgress() const override;
-    bool updatesScrollLayerPositionOnMainThread() const override { return true; }
     bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
 #if ENABLE(CSS_SCROLL_SNAP)
     bool isScrollSnapInProgress() const override;
index c1ea936..b03ff32 100644 (file)
@@ -131,7 +131,6 @@ private:
     IntPoint lastKnownMousePosition() const override;
     bool isHandlingWheelEvent() const override;
     bool shouldSuspendScrollAnimations() const override;
-    bool updatesScrollLayerPositionOnMainThread() const override { return true; }
     bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
 
     ScrollableArea* enclosingScrollableArea() const override;
index 7fccf51..38127b8 100644 (file)
@@ -1,3 +1,14 @@
+2016-05-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove updatesScrollLayerPositionOnMainThread()
+        https://bugs.webkit.org/show_bug.cgi?id=157277
+
+        Reviewed by Dean Jackson, Tim Horton.
+
+        Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.
+
+        * WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h:
+
 2016-05-02  Alex Christensen  <achristensen@webkit.org>
 
         Crash if a certificate chain has null certificates
index 1bf1bfb..f5d811c 100644 (file)
@@ -213,7 +213,6 @@ private:
     WebCore::IntRect convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntRect& parentRect) const override;
     WebCore::IntPoint convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntPoint& scrollbarPoint) const override;
     WebCore::IntPoint convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntPoint& parentPoint) const override;
-    bool updatesScrollLayerPositionOnMainThread() const override { return true; }
     bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
 
     // PDFPlugin functions.