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

Issue 26809004: Add a flag for enabling/disabling the new accelerated scrolling path (Closed)

Created:
7 years, 2 months ago by Ian Vollick
Modified:
7 years, 2 months ago
CC:
chromium-reviews, nkostylev+watch_chromium.org, jam, joi+watch-content_chromium.org, darin-cc_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, miu+watch_chromium.org
Visibility:
Public.

Description

Add a flag for enabling/disabling the new accelerated scrolling path Relanding. This patch had previously been reverted because opting into the new path could run us into bugs. All known bugs have been resolved, so this should be safe to land. We now have two accelerated scrolling paths. One accelerates scrolling whenever it's safe to promote the overflow scrolling div to a stacking container / containing block and not break stacking or clipping. The new path, the 'universal' path, enables accelerated scrolling in those cases where the old path would not opt-in. Eventually, we'd like to eliminate the old path, but this will require layer squashing (the new path can result in many composited layers that ought to be combined -- i.e., a 'layer explosion'). Until these paths are mature, we need to be able to enable and disable them both, so we have two separate sets of flags. Once we've eliminated the old path, we can replace both with --[enable/disable]-accelerated-overflow-scroll (and eventually get rid of both sets of flags). BUG=None Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=228187 R=darin@chromium.org, hartmanng@chromium.org, shawnsingh@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229421

Patch Set 1 #

Total comments: 4

Patch Set 2 : . #

Total comments: 1

Patch Set 3 : About to reland. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -0 lines) Patch
M build/ios/grit_whitelist.txt View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/chrome_restart_request.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 2 chunks +9 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 2 chunks +16 lines, -0 lines 0 comments Download
M webkit/common/webpreferences.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M webkit/common/webpreferences.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Ian Vollick
7 years, 2 months ago (2013-10-11 01:19:26 UTC) #1
shawnsingh
https://codereview.chromium.org/26809004/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/26809004/diff/1/chrome/app/generated_resources.grd#newcode5729 chrome/app/generated_resources.grd:5729: + Puts scrolling content in composited layers, even in ...
7 years, 2 months ago (2013-10-11 01:34:50 UTC) #2
Ian Vollick
https://codereview.chromium.org/26809004/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/26809004/diff/1/chrome/app/generated_resources.grd#newcode5729 chrome/app/generated_resources.grd:5729: + Puts scrolling content in composited layers, even in ...
7 years, 2 months ago (2013-10-11 01:38:59 UTC) #3
shawnsingh
non-OWNER LGTM =)
7 years, 2 months ago (2013-10-11 01:41:06 UTC) #4
hartmanng
(non-OWNER) lgtm2
7 years, 2 months ago (2013-10-11 01:43:39 UTC) #5
darin (slow to review)
LGTM https://codereview.chromium.org/26809004/diff/6001/content/renderer/render_view_impl.cc File content/renderer/render_view_impl.cc (right): https://codereview.chromium.org/26809004/diff/6001/content/renderer/render_view_impl.cc#newcode605 content/renderer/render_view_impl.cc:605: static bool ShouldUseUniversalAcceleratedCompositingForOverflowScroll() { Impressive function name!
7 years, 2 months ago (2013-10-11 04:58:12 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vollick@chromium.org/26809004/6001
7 years, 2 months ago (2013-10-11 12:15:38 UTC) #7
commit-bot: I haz the power
Change committed as 228187
7 years, 2 months ago (2013-10-11 15:22:19 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vollick@chromium.org/26809004/33001
7 years, 2 months ago (2013-10-17 21:02:01 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vollick@chromium.org/26809004/33001
7 years, 2 months ago (2013-10-18 01:30:48 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vollick@chromium.org/26809004/33001
7 years, 2 months ago (2013-10-18 02:08:55 UTC) #11
Ian Vollick
7 years, 2 months ago (2013-10-18 15:15:08 UTC) #12
Message was sent while issue was closed.
Committed patchset #3 manually as r229421 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698