Changeset 247839 in webkit


Ignore:
Timestamp:
Jul 25, 2019 2:12:01 PM (5 years ago)
Author:
Simon Fraser
Message:

[iOS WK2] A top fixed bar can flicker when scrolling with the keyboard up
​https://bugs.webkit.org/show_bug.cgi?id=200105
rdar://problem/52871975

Reviewed by Wenson Hsieh.

Source/WebCore:

ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition() computes a visual viewport
from the current scroll position and scrollableAreaSize(). This doesn't know anything about
the impact of keyboards on the visual viewport, so it computes a too-large visual viewport
when the keyboard is up, triggering incorrect manipulations of the layout viewport. This
leads to the top bar flashing to position 0 when it should be hidden off the top.

Fix by feeding into the scrolling tree the height of the visual viewport which takes
FrameView::visualViewportOverrideRect() into account. This is stored on ScrollingStateFrameScrollingNode/
ScrollingTreeFrameScrollingNode.

Test: scrollingcoordinator/ios/fixed-scrolling-with-keyboard.html

  • page/FrameView.h:
  • page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::setFrameScrollingNodeState):

  • page/scrolling/ScrollingStateFrameScrollingNode.cpp:

(WebCore::ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode):
(WebCore::ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach):
(WebCore::ScrollingStateFrameScrollingNode::setOverrideVisualViewportSize):
(WebCore::ScrollingStateFrameScrollingNode::dumpProperties const):

  • page/scrolling/ScrollingStateFrameScrollingNode.h:
  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::commitTreeState): LOG_WITH_STREAM() doesn't evaluate scrollingTreeAsText()
every time.

  • page/scrolling/ScrollingTreeFrameScrollingNode.cpp:

(WebCore::ScrollingTreeFrameScrollingNode::commitStateBeforeChildren):
(WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition const):
(WebCore::ScrollingTreeFrameScrollingNode::dumpProperties const):

  • page/scrolling/ScrollingTreeFrameScrollingNode.h:

Source/WebKit:

ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition() computes a visual viewport
from the current scroll position and scrollableAreaSize(). This doesn't know anything about
the impact of keyboards on the visual viewport, so it computes a too-large visual viewport
when the keyboard is up, triggering incorrect manipulations of the layout viewport. This
leads to the top bar flashing to position 0 when it should be hidden off the top.

Fix by feeding into the scrolling tree the height of the visual viewport which takes
FrameView::visualViewportOverrideRect() into account. This is stored on ScrollingStateFrameScrollingNode/
ScrollingTreeFrameScrollingNode.

  • Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:

(ArgumentCoder<ScrollingStateFrameScrollingNode>::encode):
(ArgumentCoder<ScrollingStateFrameScrollingNode>::decode):

LayoutTests:

  • resources/ui-helper.js:

(window.UIHelper.ensureStablePresentationUpdate.return.new.Promise):
(window.UIHelper.ensureStablePresentationUpdate):

  • scrollingcoordinator/ios/fixed-scrolling-with-keyboard-expected.txt: Added.
  • scrollingcoordinator/ios/fixed-scrolling-with-keyboard.html: Added.
Location:
trunk
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247833 r247839  
    229229        * fast/canvas/setTransfrom-aliases-transform-expected.html: Added.
    230230        * fast/canvas/setTransfrom-aliases-transform.html: Added.
     231
     2322019-07-24  Simon Fraser  <simon.fraser@apple.com>
     233
     234        [iOS WK2] A top fixed bar can flicker when scrolling with the keyboard up
     235        https://bugs.webkit.org/show_bug.cgi?id=200105
     236        rdar://problem/52871975
     237
     238        Reviewed by Wenson Hsieh.
     239
     240        * resources/ui-helper.js:
     241        (window.UIHelper.ensureStablePresentationUpdate.return.new.Promise):
     242        (window.UIHelper.ensureStablePresentationUpdate):
     243        * scrollingcoordinator/ios/fixed-scrolling-with-keyboard-expected.txt: Added.
     244        * scrollingcoordinator/ios/fixed-scrolling-with-keyboard.html: Added.
    231245
    2322462019-07-23  Tim Horton  <timothy_horton@apple.com>

  • trunk/LayoutTests/resources/ui-helper.js

    r247180 r247839  
    255255    }
    256256
     257    static ensureStablePresentationUpdate()
     258    {
     259        if (!this.isWebKit2()) {
     260            testRunner.display();
     261            return Promise.resolve();
     262        }
     263
     264        return new Promise(resolve => {
     265            testRunner.runUIScript(`
     266                uiController.doAfterNextStablePresentationUpdate(function() {
     267                    uiController.uiScriptComplete();
     268                });`, resolve);
     269        });
     270    }
     271
    257272    static ensurePositionInformationUpdateForElement(element)
    258273    {
    … …  
    970985        });
    971986    }
     987
     988    static getScrollingTree()
     989    {
     990        if (!this.isWebKit2() || !this.isIOSFamily())
     991            return Promise.resolve();
     992
     993        return new Promise(resolve => {
     994            testRunner.runUIScript(`(() => {
     995                return uiController.scrollingTreeAsText;
     996            })()`, resolve);
     997        });
     998    }
    972999}

  • trunk/Source/WebCore/ChangeLog

    r247837 r247839  
    784784        * html/canvas/CanvasRenderingContext2DBase.cpp:
    785785        (WebCore::CanvasRenderingContext2DBase::setTransform):
     786
     7872019-07-24  Simon Fraser  <simon.fraser@apple.com>
     788
     789        [iOS WK2] A top fixed bar can flicker when scrolling with the keyboard up
     790        https://bugs.webkit.org/show_bug.cgi?id=200105
     791        rdar://problem/52871975
     792
     793        Reviewed by Wenson Hsieh.
     794
     795        ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition() computes a visual viewport
     796        from the current scroll position and scrollableAreaSize(). This doesn't know anything about
     797        the impact of keyboards on the visual viewport, so it computes a too-large visual viewport
     798        when the keyboard is up, triggering incorrect manipulations of the layout viewport. This
     799        leads to the top bar flashing to position 0 when it should be hidden off the top.
     800
     801        Fix by feeding into the scrolling tree the height of the visual viewport which takes
     802        FrameView::visualViewportOverrideRect() into account. This is stored on ScrollingStateFrameScrollingNode/
     803        ScrollingTreeFrameScrollingNode.
     804
     805        Test: scrollingcoordinator/ios/fixed-scrolling-with-keyboard.html
     806
     807        * page/FrameView.h:
     808        * page/scrolling/AsyncScrollingCoordinator.cpp:
     809        (WebCore::AsyncScrollingCoordinator::setFrameScrollingNodeState):
     810        * page/scrolling/ScrollingStateFrameScrollingNode.cpp:
     811        (WebCore::ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode):
     812        (WebCore::ScrollingStateFrameScrollingNode::setPropertyChangedBitsAfterReattach):
     813        (WebCore::ScrollingStateFrameScrollingNode::setOverrideVisualViewportSize):
     814        (WebCore::ScrollingStateFrameScrollingNode::dumpProperties const):
     815        * page/scrolling/ScrollingStateFrameScrollingNode.h:
     816        * page/scrolling/ScrollingTree.cpp:
     817        (WebCore::ScrollingTree::commitTreeState): LOG_WITH_STREAM() doesn't evaluate scrollingTreeAsText()
     818        every time.
     819        * page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
     820        (WebCore::ScrollingTreeFrameScrollingNode::commitStateBeforeChildren):
     821        (WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition const):
     822        (WebCore::ScrollingTreeFrameScrollingNode::dumpProperties const):
     823        * page/scrolling/ScrollingTreeFrameScrollingNode.h:
    786824
    7878252019-07-23  Tim Horton  <timothy_horton@apple.com>

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

    r247702 r247839  
    259259
    260260    WEBCORE_EXPORT void setVisualViewportOverrideRect(Optional<LayoutRect>);
     261    Optional<LayoutRect> visualViewportOverrideRect() const { return m_visualViewportOverrideRect; }
    261262
    262263    // These are in document coordinates, unaffected by page scale (but affected by zooming).

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

    r247344 r247839  
    653653    frameScrollingNode.setMaxLayoutViewportOrigin(frameView.maxStableLayoutViewportOrigin());
    654654
     655    if (auto visualOverrideRect = frameView.visualViewportOverrideRect())
     656        frameScrollingNode.setOverrideVisualViewportSize(FloatSize(visualOverrideRect.value().size()));
     657    else
     658        frameScrollingNode.setOverrideVisualViewportSize(WTF::nullopt);
     659
    655660    frameScrollingNode.setFixedElementsLayoutRelativeToFrame(frameView.fixedElementsLayoutRelativeToFrame());
    656661

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

    r247344 r247839  
    5151    , m_minLayoutViewportOrigin(stateNode.minLayoutViewportOrigin())
    5252    , m_maxLayoutViewportOrigin(stateNode.maxLayoutViewportOrigin())
     53    , m_overrideVisualViewportSize(stateNode.overrideVisualViewportSize())
    5354    , m_frameScaleFactor(stateNode.frameScaleFactor())
    5455    , m_topContentInset(stateNode.topContentInset())
    … …  
    109110    setPropertyChangedBit(MinLayoutViewportOrigin);
    110111    setPropertyChangedBit(MaxLayoutViewportOrigin);
     112    setPropertyChangedBit(OverrideVisualViewportSize);
    111113
    112114    ScrollingStateScrollingNode::setPropertyChangedBitsAfterReattach();
    … …  
    175177    m_maxLayoutViewportOrigin = p;
    176178    setPropertyChanged(MaxLayoutViewportOrigin);
     179}
     180
     181void ScrollingStateFrameScrollingNode::setOverrideVisualViewportSize(Optional<FloatSize> viewportSize)
     182{
     183    if (viewportSize == m_overrideVisualViewportSize)
     184        return;
     185
     186    m_overrideVisualViewportSize = viewportSize;
     187    setPropertyChanged(OverrideVisualViewportSize);
    177188}
    178189
    … …  
    313324    ts.dumpProperty("max layout viewport origin", m_maxLayoutViewportOrigin);
    314325   
     326    if (m_overrideVisualViewportSize)
     327        ts.dumpProperty("override visual viewport size", m_overrideVisualViewportSize.value());
     328
    315329    if (m_behaviorForFixed == StickToViewportBounds)
    316330        ts.dumpProperty("behavior for fixed", m_behaviorForFixed);

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

    r247344 r247839  
    6767        MinLayoutViewportOrigin,
    6868        MaxLayoutViewportOrigin,
     69        OverrideVisualViewportSize,
    6970    };
    7071
    … …  
    8990    FloatPoint maxLayoutViewportOrigin() const { return m_maxLayoutViewportOrigin; }
    9091    WEBCORE_EXPORT void setMaxLayoutViewportOrigin(const FloatPoint&);
     92
     93    Optional<FloatSize> overrideVisualViewportSize() const { return m_overrideVisualViewportSize; };
     94    WEBCORE_EXPORT void setOverrideVisualViewportSize(Optional<FloatSize>);
    9195
    9296    int headerHeight() const { return m_headerHeight; }
    … …  
    155159    FloatPoint m_minLayoutViewportOrigin;
    156160    FloatPoint m_maxLayoutViewportOrigin;
     161    Optional<FloatSize> m_overrideVisualViewportSize;
    157162
    158163    float m_frameScaleFactor { 1 };

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

    r247306 r247839  
    187187    }
    188188
    189     LOG(Scrolling, "committed ScrollingTree\n%s", scrollingTreeAsText(ScrollingStateTreeAsTextBehaviorDebug).utf8().data());
     189    LOG_WITH_STREAM(Scrolling, stream << "committed ScrollingTree" << scrollingTreeAsText(ScrollingStateTreeAsTextBehaviorDebug));
    190190}
    191191

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

    r247344 r247839  
    8686    if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::MaxLayoutViewportOrigin))
    8787        m_maxLayoutViewportOrigin = state.maxLayoutViewportOrigin();
     88
     89    if (state.hasChangedProperty(ScrollingStateFrameScrollingNode::OverrideVisualViewportSize))
     90        m_overrideVisualViewportSize = state.overrideVisualViewportSize();
    8891}
    8992
    … …  
    9598FloatRect ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition(const FloatPoint& visibleContentOrigin, float scale) const
    9699{
    97     FloatRect visibleContentRect(visibleContentOrigin, scrollableAreaSize());
     100    FloatSize visualViewportSize = m_overrideVisualViewportSize.valueOr(scrollableAreaSize());
     101    FloatRect visibleContentRect(visibleContentOrigin, visualViewportSize);
    98102    LayoutRect visualViewport(FrameView::visibleDocumentRect(visibleContentRect, headerHeight(), footerHeight(), totalContentsSize(), scale));
    99103    LayoutRect layoutViewport(m_layoutViewport);
    100104
    101     LOG_WITH_STREAM(Scrolling, stream << "\nScrolling thread: " << "(visibleContentOrigin " << visibleContentOrigin << ") fixed behavior " << m_behaviorForFixed);
     105    LOG_WITH_STREAM(Scrolling, stream << "\nScrolling thread: " << "(visibleContentOrigin " << visibleContentOrigin << ", visualViewportSize " << visualViewportSize << ") fixed behavior " << m_behaviorForFixed);
    102106    LOG_WITH_STREAM(Scrolling, stream << "  layoutViewport: " << layoutViewport);
    103107    LOG_WITH_STREAM(Scrolling, stream << "  visualViewport: " << visualViewport);
    … …  
    147151    ts.dumpProperty("max layoutViewport origin", m_maxLayoutViewportOrigin);
    148152
     153    if (m_overrideVisualViewportSize)
     154        ts.dumpProperty("override visual viewport size", m_overrideVisualViewportSize.value());
     155
    149156    if (m_frameScaleFactor != 1)
    150157        ts.dumpProperty("frame scale factor", m_frameScaleFactor);

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

    r247344 r247839  
    7979    FloatPoint m_minLayoutViewportOrigin;
    8080    FloatPoint m_maxLayoutViewportOrigin;
     81    Optional<FloatSize> m_overrideVisualViewportSize;
    8182   
    8283    float m_frameScaleFactor { 1 };

  • trunk/Source/WebKit/ChangeLog

    r247838 r247839  
    661661        (WebKit::WebProcessPool::sendWebProcessDataStoreParameters):
    662662        * UIProcess/WebProcessPool.h:
     663
     6642019-07-24  Simon Fraser  <simon.fraser@apple.com>
     665
     666        [iOS WK2] A top fixed bar can flicker when scrolling with the keyboard up
     667        https://bugs.webkit.org/show_bug.cgi?id=200105
     668        rdar://problem/52871975
     669
     670        Reviewed by Wenson Hsieh.
     671
     672        ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition() computes a visual viewport
     673        from the current scroll position and scrollableAreaSize(). This doesn't know anything about
     674        the impact of keyboards on the visual viewport, so it computes a too-large visual viewport
     675        when the keyboard is up, triggering incorrect manipulations of the layout viewport. This
     676        leads to the top bar flashing to position 0 when it should be hidden off the top.
     677
     678        Fix by feeding into the scrolling tree the height of the visual viewport which takes
     679        FrameView::visualViewportOverrideRect() into account. This is stored on ScrollingStateFrameScrollingNode/
     680        ScrollingTreeFrameScrollingNode.
     681
     682        * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
     683        (ArgumentCoder<ScrollingStateFrameScrollingNode>::encode):
     684        (ArgumentCoder<ScrollingStateFrameScrollingNode>::decode):
    663685
    6646862019-07-23  Alex Christensen  <achristensen@webkit.org>

  • trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp

    r247344 r247839  
    184184    SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::MinLayoutViewportOrigin, minLayoutViewportOrigin)
    185185    SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::MaxLayoutViewportOrigin, maxLayoutViewportOrigin)
     186    SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::OverrideVisualViewportSize, overrideVisualViewportSize)
    186187
    187188    if (node.hasChangedProperty(ScrollingStateFrameScrollingNode::CounterScrollingLayer))
    … …  
    312313    SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::MinLayoutViewportOrigin, FloatPoint, setMinLayoutViewportOrigin)
    313314    SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::MaxLayoutViewportOrigin, FloatPoint, setMaxLayoutViewportOrigin)
     315    SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::OverrideVisualViewportSize, Optional<FloatSize>, setOverrideVisualViewportSize)
    314316
    315317    if (node.hasChangedProperty(ScrollingStateFrameScrollingNode::CounterScrollingLayer)) {
Note: See TracChangeset for help on using the changeset viewer.