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

Issue 2323393003: Connect mojom::DisplayController from ash to ui. (Closed)

Created:
4 years, 3 months ago by kylechar
Modified:
4 years, 3 months ago
Reviewers:
sky, dcheng
CC:
chromium-reviews, rjkroege, sadrul, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, kalyank, darin (slow to review)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Connect mojom::DisplayController from ash to ui. Connect mojom::DisplayController and the debug keyboard accelerator to add or remove a display. The accelerator is triggered in mojo:ash and the display change happens in mojo:ui. Remove the IPC response as it's unneeded. The actual adding/removing of a display isn't fully working yet but the accelerator and Mojo IPC works as intended. BUG=611475 Committed: https://crrev.com/80cc8b1af214a4caba50a0a8794622b2103a2129 Cr-Commit-Position: refs/heads/master@{#418254}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Add comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -18 lines) Patch
M ash/mus/BUILD.gn View 1 1 chunk +1 line, -0 lines 0 comments Download
M ash/mus/accelerators/accelerator_controller_delegate_mus.cc View 2 chunks +7 lines, -1 line 0 comments Download
M ash/mus/manifest.json View 1 1 chunk +1 line, -0 lines 0 comments Download
M services/ui/display/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M services/ui/display/platform_screen.h View 2 chunks +7 lines, -0 lines 0 comments Download
M services/ui/display/platform_screen_ozone.h View 3 chunks +17 lines, -6 lines 0 comments Download
M services/ui/display/platform_screen_ozone.cc View 4 chunks +15 lines, -9 lines 0 comments Download
M services/ui/display/platform_screen_stub.h View 1 chunk +1 line, -0 lines 0 comments Download
M services/ui/display/platform_screen_stub.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M services/ui/public/interfaces/display/display_controller.mojom View 1 chunk +1 line, -1 line 0 comments Download
M services/ui/service.h View 1 2 chunks +5 lines, -1 line 0 comments Download
M services/ui/service.cc View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 25 (14 generated)
kylechar
+sky for ash/* and services/ui* +tsepez for *.mojom
4 years, 3 months ago (2016-09-09 20:06:07 UTC) #2
sky
LGTM
4 years, 3 months ago (2016-09-09 22:03:48 UTC) #5
kylechar
-tsepez as he's on vacation, +dcheng for security review.
4 years, 3 months ago (2016-09-12 13:58:03 UTC) #7
dcheng
https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc File services/ui/display/platform_screen_ozone.cc (right): https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc#newcode42 services/ui/display/platform_screen_ozone.cc:42: registry->AddInterface<mojom::DisplayController>(this); Does this have other InterfaceFactory superclasses? Otherwise, I ...
4 years, 3 months ago (2016-09-12 18:04:30 UTC) #12
kylechar
https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc File services/ui/display/platform_screen_ozone.cc (right): https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc#newcode42 services/ui/display/platform_screen_ozone.cc:42: registry->AddInterface<mojom::DisplayController>(this); On 2016/09/12 18:04:30, dcheng wrote: > Does this ...
4 years, 3 months ago (2016-09-12 18:28:15 UTC) #13
dcheng
mojo lgtm but I think there might be some good long-term actions here, wrt documentation ...
4 years, 3 months ago (2016-09-13 01:04:48 UTC) #14
sky
https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc File services/ui/display/platform_screen_ozone.cc (right): https://codereview.chromium.org/2323393003/diff/1/services/ui/display/platform_screen_ozone.cc#newcode42 services/ui/display/platform_screen_ozone.cc:42: registry->AddInterface<mojom::DisplayController>(this); On 2016/09/13 01:04:48, dcheng wrote: > On 2016/09/12 ...
4 years, 3 months ago (2016-09-13 02:07:40 UTC) #15
kylechar
https://codereview.chromium.org/2323393003/diff/1/services/ui/service.cc File services/ui/service.cc (right): https://codereview.chromium.org/2323393003/diff/1/services/ui/service.cc#newcode215 services/ui/service.cc:215: platform_screen_->AddInterfaces(registry); On 2016/09/13 01:04:48, dcheng wrote: > On 2016/09/12 ...
4 years, 3 months ago (2016-09-13 14:09:53 UTC) #18
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/2323393003/60001
4 years, 3 months ago (2016-09-13 14:10:21 UTC) #21
commit-bot: I haz the power
Committed patchset #2 (id:60001)
4 years, 3 months ago (2016-09-13 15:21:22 UTC) #23
commit-bot: I haz the power
4 years, 3 months ago (2016-09-13 15:25:01 UTC) #25
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/80cc8b1af214a4caba50a0a8794622b2103a2129
Cr-Commit-Position: refs/heads/master@{#418254}

Powered by Google App Engine
This is Rietveld 408576698