New issue
Advanced search Search tips

Issue 722527 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Move ash and mus mojo services into the same process

Project Member Reported by mfomitchev@chromium.org, May 15 2017

Issue description

- Mus should still be a separate service: ash and Mus should only talk to each other over Mojo.
- Ensure that there are DEPS rules to ensure Mus/Ash don't talk to each other directly.
- Make sure Mus's thread runs with the highest priority.
 

Comment 1 by sky@chromium.org, Jun 8 2017

Blocking: 731255
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13 2017

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

commit 04cdb9269087e39efc12fc8dc21184bce075d72d
Author: mfomitchev <mfomitchev@chromium.org>
Date: Thu Jul 13 22:53:28 2017

Add support to the UI Service to run it insider browser's process.

When UI Service runs inside the browser process in its own separate thread,
there is a few things it needs to do differently:

 1. It shouldn't set its Screen as a global singleton.
 2. It shouldn't set its ClientNativePixmapFactory as a global singleton.
 3. It can't access the ResourceBundle from its own thread - that can only be
    done from the browser thread.

The third point is the source of most of the complexity in this CL. UI Service
needs to manipulate the cursor, which is done through the ui::ImageCursors
object, which needs to load resources. Since resources can't be loaded on the
UI Service's thread, a mechanism is introduced to let the UI Service do this by
posting tasks to the browser's thread task runner, which is passed into the
UI Service's constructor.

This CL doesn't contain browser-side changes to run the UI Service in-process.

BUG= 722527 

Review-Url: https://codereview.chromium.org/2979933002
Cr-Commit-Position: refs/heads/master@{#486509}

[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/common/BUILD.gn
[add] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/common/image_cursors_set.cc
[add] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/common/image_cursors_set.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/display/screen_manager_forwarding.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/display/screen_manager_forwarding.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/main.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/service.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/service.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/BUILD.gn
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/display.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/platform_display.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/platform_display.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/platform_display_default.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/platform_display_default_unittest.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/test_utils.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/test_utils.h
[add] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/threaded_image_cursors.cc
[add] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/threaded_image_cursors.h
[add] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/threaded_image_cursors_factory.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/user_display_manager.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/window_server.cc
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/window_server.h
[modify] https://crrev.com/04cdb9269087e39efc12fc8dc21184bce075d72d/services/ui/ws/window_server_delegate.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 13 2017

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

commit f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f
Author: mfomitchev <mfomitchev@chromium.org>
Date: Thu Jul 13 23:33:08 2017

The changes are as follows:

1. InputDeviceManager becomes a thread-local singleton instead of a global
   singleton. UI Service instantiates DeviceDataManager, which is the service-
   side implementation of InputDeviceManager, while the browser uses
   InputDeviceClient, which is the client-side implementation.

2. PlatformEventSource also becomes a thread-local singleton. This is to prevent
   ash and the browser from executing code on the UI Service's thread by
   registering PlatformEventObservers (e.g. via UserActivityDetector). In the
   futurer we may need a client-side implementation of PlatformEventSource.

3. GPU Service checks if a PowerMonitor singleton exists before creating one.
   This is temporary until GPU Service runs in a separate process from the UI
   Service. (crbug.com/609317).

BUG= 722527 

Review-Url: https://codereview.chromium.org/2978873002
Cr-Commit-Position: refs/heads/master@{#486534}

[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/services/ui/gpu/gpu_main.h
[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/ui/events/devices/input_device_manager.cc
[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/ui/events/devices/input_device_manager.h
[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/ui/events/platform/platform_event_source.cc
[modify] https://crrev.com/f76cad2a7e88da1ddc5a7a2df053cee1c4f3c02f/ui/events/platform/platform_event_source.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 14 2017

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

commit a872e762ddd650e53e093a4c68342eb637d5a931
Author: mfomitchev <mfomitchev@chromium.org>
Date: Fri Jul 14 06:03:44 2017

Make chrome --mus run the UI Service inside the browser process.

This CL takes advantage of the mechanisms introduced in
https://codereview.chromium.org/2979933002/ to run the UI Service inside the
browser process in Mushrome (i.e. when chrome is launched with --mus parameter).

UI Service runs on a separate thread with high priority. For now both the UI
Service and the GPU Service are moved. Once WS/GPU split if finished, GPU will
need to be moved into its separate process.

CrOS BrowserProcessPlatformPart owns the ImageCursorsSet used by the UI Service
for cursor manipulation.

BUG= 722527 

Review-Url: https://codereview.chromium.org/2983443002
Cr-Commit-Position: refs/heads/master@{#486688}

[modify] https://crrev.com/a872e762ddd650e53e093a4c68342eb637d5a931/chrome/app/BUILD.gn
[modify] https://crrev.com/a872e762ddd650e53e093a4c68342eb637d5a931/chrome/app/mash/BUILD.gn
[modify] https://crrev.com/a872e762ddd650e53e093a4c68342eb637d5a931/chrome/browser/DEPS
[modify] https://crrev.com/a872e762ddd650e53e093a4c68342eb637d5a931/chrome/browser/browser_process_platform_part_chromeos.cc
[modify] https://crrev.com/a872e762ddd650e53e093a4c68342eb637d5a931/chrome/browser/browser_process_platform_part_chromeos.h

Mus now runs inside the browser process in Mushrome (i.e. when launched via chrome --mus).

We now need to move Mus inside the Ash process for in Mustash (i.e. in chrome --mash). Ideally we should have the WindowManagerApplication service bundle Mus.

Comment 8 by sky@chromium.org, Jul 19 2017

Blocking: -731255
As this is fixed for mushrome this no longer blocks 731255.
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 16 2017

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

commit 477ab563e4dc51a19587a3372c3d487c66f6d906
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Aug 16 22:29:34 2017

Factor Services related logic out of BrowserProcessPlatforPart{chromeos}

CL introduces a EmbeddedUIServiceInfoFactory factory, and factors the UI
service logic in BrowserProcessPlatforPart{chromeos}::RegisterInProcessServices
into.

For ChromeOS there is no behavior change (both --mash and --mus continue
with the same process/service model), but this refactor allows platforms
other than ChromeOS (e.g. Ozone+Mus+Linux) more easily benefit from the
same in-process UI as chromeos/mus.

TEST=None
BUG= 722527 

Change-Id: Ic7a24778b3c57cc0c8620b83bcf555a5a98f5bf8
Reviewed-on: https://chromium-review.googlesource.com/614522
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494980}
[modify] https://crrev.com/477ab563e4dc51a19587a3372c3d487c66f6d906/chrome/browser/BUILD.gn
[modify] https://crrev.com/477ab563e4dc51a19587a3372c3d487c66f6d906/chrome/browser/browser_process_platform_part_chromeos.cc
[add] https://crrev.com/477ab563e4dc51a19587a3372c3d487c66f6d906/chrome/browser/embedded_ui_service_info_factory.cc
[add] https://crrev.com/477ab563e4dc51a19587a3372c3d487c66f6d906/chrome/browser/embedded_ui_service_info_factory.h

Owner: sky@chromium.org
Components: Internals>MUS
Labels: -Pri-3 -mustash-2 Proj-Mustash-Mash OS-Chrome Pri-1
Owner: jamescook@chromium.org
Status: Started (was: Assigned)
Summary: Move ash and mus mojo services into the same process (was: Move ash and Mus into the same process)
I'll take a look. Note to readers: Combining ash and mus is for --mash only.
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 14 2017

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

commit b9df819cfcdcbf89beb0b20e79686285b9603880
Author: James Cook <jamescook@chromium.org>
Date: Thu Dec 14 05:35:54 2017

Clean up mash mojo service registration

Ensure that code associated with the UI Service, which is used on both
chromeos and linux-ozone, uses files and functions named *mus*.

Reserve *mash* for chromeos-only code that runs under --mash and uses
the ash window manager.

Bug:  722527 
Test: chrome, chrome --mus, and chrome --mash run with target_os="chromeos" and linux-ozone still compiles (I can't run linux-ozone due to xkb issues)
Change-Id: I452e80dffb16d6445f40757d1198ff98233bb528
Reviewed-on: https://chromium-review.googlesource.com/822455
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524010}
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/browser/BUILD.gn
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/browser/mash_service_registry.cc
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/utility/BUILD.gn
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/utility/mash_service_factory.cc
[modify] https://crrev.com/b9df819cfcdcbf89beb0b20e79686285b9603880/chrome/utility/mash_service_factory.h

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 14 2017

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

commit fdb0e616bb3978f06ff1dab117db596c0863dbc1
Author: James Cook <jamescook@chromium.org>
Date: Thu Dec 14 18:36:31 2017

Only allow switch --mash on Chrome OS builds

The switch is used to enable out-of-process chrome os system ui (ash).
It used to be used by the Linux Ozone build, but isn't anymore.
Limit it to OS_CHROMEOS builds to make it clear where it is actually
used.

Bug:  722527 
Test: bots, chrome --mash, browser_tests --mash
Change-Id: If980d1f843950f93f89404b658108ae0091639d9
Reviewed-on: https://chromium-review.googlesource.com/823259
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524122}
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/app/chrome_main.cc
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/browser/chromeos/ash_config.cc
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/browser/chromeos/first_run/goodies_displayer_browsertest.cc
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/common/chrome_switches.cc
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/common/chrome_switches.h
[modify] https://crrev.com/fdb0e616bb3978f06ff1dab117db596c0863dbc1/chrome/test/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 3 2018

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

commit 6929b878d7ab8d05f4c0e4db3db769b637c2a469
Author: James Cook <jamescook@chromium.org>
Date: Wed Jan 03 21:36:34 2018

Implement process grouping for content's service manager

Adds the ability to specify a process group for embedded
out-of-process services registered within content.

Services which share a common process group name will be
spawned in the same utility process.

Use this to run ash and the ui service in the same process.

TODO: Fix cursor loading under --mash (it requires some
changes to ui::Service that are not relevant to this CL).

Bug:  722527 
Test: Added to chrome unit_tests and browser_tests

Change-Id: I01d4e2ca2849b8ce44266fee49be3cef340880b3
Reviewed-on: https://chromium-review.googlesource.com/827542
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526828}
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/browser/mash_service_registry.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/browser/mash_service_registry.h
[add] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/browser/mash_service_registry_browsertest.cc
[add] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/browser/mash_service_registry_unittest.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/test/BUILD.gn
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/chrome/utility/mash_service_factory.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/browser/service_manager/service_manager_context.h
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/child/service_factory.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/public/browser/content_browser_client.h
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/content/public/test/browser_test_utils.h
[modify] https://crrev.com/6929b878d7ab8d05f4c0e4db3db769b637c2a469/services/service_manager/embedder/embedded_service_info.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 4 2018

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

commit 2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5
Author: James Cook <jamescook@chromium.org>
Date: Thu Jan 04 16:17:14 2018

cros: Fix cursor for chrome --mash

Grouping the ash service and ui service into the same process broke
image cursors. Fix it by providing the necessary task runner for cursor
resource loading. Make MashServiceRegistry into a class so it can hold
the cursor set.

Also refactor ui::Service InitParams to make it more clear when it is
running in-process vs. out-of-process.

Bug:  722527 
Test: Run chrome --mash, hovering NTP tiles has hand cursor
Change-Id: Ie2b3fc9d523af9f06e16a1b6137f8481182169de
Reviewed-on: https://chromium-review.googlesource.com/832996
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526998}
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/chrome/utility/DEPS
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/chrome/utility/chrome_content_utility_client.h
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/chrome/utility/mash_service_factory.cc
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/chrome/utility/mash_service_factory.h
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/services/ui/main.cc
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/services/ui/service.cc
[modify] https://crrev.com/2a0ba12bf2d42a86e00ee699d2d67e69bd20e5a5/services/ui/service.h

Status: Fixed (was: Started)
Done. I did some manual testing and the discardable memory initialization seems to work OK.

Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment