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

Issue 2042503002: ozone/platform/wayland: Add support for wl_output_interface (Closed)

Created:
4 years, 6 months ago by joone
Modified:
4 years, 2 months ago
CC:
chromium-reviews, kalyank, ozone-reviews_chromium.org, vignatti (out of this project), dshwang, tonikitoo
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

ozone/platform/wayland: Add support for wl_output_interface WaylandScreen is used to keep track of the current outputs (display) that are available to Wayland clients. This is a part of upstream work for the ozone-wayland project: https://github.com/01org/ozone-wayland BUG=578890 TEST=WaylandDisplayTest.Output Committed: https://crrev.com/de57ce2d6a43b6593c770f583ccf06b8779e3ccb Cr-Commit-Position: refs/heads/master@{#421157}

Patch Set 1 : #

Total comments: 4

Patch Set 2 : Applied review comments #

Total comments: 8

Patch Set 3 : remove WaylandDisplay::instance #

Total comments: 11

Patch Set 4 : hide the use of the base::Closure #

Total comments: 18

Patch Set 5 : WaylandDisplay => WaylandConnection, WaylandScreen => WaylandOutput #

Total comments: 8

Patch Set 6 : rebase on master #

Total comments: 10

Patch Set 7 : add OutputObserver #

Patch Set 8 : remove unused headers #

Patch Set 9 : Rebase on master #

Patch Set 10 : add comments and use std::unique_ptr #

Total comments: 4

Patch Set 11 : [For landding] update comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+251 lines, -0 lines) Patch
M ui/ozone/platform/wayland/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/fake_server.h View 1 2 3 4 5 6 7 8 9 11 chunks +31 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/fake_server.cc View 1 2 3 4 5 6 7 8 9 4 chunks +19 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/wayland_connection.h View 1 2 3 4 5 6 7 8 9 3 chunks +6 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/wayland_connection.cc View 1 2 3 4 5 6 7 8 9 10 4 chunks +25 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/wayland_connection_unittest.cc View 1 2 3 4 5 6 2 chunks +38 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/wayland_object.h View 2 chunks +7 lines, -0 lines 0 comments Download
M ui/ozone/platform/wayland/wayland_object.cc View 1 chunk +3 lines, -0 lines 0 comments Download
A ui/ozone/platform/wayland/wayland_output.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +64 lines, -0 lines 0 comments Download
A ui/ozone/platform/wayland/wayland_output.cc View 1 2 3 4 5 6 1 chunk +56 lines, -0 lines 0 comments Download

Messages

Total messages: 56 (21 generated)
joone
Please take a look at this CL.
4 years, 6 months ago (2016-06-03 22:46:48 UTC) #4
tonikitoo
So WaylandScreen is a wrapper around wl_output. Its main function is inform WaylandDisplay that the ...
4 years, 6 months ago (2016-06-06 21:35:15 UTC) #8
joone
Yes, WaylandScreen is a wrapper around wl_output. https://codereview.chromium.org/2042503002/diff/40001/ui/ozone/platform/wayland/wayland_display.cc File ui/ozone/platform/wayland/wayland_display.cc (right): https://codereview.chromium.org/2042503002/diff/40001/ui/ozone/platform/wayland/wayland_display.cc#newcode73 ui/ozone/platform/wayland/wayland_display.cc:73: instance_ = ...
4 years, 6 months ago (2016-06-07 22:27:00 UTC) #9
joone
On 2016/06/06 21:35:15, tonikitoo1 wrote: > So WaylandScreen is a wrapper around wl_output. > > ...
4 years, 6 months ago (2016-06-07 22:30:06 UTC) #10
joone
forney@ hi, could you review my CL?
4 years, 6 months ago (2016-06-10 15:45:03 UTC) #11
Michael Forney
You should add one of the ozone people as a reviewer. https://codereview.chromium.org/2042503002/diff/60001/ui/ozone/platform/wayland/wayland_display.cc File ui/ozone/platform/wayland/wayland_display.cc (right): ...
4 years, 6 months ago (2016-06-10 18:12:21 UTC) #12
joone
I've updated the CL. Thanks for the review. https://codereview.chromium.org/2042503002/diff/60001/ui/ozone/platform/wayland/wayland_display.cc File ui/ozone/platform/wayland/wayland_display.cc (right): https://codereview.chromium.org/2042503002/diff/60001/ui/ozone/platform/wayland/wayland_display.cc#newcode26 ui/ozone/platform/wayland/wayland_display.cc:26: WaylandDisplay* ...
4 years, 6 months ago (2016-06-14 00:04:44 UTC) #13
Michael Forney
https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/fake_server.cc File ui/ozone/platform/wayland/fake_server.cc (right): https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/fake_server.cc#newcode292 ui/ozone/platform/wayland/fake_server.cc:292: } Maybe instead you could add a virtual method ...
4 years, 6 months ago (2016-06-14 00:55:18 UTC) #15
not to use - tonikitoo
https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/wayland_screen.cc File ui/ozone/platform/wayland/wayland_screen.cc (right): https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/wayland_screen.cc#newcode26 ui/ozone/platform/wayland/wayland_screen.cc:26: void WaylandScreen::SetOutputCompleteClosure(const base::Closure& closure) { On 2016/06/14 00:55:18, Michael ...
4 years, 6 months ago (2016-06-14 15:53:34 UTC) #17
joone
forney@ tonikitoo1@ thanks for the review. https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/fake_server.cc File ui/ozone/platform/wayland/fake_server.cc (right): https://codereview.chromium.org/2042503002/diff/80001/ui/ozone/platform/wayland/fake_server.cc#newcode292 ui/ozone/platform/wayland/fake_server.cc:292: } On 2016/06/14 ...
4 years, 6 months ago (2016-06-14 22:10:28 UTC) #18
Michael Forney
https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.cc File ui/ozone/platform/wayland/wayland_screen.cc (right): https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.cc#newcode65 ui/ozone/platform/wayland/wayland_screen.cc:65: run_loop.Run(); I think base::WaitableEvent may be more appropriate for ...
4 years, 6 months ago (2016-06-14 23:25:33 UTC) #19
rjkroege
Conceivably, I am confused as the intent of this CL. Is this for a desktop ...
4 years, 6 months ago (2016-06-14 23:29:49 UTC) #20
Michael Forney
https://codereview.chromium.org/2042503002/diff/100001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2042503002/diff/100001/ui/display/screen.h#newcode92 ui/display/screen.h:92: virtual void OnOutputGeometryChanged(int32_t name, const gfx::Rect& rect) { } ...
4 years, 6 months ago (2016-06-14 23:44:54 UTC) #21
joone
https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.cc File ui/ozone/platform/wayland/wayland_screen.cc (right): https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.cc#newcode65 ui/ozone/platform/wayland/wayland_screen.cc:65: run_loop.Run(); On 2016/06/14 23:25:33, Michael Forney wrote: > I ...
4 years, 6 months ago (2016-06-16 22:44:23 UTC) #22
joone
forney@ rjkroege@ thanks for the review. Could you take a look at the updated CL? ...
4 years, 6 months ago (2016-06-20 22:22:43 UTC) #24
joone
forney@ rjkroege@ ping?
4 years, 5 months ago (2016-06-29 17:50:43 UTC) #27
Michael Forney
https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.h File ui/ozone/platform/wayland/wayland_screen.h (right): https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.h#newcode20 ui/ozone/platform/wayland/wayland_screen.h:20: // WaylandScreen objects keep track of the current outputs ...
4 years, 5 months ago (2016-07-07 21:18:01 UTC) #28
rjkroege
Can someone point me at a design doc for the wayland platform implementation please? Or ...
4 years, 5 months ago (2016-07-08 21:59:19 UTC) #29
Michael Forney
https://codereview.chromium.org/2042503002/diff/140001/ui/ozone/platform/wayland/wayland_connection.cc File ui/ozone/platform/wayland/wayland_connection.cc (right): https://codereview.chromium.org/2042503002/diff/140001/ui/ozone/platform/wayland/wayland_connection.cc#newcode7 ui/ozone/platform/wayland/wayland_connection.cc:7: #include <xdg-shell-unstable-v5-client-protocol.h> On 2016/07/08 21:59:19, rjkroege wrote: > As ...
4 years, 5 months ago (2016-07-08 22:28:44 UTC) #30
joone
On 2016/07/08 21:59:19, rjkroege wrote: > Can someone point me at a design doc for ...
4 years, 5 months ago (2016-07-11 20:08:18 UTC) #31
rjkroege
On 2016/07/11 20:08:18, joone wrote: > On 2016/07/08 21:59:19, rjkroege wrote: > > Can someone ...
4 years, 5 months ago (2016-07-11 21:46:21 UTC) #32
joone
https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.h File ui/ozone/platform/wayland/wayland_screen.h (right): https://codereview.chromium.org/2042503002/diff/100001/ui/ozone/platform/wayland/wayland_screen.h#newcode20 ui/ozone/platform/wayland/wayland_screen.h:20: // WaylandScreen objects keep track of the current outputs ...
4 years, 5 months ago (2016-07-12 19:22:41 UTC) #33
joone
https://codereview.chromium.org/2042503002/diff/140001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2042503002/diff/140001/ui/display/screen.h#newcode92 ui/display/screen.h:92: virtual void OnOutputGeometryChanged(int32_t name, const gfx::Rect& rect) { } ...
4 years, 5 months ago (2016-07-20 00:07:20 UTC) #38
joone
On 2016/07/11 21:46:21, rjkroege wrote: > On 2016/07/11 20:08:18, joone wrote: > > On 2016/07/08 ...
4 years, 5 months ago (2016-07-20 20:56:18 UTC) #39
Michael Forney
https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h File ui/ozone/platform/wayland/wayland_output.h (right): https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h#newcode30 ui/ozone/platform/wayland/wayland_output.h:30: const base::Closure closure_; Why the closure? Why not just ...
4 years, 5 months ago (2016-07-20 21:49:01 UTC) #40
joone
https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h File ui/ozone/platform/wayland/wayland_output.h (right): https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h#newcode30 ui/ozone/platform/wayland/wayland_output.h:30: const base::Closure closure_; On 2016/07/20 21:49:00, Michael Forney wrote: ...
4 years, 5 months ago (2016-07-20 23:23:59 UTC) #41
joone
On 2016/07/20 23:23:59, joone wrote: > https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h > File ui/ozone/platform/wayland/wayland_output.h (right): > > https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/wayland_output.h#newcode30 > ...
4 years, 5 months ago (2016-07-20 23:32:31 UTC) #42
joone
Hi forney@, I added a subclass of Observer to wayland_connection_unittest.cc. Please take a look at ...
4 years, 5 months ago (2016-07-23 00:07:30 UTC) #45
rjkroege
I forget to send these from an older version of the CL. If they make ...
4 years, 5 months ago (2016-07-25 23:45:47 UTC) #46
joone
Hi rjkroege@ I've updated the CL. Please take a look it. Thanks! https://codereview.chromium.org/2042503002/diff/160001/ui/ozone/platform/wayland/fake_server.h File ui/ozone/platform/wayland/fake_server.h ...
4 years, 3 months ago (2016-09-21 21:21:22 UTC) #47
rjkroege
lgtm with two nits. Please fix before CQ-ing. https://codereview.chromium.org/2042503002/diff/240001/ui/ozone/platform/wayland/wayland_connection.cc File ui/ozone/platform/wayland/wayland_connection.cc (right): https://codereview.chromium.org/2042503002/diff/240001/ui/ozone/platform/wayland/wayland_connection.cc#newcode184 ui/ozone/platform/wayland/wayland_connection.cc:184: } ...
4 years, 2 months ago (2016-09-25 14:44:31 UTC) #48
joone
Thanks for review! https://codereview.chromium.org/2042503002/diff/240001/ui/ozone/platform/wayland/wayland_connection.cc File ui/ozone/platform/wayland/wayland_connection.cc (right): https://codereview.chromium.org/2042503002/diff/240001/ui/ozone/platform/wayland/wayland_connection.cc#newcode184 ui/ozone/platform/wayland/wayland_connection.cc:184: } else if (strcmp(interface, "wl_output") == ...
4 years, 2 months ago (2016-09-27 08:25:20 UTC) #49
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/2042503002/260001
4 years, 2 months ago (2016-09-27 08:26:48 UTC) #52
commit-bot: I haz the power
Committed patchset #11 (id:260001)
4 years, 2 months ago (2016-09-27 09:25:56 UTC) #54
commit-bot: I haz the power
4 years, 2 months ago (2016-09-27 09:28:04 UTC) #56
Message was sent while issue was closed.
Patchset 11 (id:??) landed as
https://crrev.com/de57ce2d6a43b6593c770f583ccf06b8779e3ccb
Cr-Commit-Position: refs/heads/master@{#421157}

Powered by Google App Engine
This is Rietveld 408576698