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

Issue 1102743002: OverscrollGlow for mainThread-{CHROMIUM CHANGES} (Closed)

Created:
5 years, 8 months ago by MuVen
Modified:
5 years, 6 months ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, yusukes+watch_chromium.org, shuchen+watch_chromium.org, nasko+codewatch_chromium.org, jam, penghuang+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, mkwst+moarreviews-shell_chromium.org, James Su, jochen+watch_chromium.org, sivag
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

OverscrollGlow for mainThread. Current patch passes on the mainthread overscroll params to RWHVA::DidOverscroll which shall trigger glow effect animation. Blink changes @ https://codereview.chromium.org/1056983004/ BUG=420207 Committed: https://crrev.com/582c9cedbf4fa60162fca35251b1244f5ba3b2de Cr-Commit-Position: refs/heads/master@{#333460}

Patch Set 1 #

Total comments: 8

Patch Set 2 : addressed review comments #

Patch Set 3 : addressed jared comments #

Patch Set 4 : addressed majidvp comments #

Patch Set 5 : rebased to latest #

Total comments: 2

Patch Set 6 : addressed review comments #

Total comments: 6

Patch Set 7 : addressed review comments #

Total comments: 1

Patch Set 8 : addressed nits #

Patch Set 9 : rebased to latest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -0 lines) Patch
M content/renderer/render_widget.h View 1 2 3 4 5 6 7 1 chunk +5 lines, -0 lines 0 comments Download
M content/renderer/render_widget.cc View 1 2 3 4 5 6 7 8 1 chunk +18 lines, -0 lines 0 comments Download

Messages

Total messages: 26 (10 generated)
MuVen
Hi Jdduke, PTAL of the changes, This is just a working prototype, Based on your ...
5 years, 8 months ago (2015-04-23 11:43:58 UTC) #3
jdduke (slow)
https://codereview.chromium.org/1102743002/diff/1/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/1102743002/diff/1/content/browser/renderer_host/render_widget_host_view_android.cc#newcode768 content/browser/renderer_host/render_widget_host_view_android.cc:768: void RenderWidgetHostViewAndroid::OnDidOverScrollOnMainThread( Looks like we don't make any distinctino ...
5 years, 8 months ago (2015-04-24 16:57:23 UTC) #4
MuVen
https://codereview.chromium.org/1102743002/diff/1/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/1102743002/diff/1/content/browser/renderer_host/render_widget_host_view_android.cc#newcode768 content/browser/renderer_host/render_widget_host_view_android.cc:768: void RenderWidgetHostViewAndroid::OnDidOverScrollOnMainThread( On 2015/04/24 16:57:23, jdduke wrote: > Looks ...
5 years, 8 months ago (2015-04-27 09:49:16 UTC) #7
jdduke (slow)
https://codereview.chromium.org/1102743002/diff/1/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://codereview.chromium.org/1102743002/diff/1/content/renderer/render_widget.cc#newcode2248 content/renderer/render_widget.cc:2248: Send(new ViewHostMsg_DidOverScrollOnMainThread(routing_id(), unusedDelta, On 2015/04/27 09:49:15, MuVen wrote: > ...
5 years, 8 months ago (2015-04-27 15:00:49 UTC) #8
jdduke (slow)
https://codereview.chromium.org/1102743002/diff/140001/content/renderer/render_view_impl.h File content/renderer/render_view_impl.h (right): https://codereview.chromium.org/1102743002/diff/140001/content/renderer/render_view_impl.h#newcode334 content/renderer/render_view_impl.h:334: void didOverscroll(const blink::WebFloatSize& unusedDelta, Do we need this here? ...
5 years, 6 months ago (2015-06-04 18:40:04 UTC) #10
MuVen
PTAL. https://codereview.chromium.org/1102743002/diff/140001/content/renderer/render_view_impl.h File content/renderer/render_view_impl.h (right): https://codereview.chromium.org/1102743002/diff/140001/content/renderer/render_view_impl.h#newcode334 content/renderer/render_view_impl.h:334: void didOverscroll(const blink::WebFloatSize& unusedDelta, On 2015/06/04 18:40:04, jdduke(OOO_until_June_8) ...
5 years, 6 months ago (2015-06-05 05:51:52 UTC) #11
jdduke (slow)
https://codereview.chromium.org/1102743002/diff/160001/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://codereview.chromium.org/1102743002/diff/160001/content/renderer/render_widget.cc#newcode2230 content/renderer/render_widget.cc:2230: DidOverscrollParams params; Can you add a "TODO(jdduke): Consider bundling ...
5 years, 6 months ago (2015-06-05 14:32:51 UTC) #12
MuVen
https://codereview.chromium.org/1102743002/diff/160001/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://codereview.chromium.org/1102743002/diff/160001/content/renderer/render_widget.cc#newcode2230 content/renderer/render_widget.cc:2230: DidOverscrollParams params; On 2015/06/05 14:32:51, jdduke(OOO_until_June_8) wrote: > Can ...
5 years, 6 months ago (2015-06-08 07:09:58 UTC) #13
jdduke (slow)
lgtm, but you'l need a content/renderer owner. https://codereview.chromium.org/1102743002/diff/180001/content/renderer/render_widget.h File content/renderer/render_widget.h (right): https://codereview.chromium.org/1102743002/diff/180001/content/renderer/render_widget.h#newcode194 content/renderer/render_widget.h:194: virtual void ...
5 years, 6 months ago (2015-06-08 14:52:56 UTC) #14
MuVen
sievers, PTAL. Thanks, Muven.
5 years, 6 months ago (2015-06-08 14:57:59 UTC) #16
no sievers
lgtm. Can you reference the Blink patch in the cl description since it depends on ...
5 years, 6 months ago (2015-06-08 22:37:27 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1102743002/220001
5 years, 6 months ago (2015-06-09 04:04:29 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_rel_ng/builds/31477)
5 years, 6 months ago (2015-06-09 06:10:00 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1102743002/220001
5 years, 6 months ago (2015-06-09 06:11:03 UTC) #24
commit-bot: I haz the power
Committed patchset #9 (id:220001)
5 years, 6 months ago (2015-06-09 08:03:51 UTC) #25
commit-bot: I haz the power
5 years, 6 months ago (2015-06-09 08:04:58 UTC) #26
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/582c9cedbf4fa60162fca35251b1244f5ba3b2de
Cr-Commit-Position: refs/heads/master@{#333460}

Powered by Google App Engine
This is Rietveld 408576698