Skip to content

Commit

Permalink
Reland "Move Audio Service off Service Manager"
Browse files Browse the repository at this point in the history
This is a reland of a4d27f5

The reland moves the TestingApi binder support into a component
library to avoid duplicate definitions that resulted in browser
test failures in component builds.

Original change's description:
> Move Audio Service off Service Manager
>
> All the details around if/when a service instance is started and whether
> it's in- or out-of-process have been centralized into the implementation
> of a single GetAudioService() helper in Content.
>
> A few other helpers are added to ease the transition off the Service
> Manager, though they aren't necessarily ideal APIs. For example,
> client library APIs which once took a Service Manager Connector may now
> take a RepeatingCallback to bind a specific type of interface.
>
> In addition to GetAudioService(), Content provides helpers for
> AudioSystem construction and StreamFactory binding from any sequence.
>
> Because all timeout/lifetime management logic is now generically
> supported within Mojo and Content and requires no service implementation
> details to get right (and because unit tests launching service processes
> is no longer supported outside of Content), all Audio Service test
> coverage relevant to lifetime management is deemed superfluous and has
> been removed.
>
> Bug: 977637
> Change-Id: I4c59948eafce2322547bf2b0479961e095b3edee
> Tbr: oshima@chromium.org
> Tbr: guidou@chromium.org
> Tbr: vollick@chromium.org
> Tbr: xiaohuic@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1941280
> Commit-Queue: Ken Rockot <rockot@google.com>
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#721832}

Bug: 977637
Change-Id: I183e79042c99458f2abf9423d8f9bbefedf5b374
No-Presubmit: true
Tbr: oshima@chromium.org
Tbr: guidou@chromium.org
Tbr: vollick@chromium.org
Tbr: xiaohuic@chromium.org
Tbr: avi@chromium.org
Tbr: rsesek@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1952601
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#722373}
  • Loading branch information
krockot authored and Commit Bot committed Dec 6, 2019
1 parent 196a805 commit e8d24f7
Show file tree
Hide file tree
Showing 112 changed files with 912 additions and 2,091 deletions.
12 changes: 0 additions & 12 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@
#elif defined(OS_MACOSX)
#include "chrome/browser/apps/intent_helper/mac_apps_navigation_throttle.h"
#include "chrome/browser/chrome_browser_main_mac.h"
#include "services/audio/public/mojom/constants.mojom.h"
#elif defined(OS_CHROMEOS)
#include "ash/public/cpp/tablet_mode.h"
#include "ash/public/mojom/constants.mojom.h"
Expand Down Expand Up @@ -2261,17 +2260,6 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches(
#endif
}

void ChromeContentBrowserClient::AdjustUtilityServiceProcessCommandLine(
const service_manager::Identity& identity,
base::CommandLine* command_line) {
#if defined(OS_MACOSX)
// On Mac, the audio service requires a CFRunLoop, provided by a UI message
// loop, to run AVFoundation and CoreAudio code. See https://crbug.com/834581.
if (identity.name() == audio::mojom::kServiceName)
command_line->AppendSwitch(switches::kMessageLoopTypeUi);
#endif
}

std::string
ChromeContentBrowserClient::GetApplicationClientGUIDForQuarantineCheck() {
return std::string(chrome::kApplicationClientIDStringForAVScanning);
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
const base::FilePath& profile_path) override;
void AppendExtraCommandLineSwitches(base::CommandLine* command_line,
int child_process_id) override;
void AdjustUtilityServiceProcessCommandLine(
const service_manager::Identity& identity,
base::CommandLine* command_line) override;
std::string GetApplicationClientGUIDForQuarantineCheck() override;
std::string GetApplicationLocale() override;
std::string GetAcceptLangs(content::BrowserContext* context) override;
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/chromeos/chrome_browser_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
#include "components/user_manager/user.h"
#include "components/user_manager/user_manager.h"
#include "components/user_manager/user_names.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/media_capture_devices.h"
Expand Down Expand Up @@ -650,7 +651,7 @@ void ChromeBrowserMainPartsChromeos::PreProfileInit() {
new default_app_order::ExternalLoader(true /* async */));
}

audio::SoundsManager::Create(content::GetSystemConnector()->Clone());
audio::SoundsManager::Create(content::GetAudioServiceStreamFactoryBinder());

// |arc_service_launcher_| must be initialized before NoteTakingHelper.
NoteTakingHelper::Initialize();
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/chromeos/login/kiosk_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@
#include "components/crx_file/crx_verifier.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/system_connector.h"
#include "content/public/browser/web_ui.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
Expand All @@ -98,7 +98,6 @@
#include "services/audio/public/cpp/fake_system_info.h"
#include "services/audio/public/cpp/sounds/audio_stream_handler.h"
#include "services/audio/public/cpp/sounds/sounds_manager.h"
#include "services/service_manager/public/cpp/connector.h"
#include "ui/aura/window.h"
#include "ui/base/accelerators/accelerator.h"

Expand Down Expand Up @@ -2373,7 +2372,7 @@ class KioskVirtualKeyboardTestSoundsManagerTestImpl
}

auto handler = std::make_unique<audio::AudioStreamHandler>(
content::GetSystemConnector()->Clone(), iter->second);
content::GetAudioServiceStreamFactoryBinder(), iter->second);
if (!handler->IsInitialized()) {
LOG(WARNING) << "Can't initialize AudioStreamHandler for key = " << key;
return false;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/chromeos/login/lock/screen_locker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include "components/account_id/account_id.h"
#include "components/session_manager/core/session_manager.h"
#include "components/user_manager/scoped_user_manager.h"
#include "content/public/browser/system_connector.h"
#include "content/public/browser/audio_service.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_service_manager_context.h"
#include "device/bluetooth/dbus/bluez_dbus_manager.h"
Expand Down Expand Up @@ -78,7 +78,7 @@ class ScreenLockerUnitTest : public testing::Test {
observer_ = std::make_unique<audio::TestObserver>((base::DoNothing()));
audio::AudioStreamHandler::SetObserverForTesting(observer_.get());

audio::SoundsManager::Create(content::GetSystemConnector()->Clone());
audio::SoundsManager::Create(content::GetAudioServiceStreamFactoryBinder());
input_method::InputMethodManager::Initialize(
// Owned by InputMethodManager
new input_method::MockInputMethodManagerImpl());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@
#include "chrome/common/url_constants.h"
#include "chromeos/constants/chromeos_features.h"
#include "components/user_manager/user_manager.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/system_connector.h"
#include "extensions/browser/event_router.h"
#include "extensions/common/api/virtual_keyboard.h"
#include "extensions/common/api/virtual_keyboard_private.h"
#include "media/audio/audio_system.h"
#include "services/audio/public/cpp/audio_system_factory.h"
#include "services/service_manager/public/cpp/connector.h"
#include "ui/aura/event_injector.h"
#include "ui/aura/window_tree_host.h"
#include "ui/base/ime/constants.h"
Expand Down Expand Up @@ -172,10 +170,8 @@ ChromeVirtualKeyboardDelegate::~ChromeVirtualKeyboardDelegate() {}
void ChromeVirtualKeyboardDelegate::GetKeyboardConfig(
OnKeyboardSettingsCallback on_settings_callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!audio_system_) {
audio_system_ =
audio::CreateAudioSystem(content::GetSystemConnector()->Clone());
}
if (!audio_system_)
audio_system_ = content::CreateAudioSystemForAudioService();
audio_system_->HasInputDevices(
base::BindOnce(&ChromeVirtualKeyboardDelegate::OnHasInputDevices,
weak_this_, std::move(on_settings_callback)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,16 @@
#include "chrome/browser/extensions/api/tabs/tabs_constants.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/media_device_id.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/system_connector.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/permissions/permissions_data.h"
#include "media/audio/audio_system.h"
#include "services/audio/public/cpp/audio_system_factory.h"
#include "services/service_manager/public/cpp/connector.h"
#include "url/gurl.h"
#include "url/origin.h"

Expand Down Expand Up @@ -134,10 +132,8 @@ std::string WebrtcAudioPrivateFunction::device_id_salt() const {

media::AudioSystem* WebrtcAudioPrivateFunction::GetAudioSystem() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!audio_system_) {
audio_system_ =
audio::CreateAudioSystem(content::GetSystemConnector()->Clone());
}
if (!audio_system_)
audio_system_ = content::CreateAudioSystemForAudioService();
return audio_system_.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
#include "chrome/common/buildflags.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/media_device_id.h"
#include "content/public/browser/system_connector.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "extensions/common/permissions/permission_set.h"
Expand All @@ -44,8 +44,6 @@
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "services/audio/public/cpp/audio_system_factory.h"
#include "services/service_manager/public/cpp/connector.h"
#include "testing/gtest/include/gtest/gtest.h"

#if defined(OS_WIN)
Expand All @@ -72,7 +70,7 @@ void GetAudioDeviceDescriptions(bool for_input,
AudioDeviceDescriptions* device_descriptions) {
base::RunLoop run_loop;
std::unique_ptr<media::AudioSystem> audio_system =
audio::CreateAudioSystem(content::GetSystemConnector()->Clone());
content::CreateAudioSystemForAudioService();
audio_system->GetDeviceDescriptions(
for_input,
base::BindOnce(
Expand Down
8 changes: 5 additions & 3 deletions chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@
#include "base/task/post_task.h"
#include "base/time/time.h"
#include "components/webrtc_logging/browser/text_log_list.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/system_connector.h"
#include "media/audio/audio_debug_recording_session.h"
#include "services/audio/public/cpp/debug_recording_session_factory.h"
#include "services/service_manager/public/cpp/connector.h"

using content::BrowserThread;

Expand Down Expand Up @@ -112,8 +111,11 @@ void AudioDebugRecordingsHandler::DoStartAudioDebugRecordings(
log_directory, ++current_audio_debug_recordings_id_);
host->EnableAudioDebugRecordings(prefix_path);

mojo::PendingRemote<audio::mojom::DebugRecording> debug_recording;
content::GetAudioService().BindDebugRecording(
debug_recording.InitWithNewPipeAndPassReceiver());
audio_debug_recording_session_ = audio::CreateAudioDebugRecordingSession(
prefix_path, content::GetSystemConnector()->Clone());
prefix_path, std::move(debug_recording));

if (delay.is_zero()) {
const bool is_stopped = false, is_manual_stop = false;
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/ui/ash/assistant/assistant_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/services/assistant/public/features.h"
#include "components/session_manager/core/session_manager.h"
#include "content/public/browser/audio_service.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/browser/service_process_host.h"
#include "content/public/browser/system_connector.h"
#include "content/public/common/content_switches.h"
#include "services/audio/public/mojom/constants.mojom.h"
#include "services/device/public/mojom/constants.mojom.h"
#include "services/identity/public/mojom/identity_service.mojom.h"
#include "services/media_session/public/mojom/constants.mojom.h"
Expand Down Expand Up @@ -183,8 +183,7 @@ void AssistantClient::RequestWakeLockProvider(

void AssistantClient::RequestAudioStreamFactory(
mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) {
content::GetSystemConnector()->Connect(audio::mojom::kServiceName,
std::move(receiver));
content::GetAudioService().BindStreamFactory(std::move(receiver));
}

void AssistantClient::RequestAudioDecoderFactory(
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/vr/sounds_manager_audio_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

#include "chrome/browser/vr/sounds_manager_audio_delegate.h"
#include "content/public/browser/system_connector.h"
#include "content/public/browser/audio_service.h"
#include "services/audio/public/cpp/sounds/sounds_manager.h"

namespace vr {
Expand All @@ -30,7 +30,7 @@ bool SoundsManagerAudioDelegate::RegisterSound(
DCHECK(sounds_.find(id) == sounds_.end());

if (sounds_.empty())
audio::SoundsManager::Create(content::GetSystemConnector()->Clone());
audio::SoundsManager::Create(content::GetAudioServiceStreamFactoryBinder());

sounds_[id] = std::move(data);
return audio::SoundsManager::Get()->Initialize(id, *sounds_[id]);
Expand Down
1 change: 0 additions & 1 deletion chromeos/services/assistant/assistant_state_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "ash/public/mojom/assistant_state_controller.mojom.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/service_manager/public/cpp/connector.h"

namespace chromeos {
namespace assistant {
Expand Down
4 changes: 2 additions & 2 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,8 @@ jumbo_source_set("browser") {
"//net/server:http_server",
"//ppapi/buildflags",
"//printing/buildflags",
"//services/audio:lib",
"//services/audio",
"//services/audio/public/cpp",
"//services/audio/public/cpp:manifest",
"//services/audio/public/mojom",
"//services/content:impl",
"//services/content/public/cpp",
Expand Down Expand Up @@ -433,6 +432,7 @@ jumbo_source_set("browser") {
"appcache/appcache_working_set.h",
"appcache/chrome_appcache_service.cc",
"appcache/chrome_appcache_service.h",
"audio/audio_service.cc",
"background_fetch/background_fetch_context.cc",
"background_fetch/background_fetch_context.h",
"background_fetch/background_fetch_cross_origin_filter.cc",
Expand Down
2 changes: 2 additions & 0 deletions content/browser/audio/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
file://services/audio/OWNERS

Loading

0 comments on commit e8d24f7

Please sign in to comment.