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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 629583002: Make compositor and blink talk in fractional scroll offset (Closed)

Created:
6 years, 2 months ago by Yufeng Shen (Slow to review)
Modified:
6 years, 2 months ago
Reviewers:
eae, Rick Byers, eseidel
CC:
blink-layers+watch_chromium.org, blink-reviews, Rik, danakj, dglazkov+blink, krit, jamesr, jbroman, kenneth.christiansen, mkwst+moarreviews_chromium.org, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Make compositor and blink talk in fractional scroll offset Make the WebLayer scrolling interface setScrollPositionDouble() and scrollPositionDouble() to use WebDoublePoint so that compositor and blink are both using fractional scroll offset. This enables document fractional scroll on main thread. BUG=414283 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183504

Patch Set 1 #

Total comments: 14

Patch Set 2 : address comments #

Patch Set 3 : address comments #

Patch Set 4 : rebase #

Patch Set 5 : update test expectation #

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+95 lines, -11 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/fast/scrolling/fractional-scroll-offset-document.html View 1 2 1 chunk +36 lines, -0 lines 0 comments Download
A LayoutTests/fast/scrolling/fractional-scroll-offset-document-expected.txt View 1 2 1 chunk +17 lines, -0 lines 0 comments Download
M Source/core/page/scrolling/ScrollingCoordinator.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/graphics/GraphicsLayer.cpp View 1 2 3 4 1 chunk +5 lines, -2 lines 0 comments Download
M Source/platform/graphics/GraphicsLayerTest.cpp View 1 2 3 2 chunks +8 lines, -5 lines 0 comments Download
M Source/web/tests/ScrollingCoordinatorChromiumTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M public/platform/WebDoublePoint.h View 2 chunks +20 lines, -0 lines 0 comments Download
M public/platform/WebLayer.h View 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (7 generated)
Yufeng Shen (Slow to review)
6 years, 2 months ago (2014-10-03 23:05:44 UTC) #2
Rick Byers
Looks good, thanks. Just a few comments. https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html File LayoutTests/fast/scrolling/fractional-scroll-offset-document.html (right): https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html#newcode2 LayoutTests/fast/scrolling/fractional-scroll-offset-document.html:2: <html> nit: ...
6 years, 2 months ago (2014-10-04 00:04:25 UTC) #3
Yufeng Shen (Slow to review)
https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html File LayoutTests/fast/scrolling/fractional-scroll-offset-document.html (right): https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html#newcode2 LayoutTests/fast/scrolling/fractional-scroll-offset-document.html:2: <html> On 2014/10/04 00:04:24, Rick Byers wrote: > nit: ...
6 years, 2 months ago (2014-10-06 17:54:58 UTC) #4
Rick Byers
LGTM with nit https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html File LayoutTests/fast/scrolling/fractional-scroll-offset-document.html (right): https://codereview.chromium.org/629583002/diff/1/LayoutTests/fast/scrolling/fractional-scroll-offset-document.html#newcode33 LayoutTests/fast/scrolling/fractional-scroll-offset-document.html:33: testRunner.notifyDone(); On 2014/10/06 17:54:57, Yufeng Shen ...
6 years, 2 months ago (2014-10-06 21:11:30 UTC) #5
Yufeng Shen (Slow to review)
+ Adam for OWNER of public/platform/* Source/web/*
6 years, 2 months ago (2014-10-07 22:21:57 UTC) #7
eseidel
lgtm
6 years, 2 months ago (2014-10-08 21:58:27 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/629583002/120002
6 years, 2 months ago (2014-10-10 00:24:20 UTC) #14
commit-bot: I haz the power
6 years, 2 months ago (2014-10-10 03:52:17 UTC) #15
Message was sent while issue was closed. 
Committed patchset #6 (id:120002) as 183504

Powered by Google App Engine
This is Rietveld 408576698