Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2531343002: Don't assume current root scroller is a LayoutBox. (Closed)

Created:
4 years ago by bokan
Modified:
4 years ago
Reviewers:
dtapuska
CC:
chromium-reviews, blink-reviews, kenneth.christiansen, kinuko+watch
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't assume current root scroller is a LayoutBox. There was an edge case here where we might get an arbitrary element as the document.documentElement. In these cases, the element won't get a LayoutObject as LayoutView will refuse to attach to it, but when we replace it a layout will create a non-LayoutBox for it and when RootScroller tries to replace it as the effective root scroller it needs to do some book keeping using the old root scroller. It incorrectly assumed it must be a LayoutBox. This patch now guards against this edge case. BUG=668553 Committed: https://crrev.com/1a413b53d66a15908a7960fe1d7a3dc3c70a8a5e Cr-Commit-Position: refs/heads/master@{#434771}

Patch Set 1 #

Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -2 lines) Patch
M third_party/WebKit/Source/core/page/scrolling/RootScrollerUtil.cpp View 1 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/web/tests/RootScrollerTest.cpp View 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (6 generated)
bokan
4 years ago (2016-11-28 17:49:32 UTC) #3
dtapuska
On 2016/11/28 17:49:32, bokan wrote: lgtm
4 years ago (2016-11-28 17:51:19 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2531343002/20001
4 years ago (2016-11-28 17:52:11 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-11-28 23:28:00 UTC) #9
commit-bot: I haz the power
4 years ago (2016-11-28 23:29:54 UTC) #11
Message was sent while issue was closed. 
Patchset 2 (id:??) landed as
https://crrev.com/1a413b53d66a15908a7960fe1d7a3dc3c70a8a5e
Cr-Commit-Position: refs/heads/master@{#434771}

Powered by Google App Engine
This is Rietveld 408576698