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

Issue 2854953006: Move disable_display_vsync out of RendererSettings (Closed)

Created:
3 years, 7 months ago by Alex Z.
Modified:
3 years, 7 months ago
Reviewers:
danakj, piman
CC:
chromium-reviews, Ian Vollick, jam, jbauman+watch_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, kalyank, cc-bugs_chromium.org, danakj+watch_chromium.org, Fady Samuel
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move disable_display_vsync out of RendererSettings disable_display_vsync is read by GpuProcessTransportFactory and TestCompositorFrameSink only. It is not used by Renderers so it should not be in RendererSettings. GpuProcessTransportFactory gets disable_display_vsync from command line switch and stores it in disable_display_vsync_. LayerTreeTest and its subclasses pass disable_display_vsync in to TestCompositorFrameSink via its constructor. LayerTreeHostPerfTest overrides LayerTreeTest::CreateCompositorFrameSink to set disable_display_vsync to true to preserve behaviors. BUG=717959 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel TBR=piman@chromium.org Review-Url: https://codereview.chromium.org/2854953006 Cr-Commit-Position: refs/heads/master@{#469439} Committed: https://chromium.googlesource.com/chromium/src/+/60f32e2dc84431ea6131e760fd5480b415aff5e8

Patch Set 1 #

Patch Set 2 : Fix LayoutTestSupport #

Total comments: 10

Patch Set 3 : Address Comments #

Patch Set 4 : Correct typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -28 lines) Patch
M cc/layers/texture_layer_unittest.cc View 2 chunks +2 lines, -1 line 0 comments Download
M cc/output/renderer_settings.h View 1 chunk +0 lines, -1 line 0 comments Download
M cc/test/layer_tree_pixel_test.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M cc/test/layer_tree_test.cc View 2 chunks +2 lines, -1 line 0 comments Download
M cc/test/test_compositor_frame_sink.h View 2 chunks +3 lines, -1 line 0 comments Download
M cc/test/test_compositor_frame_sink.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M cc/trees/layer_tree_host_impl_unittest.cc View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M cc/trees/layer_tree_host_perftest.cc View 1 2 2 chunks +13 lines, -6 lines 0 comments Download
M cc/trees/layer_tree_host_unittest.cc View 3 chunks +4 lines, -2 lines 0 comments Download
M content/browser/compositor/gpu_process_transport_factory.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/compositor/gpu_process_transport_factory.cc View 1 2 3 2 chunks +13 lines, -1 line 0 comments Download
M content/test/layouttest_support.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M ui/compositor/compositor.cc View 1 chunk +0 lines, -10 lines 0 comments Download

Messages

Total messages: 26 (19 generated)
Alex Z.
danakj@: PTAL.
3 years, 7 months ago (2017-05-03 21:51:11 UTC) #5
danakj
LGTM https://codereview.chromium.org/2854953006/diff/20001/cc/test/layer_tree_pixel_test.cc File cc/test/layer_tree_pixel_test.cc (right): https://codereview.chromium.org/2854953006/diff/20001/cc/test/layer_tree_pixel_test.cc#newcode45 cc/test/layer_tree_pixel_test.cc:45: constexpr bool kDisableDisplayVsync = false; can you put ...
3 years, 7 months ago (2017-05-04 15:36:36 UTC) #12
Alex Z.
piman@: Please review changes in content. https://codereview.chromium.org/2854953006/diff/20001/cc/test/layer_tree_pixel_test.cc File cc/test/layer_tree_pixel_test.cc (right): https://codereview.chromium.org/2854953006/diff/20001/cc/test/layer_tree_pixel_test.cc#newcode45 cc/test/layer_tree_pixel_test.cc:45: constexpr bool kDisableDisplayVsync ...
3 years, 7 months ago (2017-05-04 18:41:56 UTC) #15
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/2854953006/40001
3 years, 7 months ago (2017-05-04 18:42:43 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/builds/8930) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
3 years, 7 months ago (2017-05-04 18:54:26 UTC) #20
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/2854953006/60001
3 years, 7 months ago (2017-05-04 18:57:54 UTC) #23
commit-bot: I haz the power
3 years, 7 months ago (2017-05-04 20:05:19 UTC) #26
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/60f32e2dc84431ea6131e760fd54...

Powered by Google App Engine
This is Rietveld 408576698