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

Issue 11414308: Add vsync notification command line switch (Closed)

Created:
8 years ago by Sami
Modified:
7 years, 9 months ago
CC:
chromium-reviews, cc-bugs_chromium.org, jonathan.backer
Visibility:
Public.

Description

Add vsync notification command line switch This switch will make the renderer subscribe to a vsync notification from the browser instead of using a timer approximate the vsync pulse. BUG=149227 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190740

Patch Set 1 #

Patch Set 2 : Pipe setting from browser to renderer. #

Patch Set 3 : Rebased. #

Patch Set 4 : Rebased. #

Total comments: 1

Patch Set 5 : Also set flag in RenderWidgetCompositor::Create. #

Patch Set 6 : rebase #

Total comments: 2

Patch Set 7 : rebase #

Patch Set 8 : Move setting to content. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -0 lines) Patch
M cc/trees/layer_tree_settings.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M cc/trees/layer_tree_settings.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/login/chrome_restart_request.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/gpu/render_widget_compositor.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
Sami
8 years ago (2012-12-04 12:19:05 UTC) #1
jamesr
Can we do this as a LayerTreeHostSetting and pipe it in to the render process ...
8 years ago (2012-12-04 17:29:27 UTC) #2
Sami
On 2012/12/04 17:29:27, jamesr wrote: > Can we do this as a LayerTreeHostSetting and pipe ...
8 years ago (2012-12-04 17:43:24 UTC) #3
Sami
This version pipes the vsync notification setting from the browser to the renderer similarly as ...
7 years, 11 months ago (2013-01-04 18:47:22 UTC) #4
jamesr
I think this gets a lot simpler once https://codereview.chromium.org/11575049/ lands since you won't have to ...
7 years, 11 months ago (2013-01-04 19:04:08 UTC) #5
Sami
I'm not sure how https://codereview.chromium.org/11575049/ would change this as we'd still have to pass the ...
7 years, 11 months ago (2013-01-17 11:44:34 UTC) #6
Sami
Any love for this one?
7 years, 10 months ago (2013-02-07 19:21:16 UTC) #7
danakj
https://codereview.chromium.org/11414308/diff/13001/webkit/compositor_bindings/web_layer_tree_view_impl.cc File webkit/compositor_bindings/web_layer_tree_view_impl.cc (right): https://codereview.chromium.org/11414308/diff/13001/webkit/compositor_bindings/web_layer_tree_view_impl.cc#newcode44 webkit/compositor_bindings/web_layer_tree_view_impl.cc:44: settings.renderVSyncNotificationEnabled = webSettings.renderVSyncNotificationEnabled; You'll need to do this in ...
7 years, 10 months ago (2013-02-07 19:23:17 UTC) #8
Sami
Thanks Dana, fixed.
7 years, 10 months ago (2013-02-07 19:35:31 UTC) #9
mkosiba (inactive)
Hello! I've offered to help Sami get this (and dependent changes) in. PTAL, thanks!
7 years, 9 months ago (2013-03-19 19:11:14 UTC) #10
danakj
This needs a rebase https://codereview.chromium.org/11414308/diff/22001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/11414308/diff/22001/content/browser/renderer_host/render_process_host_impl.cc#newcode904 content/browser/renderer_host/render_process_host_impl.cc:904: cc::switches::kEnableVsyncNotification, alphabetical order add this ...
7 years, 9 months ago (2013-03-20 18:25:33 UTC) #11
mkosiba (inactive)
https://codereview.chromium.org/11414308/diff/22001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/11414308/diff/22001/content/browser/renderer_host/render_process_host_impl.cc#newcode904 content/browser/renderer_host/render_process_host_impl.cc:904: cc::switches::kEnableVsyncNotification, On 2013/03/20 18:25:33, danakj wrote: > alphabetical order ...
7 years, 9 months ago (2013-03-21 12:02:11 UTC) #12
mkosiba (inactive)
Ping? Sami verified this works together with https://codereview.chromium.org/12674030/
7 years, 9 months ago (2013-03-22 16:15:18 UTC) #13
jamesr
I don't think this belongs in cc::switches. To cc, this value comes in as a ...
7 years, 9 months ago (2013-03-22 17:41:38 UTC) #14
Sami
On 2013/03/22 17:41:38, jamesr wrote: > I don't think this belongs in cc::switches. To cc, ...
7 years, 9 months ago (2013-03-22 18:59:13 UTC) #15
jamesr
lgtm
7 years, 9 months ago (2013-03-22 21:47:26 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skyostil@chromium.org/11414308/34001
7 years, 9 months ago (2013-03-25 13:28:50 UTC) #17
commit-bot: I haz the power
Presubmit check for 11414308-34001 failed and returned exit status 1. INFO:root:Found 7 file(s). Running presubmit ...
7 years, 9 months ago (2013-03-25 13:28:54 UTC) #18
Sami
Thanks James. +{joi,sky,piman} for owners' review please.
7 years, 9 months ago (2013-03-25 13:34:03 UTC) #19
Sami
Sorry for not being more specific: piman: Could you please check chrome/browser/chromeos/login? sky: Same for ...
7 years, 9 months ago (2013-03-26 17:21:57 UTC) #20
sky
Hm. I don't see any changes to content/browser/renderer_host.
7 years, 9 months ago (2013-03-26 17:25:01 UTC) #21
sky
GAH! Never mind, I see it. Let me look.
7 years, 9 months ago (2013-03-26 17:25:59 UTC) #22
sky
content/browser/renderer_host/render_process_host_impl.cc LGTM
7 years, 9 months ago (2013-03-26 17:27:19 UTC) #23
piman
cc:backer LGTM for the flag, note though I foresee duplication with what Aura does wrt ...
7 years, 9 months ago (2013-03-26 17:28:10 UTC) #24
Sami
Thanks. Who should I talk to about the cros vsync changes?
7 years, 9 months ago (2013-03-26 17:30:50 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skyostil@chromium.org/11414308/34001
7 years, 9 months ago (2013-03-26 17:32:24 UTC) #26
piman
On Tue, Mar 26, 2013 at 10:30 AM, <skyostil@chromium.org> wrote: > Thanks. Who should I ...
7 years, 9 months ago (2013-03-26 17:33:03 UTC) #27
commit-bot: I haz the power
7 years, 9 months ago (2013-03-26 19:47:23 UTC) #28
Message was sent while issue was closed.
Change committed as 190740

Powered by Google App Engine
This is Rietveld 408576698