New issue
Advanced search Search tips

Issue 875161 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug


Sign in to add a comment

[ozone/wayland] Implement PlatformScreen for Wayland.

Project Member Reported by msi...@igalia.com, Aug 17

Issue description

Implement PlatformScreen for Wayland.
 
Blocking: 578890 872339
At the moment, Wayland uses NativeDisplayDelegate and DesktopScreenOzone. The first one is more intended for drm like backends and involves unnecessary functionality.  
Labels: Wayland
Summary: [ozone/wayland] Implement PlatformScreen for Wayland. (was: Implement PlatformScreen for Wayland.)
Owner: msi...@igalia.com
Status: Started (was: Available)
Blockedon: 890271
Blockedon: 890272
Blockedon: 890276
Components: UI
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd70a2cdfe9357fa1f9d1e525629d69648533065

commit dd70a2cdfe9357fa1f9d1e525629d69648533065
Author: Maksim Sisov <msisov@igalia.com>
Date: Wed Oct 03 07:42:26 2018

[ozone/wayland] Migrate to initial WaylandScreen implementation.

This patch makes use of the PlatformScreen and ScreenOzone implementation.

At this point, only basic functionality is added. Multi-display
and other features will be added in a follow-up CL.

How display fetching mechanism works:
During the WaylandConnection::Initialize process, a Wayland compositor
announces available global objects including a wl_output, which
corresponds to one physical display.

However, at this step, the WaylandScreen is not created, but
the interface must be bound. Otherwise, we won't get properties
of a wl_output aka physical display until something is changed
on the display.

That is, when a wl_output is announced, the WaylandConnection
calls to a WaylandOutputManager, which creates a WaylandOutput wrapper
class, and binds to the announced interface. As soon as the interface
is bound, the Wayland compositor starts to send properties of
the corresponding display through a wl_output_listener interface.

After this, the WaylandOutput starts to receive property events, which
are always stored and sent to a Delegate through a
WaylandOutput::Delegate::OnOutputHandleMetrics, which WaylandOutputManager
implements. The WaylandOutputManager then forwards those calls
to the WaylandScreen (derived from the PlatformScreen), which it has
a non-owned pointer to.

Talking again about the WaylandScreen, once it is created,
WaylandOutputManager has to manually tell the WaylandScreen about the existing
WaylandOutputs and share their ids. The WaylandScreen starts to maintain
a DisplayList based on those ids. Then, the manager calls the
WaylandOutput::TriggerDelegateNotification() method, which triggers
property update notifications of the existing WaylandOutputs.

Once the browser is running, and the WaylandScreen exists, there is
no need to manually trigger the updates. Instead, the WaylandOutputs
immediately send the updates on each property change as described above.

In the follow up cls, I will add more functionality and implement
not implemented methods.

Bug: 875161
Change-Id: I8d533149641bd5ee1970cd36c3c8601d5a81b7db
Reviewed-on: https://chromium-review.googlesource.com/c/1236268
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596144}
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/BUILD.gn
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/fake_server.cc
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/ozone_platform_wayland.cc
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_connection.h
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_connection_unittest.cc
[delete] https://crrev.com/2a5964302c3466f9c05829ae399ac622f003e858/ui/ozone/platform/wayland/wayland_native_display_delegate.cc
[delete] https://crrev.com/2a5964302c3466f9c05829ae399ac622f003e858/ui/ozone/platform/wayland/wayland_native_display_delegate.h
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_output.h
[add] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_output_manager.cc
[add] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_output_manager.h
[add] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_screen.cc
[add] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_screen.h
[add] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/ozone/platform/wayland/wayland_screen_unittest.cc
[modify] https://crrev.com/dd70a2cdfe9357fa1f9d1e525629d69648533065/ui/views/widget/desktop_aura/desktop_screen_ozone.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14de2307d2980ecb5282767d3bef6bd568ad282b

commit 14de2307d2980ecb5282767d3bef6bd568ad282b
Author: Maksim Sisov <msisov@igalia.com>
Date: Wed Oct 03 08:42:17 2018

[ozone/wayland] Add remove output functionality

This CL adds a new functionality to the existing WaylandOutputManager
and WaylandScreen classes. Now, it is possible to remove and
change primary displays on hot plugs and removes.

To test that functionality, a new test has also been added.

The logic that assigns output ids has also been changed.
Now, we are using numeric names that a Wayland compositor
announces during Global and GlobalRemove calls. That is,
each global object is assigned an unique numeric name, which can
be freely used as an id.

Bug:  890276 , 875161
Change-Id: I775bdd40a946c9f0c5dde89eba6e36c4615e1f21
Reviewed-on: https://chromium-review.googlesource.com/c/1254146
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596155}
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/fake_server.h
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_output.h
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/14de2307d2980ecb5282767d3bef6bd568ad282b/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67

commit b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed Oct 03 15:32:53 2018

Revert "[ozone/wayland] Add remove output functionality"

This reverts commit 14de2307d2980ecb5282767d3bef6bd568ad282b.

Reason for revert: Newly added test WaylandScreenTest.MultipleOutputsAddedAndRemoved fails on ChromeOS ASAN/LSAN; see  crbug.com/891718 

Original change's description:
> [ozone/wayland] Add remove output functionality
> 
> This CL adds a new functionality to the existing WaylandOutputManager
> and WaylandScreen classes. Now, it is possible to remove and
> change primary displays on hot plugs and removes.
> 
> To test that functionality, a new test has also been added.
> 
> The logic that assigns output ids has also been changed.
> Now, we are using numeric names that a Wayland compositor
> announces during Global and GlobalRemove calls. That is,
> each global object is assigned an unique numeric name, which can
> be freely used as an id.
> 
> Bug:  890276 , 875161
> Change-Id: I775bdd40a946c9f0c5dde89eba6e36c4615e1f21
> Reviewed-on: https://chromium-review.googlesource.com/c/1254146
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596155}

TBR=rjkroege@chromium.org,msisov@igalia.com

Change-Id: I8ae1f0fa1df71b464096abd0fcd504c1d4f5f480
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  890276 , 875161,  891718 
Reviewed-on: https://chromium-review.googlesource.com/c/1258914
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596221}
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/fake_server.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42776a4625e1aa02206a460934d4ce240386e2f6

commit 42776a4625e1aa02206a460934d4ce240386e2f6
Author: Maksim Sisov <msisov@igalia.com>
Date: Tue Oct 09 14:54:18 2018

Reland [ozone/wayland] Add remove output functionality

This patch relands 14de2307d2980ecb5282767d3bef6bd568ad282b.

The problem was that global output objects were destroyed
before the client had unbound from those outputs, which
resulted in the stack-use-after-return problem.

To address the issue, new global objects are now stored by
the FakeServer and destroyed upon its destruction now. From
now on, the fix makes it sure that the FakeServer follows
the Wayland documentation, which says that global objects
must be kept alive until a client unbounds from them.

* Original CL's description:

> This CL adds a new functionality to the existing WaylandOutputManager
> and WaylandScreen classes. Now, it is possible to remove and
> change primary displays on hot plugs and removes.
>
> To test that functionality, a new test has also been added.
>
> The logic that assigns output ids has also been changed.
> Now, we are using numeric names that a Wayland compositor
> announces during Global and GlobalRemove calls. That is,
> each global object is assigned an unique numeric name, which can
> be freely used as an id.
>
> Bug:  890276 , 875161
> Change-Id: I775bdd40a946c9f0c5dde89eba6e36c4615e1f21
> Reviewed-on: https://chromium-review.googlesource.com/c/1254146
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596155}

Bug:  890276 , 875161,  891718 
Change-Id: I2f8aeebcfb67bda75d7d1ba0b0cd331a5eb50c6f
Reviewed-on: https://chromium-review.googlesource.com/c/1268020
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#597912}
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/fake_server.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Labels: Hotlist-DesktopUIValid Hotlist-DesktopUIChecked
***Mass UI Triage***

msisov@ could you please help in verifying the issue?
jmukthavaram@, the issue has been completed partly. But some uncertainties are to be fixed yet.
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c1fe59f004876cb3061e21f4889f57fd56ecdbe

commit 0c1fe59f004876cb3061e21f4889f57fd56ecdbe
Author: Maksim Sisov <msisov@igalia.com>
Date: Mon Jan 14 07:48:48 2019

[ozone/wayland]: WaylandScreen: return widget on screen point.

This change implements a method, which returns a widget on
a screen point. This method is needed, for example, to
properly update cursors (done by
RenderWidgetHostViewAura::UpdateCursorIfOverSelf) and etc.

As long as it's not possible to get windows on global
screen cordinates, use a focused windows instead (
the one under a pointer), and check if it contains the point.

Bug: 875161

Change-Id: I7187d54ea38ac3196503758de93e06e7882042d9
Reviewed-on: https://chromium-review.googlesource.com/c/1402882
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#622393}
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/ozone_platform_wayland.cc
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_object.h
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_screen.cc
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_screen.h
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_screen_unittest.cc
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_window.cc
[modify] https://crrev.com/0c1fe59f004876cb3061e21f4889f57fd56ecdbe/ui/ozone/platform/wayland/wayland_window.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/985ec97fc79f3581c524e51f48af9b7565f33e08

commit 985ec97fc79f3581c524e51f48af9b7565f33e08
Author: Maksim Sisov <msisov@igalia.com>
Date: Mon Jan 14 16:49:52 2019

[ozone/wayland]: Switch to wl_output version 2.

wl_output version 2 has some benefits over the previous
version we used to bind to: support of scale values
(in version 1, it always sends value 1), OutputDone
events, which notifies that changes WaylandOutput through that
OutputHandleGeometry, or OutputHandleMode, or OutputHandleScale
has been sent and it's time to notify WaylandOutput
clients about the changes.

Change-Id: I0ccf83357e755a29c30531bd6b7d2a3e13acfd3c
Bug: 875161
Reviewed-on: https://chromium-review.googlesource.com/c/1408974
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#622479}
[modify] https://crrev.com/985ec97fc79f3581c524e51f48af9b7565f33e08/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/985ec97fc79f3581c524e51f48af9b7565f33e08/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/985ec97fc79f3581c524e51f48af9b7565f33e08/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/985ec97fc79f3581c524e51f48af9b7565f33e08/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Comment 17 by nickdi...@igalia.com, Jan 18 (4 days ago)

Cc: nickdi...@igalia.com

Comment 18 by nickdi...@igalia.com, Jan 18 (4 days ago)

Blocking: 896640

Comment 19 by msi...@igalia.com, Today (22 hours ago)

Blockedon: 924088

Sign in to add a comment