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

Issue 2470143006: Remove dependency on SurfaceManager in FrameGenerator (Closed)

Created:
4 years, 1 month ago by Fady Samuel
Modified:
4 years, 1 month ago
Reviewers:
kylechar, sky
CC:
chromium-reviews, rjkroege
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove dependency on SurfaceManager in FrameGenerator FrameGenerator lives in the window server and thus should not have direct access to SurfaceManager. This CL moves FrameGenerator's accesses to surface manager to DisplayCompositor which will be placed behind a mojo interface in a subsequent CL. This CL takes us a step closer to removing the need for GpuChannelHost in mus-ws by decoupling mus-ws and the display compositor. BUG=661278 Committed: https://crrev.com/0146e3cdbf9163f044d0fc300a61747f98268878 Cr-Commit-Position: refs/heads/master@{#429971}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed nit #

Patch Set 3 : Pass vector by const ref like mojo would #

Patch Set 4 : Remove unnecessary display compositor references #

Patch Set 5 : Remove a couple of bad std::moves #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -53 lines) Patch
M services/ui/surfaces/display_compositor.h View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M services/ui/surfaces/display_compositor.cc View 1 2 2 chunks +19 lines, -5 lines 0 comments Download
M services/ui/ws/display_manager.cc View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M services/ui/ws/frame_generator.h View 1 2 3 2 chunks +3 lines, -4 lines 0 comments Download
M services/ui/ws/frame_generator.cc View 1 2 3 4 3 chunks +11 lines, -20 lines 0 comments Download
M services/ui/ws/frame_generator_unittest.cc View 1 2 3 4 chunks +2 lines, -5 lines 0 comments Download
M services/ui/ws/platform_display.cc View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M services/ui/ws/platform_display_init_params.h View 1 2 3 2 chunks +0 lines, -4 lines 0 comments Download
M services/ui/ws/test_utils.cc View 1 2 3 2 chunks +1 line, -6 lines 0 comments Download
M services/ui/ws/window_tree.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M services/ui/ws/window_tree_host_factory.cc View 1 2 3 2 chunks +0 lines, -3 lines 0 comments Download

Messages

Total messages: 22 (14 generated)
Fady Samuel
4 years, 1 month ago (2016-11-04 15:36:32 UTC) #2
kylechar
lgtm https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc#newcode1379 services/ui/ws/window_tree.cc:1379: sequences.push_back(sequence.sequence); Optional change: std::vector<uint32_t> sequences({sequence.sequence});
4 years, 1 month ago (2016-11-04 15:53:51 UTC) #3
Fady Samuel
+sky@ for OWNERS https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc#newcode1379 services/ui/ws/window_tree.cc:1379: sequences.push_back(sequence.sequence); On 2016/11/04 15:53:51, kylechar wrote: ...
4 years, 1 month ago (2016-11-04 17:17:06 UTC) #6
Fady Samuel
+sky@ for OWNERS https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2470143006/diff/1/services/ui/ws/window_tree.cc#newcode1379 services/ui/ws/window_tree.cc:1379: sequences.push_back(sequence.sequence); On 2016/11/04 15:53:51, kylechar wrote: ...
4 years, 1 month ago (2016-11-04 17:17:06 UTC) #7
sky
LGTM
4 years, 1 month ago (2016-11-04 17:58:18 UTC) #14
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/2470143006/80001
4 years, 1 month ago (2016-11-04 19:08:26 UTC) #19
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 1 month ago (2016-11-04 19:16:25 UTC) #20
commit-bot: I haz the power
4 years, 1 month ago (2016-11-04 19:18:50 UTC) #22
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/0146e3cdbf9163f044d0fc300a61747f98268878
Cr-Commit-Position: refs/heads/master@{#429971}

Powered by Google App Engine
This is Rietveld 408576698