New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 734668 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 731255



Sign in to add a comment

Investigate usage of OzonePlatform in content

Project Member Reported by sky@chromium.org, Jun 19 2017

Issue description

There are a handful of calls from content to OzonePlatform. These won't work in the mus world. We need to make sure we don't hit these code paths, and add DEPS exclusions to make sure we don't add new ones in the future.

 

Comment 1 by sky@chromium.org, Aug 15 2017

Owner: e...@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by e...@chromium.org, Aug 16 2017

* Auditing Ozone usage in //content
*** //content/browser/compositor/gpu_process_transport_factory.cc
***** Only creator of //content/browser/compositor/software_output_device_ozone.cc
******* Not a problem since this is guarded by a check to aura::Env::mode().
***** In case of --enable-hardware-overlays, will use ozone's OverlayManager.
*** //content/browser/compositor/software_output_device_ozone.cc
***** Only used in GpuProcessTransportFactory above.
*** //content/browser/gpu/gpu_data_manager_impl_private.cc
***** Only uses ozone switches.
*** //content/browser/gpu/gpu_process_host.cc
***** Glue code that delivers IPC::Message s to Ozone.
***** May be unused in mojo land? I couldn't get them called in --mus.
*** //content/browser/renderer_host/render_process_host_impl.cc
***** Only uses ozone switches.
*** //content/common/cursors/webcursor_ozone.cc
***** Deeply intertwined with CursorFactoryOzone.
***** Previous work was done to make this work in --mus and --mash.
*** //content/gpu/gpu_child_thread.cc
***** GPUChildThread doesn't appear to be instantiated in --mus builds.
***** (But it is instantiated in classic ash.)
*** //content/gpu/gpu_main.cc
***** Doesn't get executed in --mus.
***** (It's just used to create a base::MessageLoop, anyway.)
*** //content/renderer/renderer_main.cc
***** This is in the renderer, but it then shells out to ozone.
***** I think this is safe?
***** This is done in the renderer which already doesn't have access to ozone.

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 17 2017

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

commit 2f18c48d9bc872338fa02eda3778461b4f091139
Author: Elliot Glaysher <erg@chromium.org>
Date: Thu Aug 17 18:18:44 2017

Constrict DEPS so //content/ can't add further dependencies on ozone.

Under --mus and --mash, the code in //content/ can't access the raw
ozone devices. It appears the most of the uses of ozone aren't used in
mus builds anyway. Further constrain DEPS so new instances can't be
added.

Bug:  734668 
Change-Id: I5b8661aa4c000a58602ffce0e6f3e9cf35cff87e
Reviewed-on: https://chromium-review.googlesource.com/617591
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495228}
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/DEPS
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/browser/compositor/DEPS
[add] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/browser/gpu/DEPS
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/browser/renderer_host/DEPS
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/common/cursors/DEPS
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/gpu/DEPS
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/public/test/content_test_suite_base.cc
[modify] https://crrev.com/2f18c48d9bc872338fa02eda3778461b4f091139/content/renderer/DEPS

Comment 4 by e...@chromium.org, Aug 17 2017

The only potentially unwanted use of ozone in content which occurs in --mus is in RenderMain:

  g_pixmap_factory.Get() = ui::CreateClientNativePixmapFactoryOzone();

This should be OK, in as much as we already build one of these objects not just in the renderer, but in every aura using process, but I'm not 100% sure of this.

Comment 5 by e...@chromium.org, Aug 17 2017

Status: Fixed (was: Assigned)
Given the comments in https://codereview.chromium.org/2803523002, I'm going to say that that final usage is ok.

Comment 6 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 16

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

commit 891067b2c40e0624546cb9d8ee5c9777b9760a3b
Author: Evan Stade <estade@chromium.org>
Date: Fri Nov 16 01:26:33 2018

Don't use ui::CursorData as PlatformCursor for mash clients.

ui::Cursor now holds the data necessary for custom cursors, although for
simplicity this patch still converts to CursorData for serializing over
mojo.

CursorFactoryOzone goes back to being a true singleton, which shouldn't
be used from client code in Mash (in Oop Mash, it's actually null; in
single process Mash it is avoided by checking the feature switch).

This patch also makes some tweaks to how custom web cursors are stored
as ui::Cursors, which shouldn't impact other platforms but brings the
ui::Cursor representation into alignment with the PlatformCursor
representation for ozone.

As a future cleanup we should be able to get rid of CursorData and
serialize ui::Cursor directly.

Bug:  904039 ,  734668 
Change-Id: I4168c0266899bd276c60a0ac6a286b3a79df572f
Reviewed-on: https://chromium-review.googlesource.com/c/1334968
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608617}
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ash/components/tap_visualizer/tap_visualizer_app_unittest.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ash/ws/ash_window_manager_unittest.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor.h
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor_aura.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor_aurawin.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor_aurax11.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor_ozone.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/content/common/cursors/webcursor_unittest.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ui/base/BUILD.gn
[delete] https://crrev.com/7de3a17b0ef130a5723f012829bb9170139394e8/ui/base/cursor/ozone/cursor_data_factory_ozone.cc
[delete] https://crrev.com/7de3a17b0ef130a5723f012829bb9170139394e8/ui/base/cursor/ozone/cursor_data_factory_ozone.h
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ui/ozone/public/cursor_factory_ozone.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ui/views/mus/mus_client.cc
[modify] https://crrev.com/891067b2c40e0624546cb9d8ee5c9777b9760a3b/ui/views/mus/mus_client.h

Sign in to add a comment