Changeset 200342 in webkit


Ignore:
Timestamp:
May 2, 2016, 3:19:29 PM (10 years ago)
Author:
Simon Fraser
Message:

Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove updatesScrollLayerPositionOnMainThread()
​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:
Location:
trunk/Source
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r200341 r200342  
     12016-05-02  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove updatesScrollLayerPositionOnMainThread()
     4        https://bugs.webkit.org/show_bug.cgi?id=157277
     5
     6        Reviewed by Dean Jackson, Tim Horton.
     7
     8        shouldUpdateScrollLayerPositionSynchronously() gave an answer for the main frame even if
     9        called for a subframe. This could have caused scroll snapping, and isRubberBandInProgress()
     10        to give wrong answers sometimes. Fix by passing in the FrameView.
     11
     12        I was unable to easily come up with a testcase to detect the incorrect behavior.
     13
     14        Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.
     15
     16        * page/FrameView.cpp:
     17        (WebCore::FrameView::isScrollSnapInProgress):
     18        (WebCore::FrameView::shouldUpdateCompositingLayersAfterScrolling):
     19        (WebCore::FrameView::isRubberBandInProgress):
     20        (WebCore::FrameView::updatesScrollLayerPositionOnMainThread): Deleted.
     21        * page/FrameView.h:
     22        * page/scrolling/ScrollingCoordinator.cpp:
     23        (WebCore::ScrollingCoordinator::updateSynchronousScrollingReasons):
     24        (WebCore::ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously):
     25        * page/scrolling/ScrollingCoordinator.h:
     26        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     27        (WebCore::ScrollingCoordinatorMac::updateTiledScrollingIndicator):
     28        * platform/ScrollableArea.h:
     29        * platform/win/PopupMenuWin.h:
     30        * rendering/RenderLayer.cpp:
     31        (WebCore::RenderLayer::setupFontSubpixelQuantization):
     32        * rendering/RenderLayer.h:
     33        * rendering/RenderListBox.h:
     34
    1352016-05-02  Simon Fraser  <simon.fraser@apple.com>
    236

  • trunk/Source/WebCore/page/FrameView.cpp

    r200161 r200342  
    971971    if (Page* page = frame().page()) {
    972972        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
    973             if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
     973            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
    974974                return scrollingCoordinator->isScrollSnapInProgress();
    975975        }
    … …  
    23082308        return true;
    23092309
    2310     if (scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
     2310    if (scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
    23112311        return true;
    23122312
    … …  
    23412341    if (Page* page = frame().page()) {
    23422342        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
    2343             if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously())
     2343            if (!scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(*this))
    23442344                return scrollingCoordinator->isRubberBandInProgress();
    23452345        }
    … …  
    35263526    Page* page = frame().page();
    35273527    return page && page->focusController().isActive();
    3528 }
    3529 
    3530 bool FrameView::updatesScrollLayerPositionOnMainThread() const
    3531 {
    3532     if (Page* page = frame().page()) {
    3533         if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
    3534             return scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously();
    3535     }
    3536 
    3537     return true;
    35383528}
    35393529

  • trunk/Source/WebCore/page/FrameView.h

    r200116 r200342  
    487487
    488488    bool isActive() const override;
    489     bool updatesScrollLayerPositionOnMainThread() const override;
    490489    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
    491490

  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp

    r197841 r200342  
    348348}
    349349
    350 void ScrollingCoordinator::updateSynchronousScrollingReasons(FrameView& frameView)
     350void ScrollingCoordinator::updateSynchronousScrollingReasons(const FrameView& frameView)
    351351{
    352352    // FIXME: Once we support async scrolling of iframes, we'll have to track the synchronous scrolling
    … …  
    368368}
    369369
    370 bool ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously() const
    371 {
    372     if (FrameView* frameView = m_page->mainFrame().view())
    373         return synchronousScrollingReasons(*frameView);
     370bool ScrollingCoordinator::shouldUpdateScrollLayerPositionSynchronously(const FrameView& frameView) const
     371{
     372    if (&frameView == m_page->mainFrame().view())
     373        return synchronousScrollingReasons(frameView);
     374   
    374375    return true;
    375376}

  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r197841 r200342  
    196196
    197197    SynchronousScrollingReasons synchronousScrollingReasons(const FrameView&) const;
    198     bool shouldUpdateScrollLayerPositionSynchronously() const;
     198    bool shouldUpdateScrollLayerPositionSynchronously(const FrameView&) const;
    199199
    200200    virtual void willDestroyScrollableArea(ScrollableArea&) { }
    … …  
    228228
    229229    virtual bool hasVisibleSlowRepaintViewportConstrainedObjects(const FrameView&) const;
    230     void updateSynchronousScrollingReasons(FrameView&);
     230    void updateSynchronousScrollingReasons(const FrameView&);
    231231
    232232    Region absoluteNonFastScrollableRegionForFrame(const Frame&) const;

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

    r194496 r200342  
    136136
    137137    ScrollingModeIndication indicatorMode;
    138     if (shouldUpdateScrollLayerPositionSynchronously())
     138    if (shouldUpdateScrollLayerPositionSynchronously(*frameView))
    139139        indicatorMode = SynchronousScrollingBecauseOfStyleIndication;
    140140    else

  • trunk/Source/WebCore/platform/ScrollableArea.h

    r200116 r200342  
    156156    WEBCORE_EXPORT virtual void invalidateScrollCorner(const IntRect&);
    157157
    158     virtual bool updatesScrollLayerPositionOnMainThread() const = 0;
    159 
    160158    virtual bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const = 0;
    161159

  • trunk/Source/WebCore/platform/win/PopupMenuWin.h

    r200116 r200342  
    107107    IntSize contentsSize() const override;
    108108    IntRect scrollableAreaBoundingBox(bool* = nullptr) const override;
    109     bool updatesScrollLayerPositionOnMainThread() const override { return true; }
    110109    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override { return false; }
    111110    bool shouldPlaceBlockDirectionScrollbarOnLeft() const final { return false; }

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r200284 r200342  
    40254025    if (Page* page = renderer().frame().page()) {
    40264026        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
    4027             scrollingOnMainThread = scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously();
     4027            scrollingOnMainThread = scrollingCoordinator->shouldUpdateScrollLayerPositionSynchronously(renderer().view().frameView());
    40284028    }
    40294029#endif

  • trunk/Source/WebCore/rendering/RenderLayer.h

    r200284 r200342  
    884884    IntRect scrollableAreaBoundingBox(bool* isInsideFixed = nullptr) const override;
    885885    bool isRubberBandInProgress() const override;
    886     bool updatesScrollLayerPositionOnMainThread() const override { return true; }
    887886    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
    888887#if ENABLE(CSS_SCROLL_SNAP)

  • trunk/Source/WebCore/rendering/RenderListBox.h

    r200265 r200342  
    132132    bool isHandlingWheelEvent() const override;
    133133    bool shouldSuspendScrollAnimations() const override;
    134     bool updatesScrollLayerPositionOnMainThread() const override { return true; }
    135134    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
    136135

  • trunk/Source/WebKit2/ChangeLog

    r200335 r200342  
     12016-05-02  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Fix shouldUpdateScrollLayerPositionSynchronously() for non-main frames. Remove updatesScrollLayerPositionOnMainThread()
     4        https://bugs.webkit.org/show_bug.cgi?id=157277
     5
     6        Reviewed by Dean Jackson, Tim Horton.
     7
     8        Remove updatesScrollLayerPositionOnMainThread() which is unused by all ports.
     9
     10        * WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h:
     11
    1122016-05-02  Alex Christensen  <achristensen@webkit.org>
    213

  • trunk/Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h

    r200116 r200342  
    214214    WebCore::IntPoint convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntPoint& scrollbarPoint) const override;
    215215    WebCore::IntPoint convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntPoint& parentPoint) const override;
    216     bool updatesScrollLayerPositionOnMainThread() const override { return true; }
    217216    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
    218217
Note: See TracChangeset for help on using the changeset viewer.