New issue
Advanced search Search tips

Issue 750390 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Make chrome process run ServiceManager for mushrome

Project Member Reported by sky@chromium.org, Jul 28 2017

Issue description

Currently mushrome starts up a separate process for the ServiceManager. This makes debugging awkward, and testing uses a different path than is actually run. Instead we should run the ServiceManager in the chrome process to simplify our life.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1 2017

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

commit 27e883682d662342f985e868b2a65f2840f8b6cd
Author: Scott Violet <sky@chromium.org>
Date: Tue Aug 01 05:22:53 2017

chromeos: convert checks for ServiceManagerIsRemote() to aura::Env

These checks were really a roundabout way for testing if we're in
mus. Change code to directly test for mus, which means it'll work
regardless of what process the ServiceManager ends up running in.

BUG= 750390 
TEST=covered by tests

Change-Id: I9034386addc926108008e4f5e6ec6416fc7f5ddc
Reviewed-on: https://chromium-review.googlesource.com/592519
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490859}
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/fullscreen_ozone.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/chrome_browser_main_extra_parts_views.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/frame/browser_non_client_frame_view_factory_views.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/frame/native_browser_frame_factory_aurawin.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/frame/native_browser_frame_factory_aurax11.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/frame/native_browser_frame_factory_ozone.cc
[modify] https://crrev.com/27e883682d662342f985e868b2a65f2840f8b6cd/chrome/browser/ui/views/status_bubble_views.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2 2017

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

commit 8af2ef71a597aec21f76a17e345f306c78d81b79
Author: Scott Violet <sky@chromium.org>
Date: Wed Aug 02 21:59:26 2017

chromeos: make creation of DiscardableSharedMemoryManager conditional

Currently creation of DiscardableSharedMemoryManager is controlled by whether
the ServiceManager is remote. Rather than have content make this decision this
patch adds a parameter that lets the consumer of content control if
DiscardableSharedMemoryManager should be created.

This control is important for a config of ChromeOS where we want
content to create DiscardableSharedMemoryManager. This is needed to
avoid shutdown races as the implementation details of
DiscardableSharedMemoryManager assume DiscardableSharedMemoryManager
is shutdown shortly before process exit.

I'm doing this as a precursor to having ServiceManager run in process
in a config of ChromeOS (ash and chrome in same process), but this is
the source of some existing flakiness.

BUG= 750390 
TEST=covered by tests

Change-Id: Ia16e27b2cf85e7129819602e5acb889a21ac0a46
Reviewed-on: https://chromium-review.googlesource.com/595209
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491518}
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/chrome/app/chrome_main.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/chrome/browser/browser_process_platform_part_chromeos.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/chrome/browser/ui/ash/ash_init.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/chrome/test/base/mash_browser_tests_main.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/app/content_main_runner.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/browser/BUILD.gn
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/browser/browser_main_loop.cc
[add] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/browser/discardable_shared_memory_manager.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/public/app/content_main.h
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/public/browser/BUILD.gn
[add] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/public/browser/discardable_shared_memory_manager.h
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/content/public/common/main_function_params.h
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/services/ui/service.cc
[modify] https://crrev.com/8af2ef71a597aec21f76a17e345f306c78d81b79/services/ui/service.h

Project Member

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

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

commit 3ee76e9998863e4cf2396c299cf2aef27baba2bf
Author: Scott Violet <sky@chromium.org>
Date: Fri Aug 04 23:55:12 2017

chromeos: makes --mus run service manager in process

Previously --mus ran the service manager out of process. This leads to
more processes with little gain (makes debugging harder for one). This
patch makes --mus run in the service manager in process, which means
the process you start is the one that actually runs chromes, ash and
the ui service.

Because we don't spawn any additional processes --run-in-mus isn't
needed for the tests, instead you can use --mus (as you would
expect).

BUG= 750390 
TEST=covered by tests

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Iba3ce2c4f586b1c3c869d4976bbf3c1364db551d
Reviewed-on: https://chromium-review.googlesource.com/601257
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492178}
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/chrome/app/chrome_main.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/chrome/browser/chromeos/ash_config.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/chrome/test/base/mash_browser_tests_main.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/browser/browser_main_loop.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/browser/gpu/browser_gpu_channel_host_factory.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/content/public/test/test_launcher.cc
[modify] https://crrev.com/3ee76e9998863e4cf2396c299cf2aef27baba2bf/testing/buildbot/gn_isolate_map.pyl

Comment 4 by sky@chromium.org, Aug 7 2017

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2017

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

commit bcf9e15c04280dfda1d8b08503cad5105fc4f9bc
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Fri Aug 25 02:24:42 2017

Converts usage of ServiceManagerIsRemote to aura::Env::Mode

CL changes window_finder_mus.cc to use aura::Env::Mode::Mus instead
of service_manager::ServiceManagerIsRemote, like done in [1].

In practice, it does not change the behavior that currently works for
chromeos/mash (which also sets MUS as its "aura env mode"). However,
it allows downstream configurations (including linux/chrome/mus
(non-chromeos) [2]) to benefit from the Ozone/Mus WindowFinder.

[1] https://chromium-review.googlesource.com/c/592519
[2] https://github.com/Igalia/chromium

TEST=covered by existing tests + tab dragging in chromeos/mash.

Bug:  750390 
Change-Id: Id6667a26a0ef3c08251aa908cf7b03c6e23cd987
Reviewed-on: https://chromium-review.googlesource.com/633218
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497304}
[modify] https://crrev.com/bcf9e15c04280dfda1d8b08503cad5105fc4f9bc/chrome/browser/ui/views/tabs/window_finder_mus.cc

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

Status: Archived (was: Fixed)

Sign in to add a comment