Revert of Fix scroll chaining for non-descendants of root scroller. (issue 2382913003 by bokan@chromium.org)

0 views
Skip to first unread message

bo...@chromium.org

unread,
Sep 30, 2016, 12:05:02 PM9/30/16
to ael...@chromium.org, jayd...@chromium.org, tdre...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, chromium...@chromium.org, dtapuska+...@chromium.org, nzolghadr+...@chromium.org
Reviewers: aelias, jaydasika, tdresser (OOO Sept 26-28)
CL: https://codereview.chromium.org/2382913003/

Message:
Created Revert of Fix scroll chaining for non-descendants of root scroller.

Description:
Revert of Fix scroll chaining for non-descendants of root scroller. (patchset #4
id:60001 of https://codereview.chromium.org/2365793002/ )

Reason for revert:
Broke WebView scrolling. See crbug.com/651515

Original issue's description:
> Fix scroll chaining for non-descendants of root scroller.
>
> This CL fixes the scroll chaining behavior for elements that aren't
descendants
> of the root scroller element. This can only happen in the non-document root
> scroller proposal.
>
> On the main thread scrolling path, we were previously assuming that all
scrolls
> would chain through the effective root scroller. This CL fixes the terminating
> condition as well as removes an invalid DCHECK.
>
> On the compositor side, the chaining mechanisms would previously use the inner
> viewport scroll layer to designate viewport scrolling. Further on, we would
> explicitly check for the inner viewport layer and scroll using cc::Viewport.
> When scrolling a non-descendant of the root scroller, we chain up to the inner
> viewport scroll layer without going through the outer viewport. Scrolling
> cc::Viewport causes scrolling in the outer viewport though so we would scroll
> it inadvertaintly. I've made changes so that we use the outer viewport to
> designate cc::Viewport scrolling. Scrolls will terminate at the outer viewport
> scroll layer, but they don't have to chain through it.
>
> Unrelated, but I also removed LayerImpl::ApplyScroll as it's unused.
>
> BUG=505516
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
>
> Committed: https://crrev.com/bc48cb30d9e473b693dc996b7750510d05d50f69
> Cr-Commit-Position: refs/heads/master@{#420763}

TBR=ael...@chromium.org,jayd...@chromium.org,tdre...@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=505516

Affected files (+67, -361 lines):
M cc/layers/layer_impl.h
M cc/layers/layer_impl.cc
M cc/trees/layer_tree_host_impl.cc
M cc/trees/layer_tree_host_impl_unittest.cc
D third_party/WebKit/LayoutTests/fast/scrolling/scroll-non-descendant-of-root-scroller.html
M third_party/WebKit/Source/core/input/ScrollManager.cpp


commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 12:05:21 PM9/30/16
to bo...@chromium.org, ael...@chromium.org, jayd...@chromium.org, tdre...@chromium.org, commi...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, chromium...@chromium.org, dtapuska+...@chromium.org, nzolghadr+...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 2:04:30 PM9/30/16
to bo...@chromium.org, ael...@chromium.org, jayd...@chromium.org, tdre...@chromium.org, commi...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, chromium...@chromium.org, dtapuska+...@chromium.org, nzolghadr+...@chromium.org
Committed patchset #1 (id:1)

https://codereview.chromium.org/2382913003/

commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 2:05:56 PM9/30/16
to bo...@chromium.org, ael...@chromium.org, jayd...@chromium.org, tdre...@chromium.org, commi...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, chromium...@chromium.org, dtapuska+...@chromium.org, nzolghadr+...@chromium.org
Patchset 1 (id:??) landed as
https://crrev.com/29bd015dd8bff8eb77bbd687b7135e45061c021f
Cr-Commit-Position: refs/heads/master@{#422152}

https://codereview.chromium.org/2382913003/
Reply all
Reply to author
Forward
0 new messages