diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 5cdfec3a834b69..bc92516a1fea46 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -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" @@ -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); diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h index d6b48bfa43147c..db350881f241f6 100644 --- a/chrome/browser/chrome_content_browser_client.h +++ b/chrome/browser/chrome_content_browser_client.h @@ -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; diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc index f7070fb175ac22..bc15c8f200b511 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc @@ -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" @@ -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(); diff --git a/chrome/browser/chromeos/login/kiosk_browsertest.cc b/chrome/browser/chromeos/login/kiosk_browsertest.cc index 7eb077835ea769..78b91589d76442 100644 --- a/chrome/browser/chromeos/login/kiosk_browsertest.cc +++ b/chrome/browser/chromeos/login/kiosk_browsertest.cc @@ -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" @@ -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" @@ -2373,7 +2372,7 @@ class KioskVirtualKeyboardTestSoundsManagerTestImpl } auto handler = std::make_unique( - content::GetSystemConnector()->Clone(), iter->second); + content::GetAudioServiceStreamFactoryBinder(), iter->second); if (!handler->IsInitialized()) { LOG(WARNING) << "Can't initialize AudioStreamHandler for key = " << key; return false; diff --git a/chrome/browser/chromeos/login/lock/screen_locker_unittest.cc b/chrome/browser/chromeos/login/lock/screen_locker_unittest.cc index f942b4d18e8691..d02c6f6e87eb56 100644 --- a/chrome/browser/chromeos/login/lock/screen_locker_unittest.cc +++ b/chrome/browser/chromeos/login/lock/screen_locker_unittest.cc @@ -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" @@ -78,7 +78,7 @@ class ScreenLockerUnitTest : public testing::Test { observer_ = std::make_unique((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()); diff --git a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc index 060590f6dcc3bf..8606e917ff72fc 100644 --- a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc +++ b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc @@ -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" @@ -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))); diff --git a/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc b/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc index 9d920b10a5c4ba..9e331b4fdb905e 100644 --- a/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc +++ b/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc @@ -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" @@ -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(); } diff --git a/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc b/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc index 500733126428b2..2a399b87e64425 100644 --- a/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc +++ b/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc @@ -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" @@ -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) @@ -72,7 +70,7 @@ void GetAudioDeviceDescriptions(bool for_input, AudioDeviceDescriptions* device_descriptions) { base::RunLoop run_loop; std::unique_ptr audio_system = - audio::CreateAudioSystem(content::GetSystemConnector()->Clone()); + content::CreateAudioSystemForAudioService(); audio_system->GetDeviceDescriptions( for_input, base::BindOnce( diff --git a/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc b/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc index 44ee73b6ae7403..087351469e4d2a 100644 --- a/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc +++ b/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc @@ -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; @@ -112,8 +111,11 @@ void AudioDebugRecordingsHandler::DoStartAudioDebugRecordings( log_directory, ++current_audio_debug_recordings_id_); host->EnableAudioDebugRecordings(prefix_path); + mojo::PendingRemote 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; diff --git a/chrome/browser/ui/ash/assistant/assistant_client.cc b/chrome/browser/ui/ash/assistant/assistant_client.cc index 57018b6c9ef1fa..f544b6ca4624c6 100644 --- a/chrome/browser/ui/ash/assistant/assistant_client.cc +++ b/chrome/browser/ui/ash/assistant/assistant_client.cc @@ -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" @@ -183,8 +183,7 @@ void AssistantClient::RequestWakeLockProvider( void AssistantClient::RequestAudioStreamFactory( mojo::PendingReceiver receiver) { - content::GetSystemConnector()->Connect(audio::mojom::kServiceName, - std::move(receiver)); + content::GetAudioService().BindStreamFactory(std::move(receiver)); } void AssistantClient::RequestAudioDecoderFactory( diff --git a/chrome/browser/vr/sounds_manager_audio_delegate.cc b/chrome/browser/vr/sounds_manager_audio_delegate.cc index 80781150162b2a..758b1de63074a1 100644 --- a/chrome/browser/vr/sounds_manager_audio_delegate.cc +++ b/chrome/browser/vr/sounds_manager_audio_delegate.cc @@ -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 { @@ -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]); diff --git a/chromeos/services/assistant/assistant_state_proxy.cc b/chromeos/services/assistant/assistant_state_proxy.cc index a9bed5f362942d..bee65920076447 100644 --- a/chromeos/services/assistant/assistant_state_proxy.cc +++ b/chromeos/services/assistant/assistant_state_proxy.cc @@ -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 { diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index 2cb2b5bbdc8824..fc7f9d2c73ceb2 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn @@ -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", @@ -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", diff --git a/content/browser/audio/OWNERS b/content/browser/audio/OWNERS new file mode 100644 index 00000000000000..8c158a5248de79 --- /dev/null +++ b/content/browser/audio/OWNERS @@ -0,0 +1,2 @@ +file://services/audio/OWNERS + diff --git a/content/browser/audio/audio_service.cc b/content/browser/audio/audio_service.cc new file mode 100644 index 00000000000000..940e420e7c70b2 --- /dev/null +++ b/content/browser/audio/audio_service.cc @@ -0,0 +1,176 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/public/browser/audio_service.h" + +#include "base/command_line.h" +#include "base/deferred_sequenced_task_runner.h" +#include "base/metrics/field_trial_params.h" +#include "base/no_destructor.h" +#include "base/optional.h" +#include "base/strings/string_number_conversions.h" +#include "base/task/post_task.h" +#include "base/threading/sequence_local_storage_slot.h" +#include "base/time/time.h" +#include "build/build_config.h" +#include "content/browser/browser_main_loop.h" +#include "content/public/browser/browser_task_traits.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/content_browser_client.h" +#include "content/public/browser/service_process_host.h" +#include "content/public/common/content_client.h" +#include "content/public/common/content_features.h" +#include "content/public/common/content_switches.h" +#include "media/audio/audio_manager.h" +#include "media/base/media_switches.h" +#include "mojo/public/cpp/bindings/remote.h" +#include "services/audio/public/cpp/audio_system_to_service_adapter.h" +#include "services/audio/service.h" +#include "services/audio/service_factory.h" + +namespace content { + +namespace { + +base::Optional GetFieldTrialIdleTimeout() { + std::string timeout_str = + base::GetFieldTrialParamValue("AudioService", "teardown_timeout_s"); + int timeout_s = 0; + if (!base::StringToInt(timeout_str, &timeout_s)) + return base::nullopt; + return base::TimeDelta::FromSeconds(timeout_s); +} + +base::Optional GetCommandLineIdleTimeout() { + const base::CommandLine& command_line = + *base::CommandLine::ForCurrentProcess(); + std::string timeout_str = + command_line.GetSwitchValueASCII(switches::kAudioServiceQuitTimeoutMs); + int timeout_ms = 0; + if (!base::StringToInt(timeout_str, &timeout_ms)) + return base::nullopt; + return base::TimeDelta::FromMilliseconds(timeout_ms); +} + +base::Optional GetAudioServiceProcessIdleTimeout() { + base::Optional timeout = GetCommandLineIdleTimeout(); + if (!timeout) + timeout = GetFieldTrialIdleTimeout(); + if (timeout && *timeout < base::TimeDelta()) + return base::nullopt; + return timeout; +} + +bool IsAudioServiceOutOfProcess() { + return base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess) && + !GetContentClient()->browser()->OverridesAudioManager(); +} + +void BindSystemInfoFromAnySequence( + mojo::PendingReceiver receiver) { + if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { + base::PostTask( + FROM_HERE, {BrowserThread::UI}, + base::BindOnce(&BindSystemInfoFromAnySequence, std::move(receiver))); + return; + } + + GetAudioService().BindSystemInfo(std::move(receiver)); +} + +void BindStreamFactoryFromAnySequence( + mojo::PendingReceiver receiver) { + if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { + base::PostTask( + FROM_HERE, {BrowserThread::UI}, + base::BindOnce(&BindStreamFactoryFromAnySequence, std::move(receiver))); + return; + } + + GetAudioService().BindStreamFactory(std::move(receiver)); +} + +void LaunchAudioServiceInProcess( + mojo::PendingReceiver receiver) { + // NOTE: If BrowserMainLoop is uninitialized, we have no AudioManager. In + // this case we discard the receiver. The remote will always discard + // messages. This is to work around unit testing environments where no + // BrowserMainLoop is initialized. + if (!BrowserMainLoop::GetInstance()) + return; + + // TODO(https://crbug.com/853254): Remove + // BrowserMainLoop::GetAudioManager(). + audio::Service::GetInProcessTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce( + [](media::AudioManager* audio_manager, + mojo::PendingReceiver receiver) { + static base::NoDestructor< + base::SequenceLocalStorageSlot>> + service; + service->GetOrCreateValue() = audio::CreateEmbeddedService( + audio_manager, std::move(receiver)); + }, + BrowserMainLoop::GetAudioManager(), std::move(receiver))); +} + +void LaunchAudioServiceOutOfProcess( + mojo::PendingReceiver receiver) { + ServiceProcessHost::Launch( + std::move(receiver), + ServiceProcessHost::Options() + .WithDisplayName("Audio Service") + .WithSandboxType(service_manager::SANDBOX_TYPE_AUDIO) +#if defined(OS_MACOSX) + // On Mac, the audio service requires a CFRunLoop provided by a + // UI MessageLoop type, to run AVFoundation and CoreAudio code. + // See https://crbug.com/834581. + .WithExtraCommandLineSwitches({switches::kMessageLoopTypeUi}) +#endif + .Pass()); +} + +void LaunchAudioService(mojo::Remote* remote) { + auto receiver = remote->BindNewPipeAndPassReceiver(); + if (IsAudioServiceOutOfProcess()) { + LaunchAudioServiceOutOfProcess(std::move(receiver)); + if (auto idle_timeout = GetAudioServiceProcessIdleTimeout()) + remote->reset_on_idle_timeout(*idle_timeout); + } else { + LaunchAudioServiceInProcess(std::move(receiver)); + } + remote->reset_on_disconnect(); +} + +} // namespace + +audio::mojom::AudioService& GetAudioService() { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + // NOTE: We use sequence-local storage slot not because we support access from + // any sequence, but to limit the lifetime of this Remote to the lifetime of + // UI-thread sequence. This is to support re-creation after task environment + // shutdown and reinitialization e.g. between unit tests. + static base::NoDestructor< + base::SequenceLocalStorageSlot>> + remote_slot; + auto& remote = remote_slot->GetOrCreateValue(); + if (!remote) + LaunchAudioService(&remote); + return *remote.get(); +} + +std::unique_ptr CreateAudioSystemForAudioService() { + constexpr auto kServiceDisconnectTimeout = base::TimeDelta::FromSeconds(1); + return std::make_unique( + base::BindRepeating(&BindSystemInfoFromAnySequence), + kServiceDisconnectTimeout); +} + +AudioServiceStreamFactoryBinder GetAudioServiceStreamFactoryBinder() { + return base::BindRepeating(&BindStreamFactoryFromAnySequence); +} + +} // namespace content diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc index 9da6475030a796..a23f65b9aee04c 100644 --- a/content/browser/browser_main_loop.cc +++ b/content/browser/browser_main_loop.cc @@ -99,6 +99,7 @@ #include "content/browser/webui/url_data_manager.h" #include "content/common/content_switches_internal.h" #include "content/common/thread_pool_util.h" +#include "content/public/browser/audio_service.h" #include "content/public/browser/browser_main_parts.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" @@ -133,8 +134,6 @@ #include "net/socket/client_socket_factory.h" #include "net/ssl/ssl_config_service.h" #include "ppapi/buildflags/buildflags.h" -#include "services/audio/public/cpp/audio_system_factory.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/service.h" #include "services/content/public/cpp/navigable_contents_view.h" #include "services/data_decoder/public/cpp/service_provider.h" @@ -1599,17 +1598,11 @@ void BrowserMainLoop::InitializeAudio() { {content::BrowserThread::UI, base::TaskPriority::BEST_EFFORT}, base::BindOnce([]() { TRACE_EVENT0("audio", "Starting audio service"); - auto* connector = GetSystemConnector(); - if (connector) { - // The browser is not shutting down: |connection| would be null - // otherwise. - connector->WarmService(service_manager::ServiceFilter::ByName( - audio::mojom::kServiceName)); - } + GetAudioService(); })); } - audio_system_ = audio::CreateAudioSystem(GetSystemConnector()->Clone()); + audio_system_ = CreateAudioSystemForAudioService(); CHECK(audio_system_); } diff --git a/content/browser/builtin_service_manifests.cc b/content/browser/builtin_service_manifests.cc index 26281550f4af71..2319dc5d57c639 100644 --- a/content/browser/builtin_service_manifests.cc +++ b/content/browser/builtin_service_manifests.cc @@ -15,33 +15,17 @@ #include "media/mojo/buildflags.h" #include "media/mojo/services/cdm_manifest.h" #include "media/mojo/services/media_manifest.h" -#include "services/audio/public/cpp/manifest.h" #include "services/device/public/cpp/manifest.h" #include "services/media_session/public/cpp/manifest.h" #include "services/service_manager/public/cpp/manifest_builder.h" namespace content { -namespace { - -bool IsAudioServiceOutOfProcess() { - return base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess) && - !GetContentClient()->browser()->OverridesAudioManager(); -} - -} // namespace - const std::vector& GetBuiltinServiceManifests() { static base::NoDestructor> manifests{ std::vector{ GetContentBrowserManifest(), - audio::GetManifest(IsAudioServiceOutOfProcess() - ? service_manager::Manifest::ExecutionMode:: - kOutOfProcessBuiltin - : service_manager::Manifest::ExecutionMode:: - kInProcessBuiltin), - #if BUILDFLAG(ENABLE_LIBRARY_CDMS) media::GetCdmManifest(), #endif diff --git a/content/browser/media/forwarding_audio_stream_factory.cc b/content/browser/media/forwarding_audio_stream_factory.cc index de0bec2594d713..1ba3ccd16e4df7 100644 --- a/content/browser/media/forwarding_audio_stream_factory.cc +++ b/content/browser/media/forwarding_audio_stream_factory.cc @@ -9,10 +9,12 @@ #include "base/bind.h" #include "base/location.h" #include "base/logging.h" +#include "base/no_destructor.h" #include "base/stl_util.h" #include "base/task/post_task.h" #include "base/trace_event/trace_event.h" #include "content/browser/web_contents/web_contents_impl.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/render_frame_host.h" @@ -20,26 +22,44 @@ #include "content/public/browser/web_contents.h" #include "media/base/audio_parameters.h" #include "media/base/user_input_monitor.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/service_manager/public/cpp/connector.h" #include "ui/gfx/geometry/size.h" namespace content { +namespace { + +ForwardingAudioStreamFactory::StreamFactoryBinder& +GetStreamFactoryBinderOverride() { + static base::NoDestructor + binder; + return *binder; +} + +void BindStreamFactoryFromUIThread( + mojo::PendingReceiver receiver) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + const auto& binder_override = GetStreamFactoryBinderOverride(); + if (binder_override) { + binder_override.Run(std::move(receiver)); + return; + } + + GetAudioService().BindStreamFactory(std::move(receiver)); +} + +} // namespace + ForwardingAudioStreamFactory::Core::Core( base::WeakPtr owner, media::UserInputMonitorBase* user_input_monitor, - std::unique_ptr connector, std::unique_ptr broker_factory) : user_input_monitor_(user_input_monitor), owner_(std::move(owner)), broker_factory_(std::move(broker_factory)), - group_id_(base::UnguessableToken::Create()), - connector_(std::move(connector)) { + group_id_(base::UnguessableToken::Create()) { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(owner_); DCHECK(broker_factory_); - DCHECK(connector_); } ForwardingAudioStreamFactory::Core::~Core() { @@ -206,13 +226,11 @@ ForwardingAudioStreamFactory::Core* ForwardingAudioStreamFactory::CoreForFrame( ForwardingAudioStreamFactory::ForwardingAudioStreamFactory( WebContents* web_contents, media::UserInputMonitorBase* user_input_monitor, - std::unique_ptr connector, std::unique_ptr broker_factory) : WebContentsObserver(web_contents), core_() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - core_ = - std::make_unique(weak_ptr_factory_.GetWeakPtr(), user_input_monitor, - std::move(connector), std::move(broker_factory)); + core_ = std::make_unique(weak_ptr_factory_.GetWeakPtr(), + user_input_monitor, std::move(broker_factory)); } ForwardingAudioStreamFactory::~ForwardingAudioStreamFactory() { @@ -268,6 +286,11 @@ void ForwardingAudioStreamFactory::FrameDeleted( render_frame_host->GetRoutingID())); } +void ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + StreamFactoryBinder binder) { + GetStreamFactoryBinderOverride() = std::move(binder); +} + void ForwardingAudioStreamFactory::Core::CleanupStreamsBelongingTo( int render_process_id, int render_frame_id) { @@ -317,8 +340,10 @@ audio::mojom::StreamFactory* ForwardingAudioStreamFactory::Core::GetFactory() { TRACE_EVENT_INSTANT1( "audio", "ForwardingAudioStreamFactory: Binding new factory", TRACE_EVENT_SCOPE_THREAD, "group", group_id_.GetLowForSerialization()); - connector_->Connect(audio::mojom::kServiceName, - remote_factory_.BindNewPipeAndPassReceiver()); + base::PostTask( + FROM_HERE, {BrowserThread::UI}, + base::BindOnce(&BindStreamFactoryFromUIThread, + remote_factory_.BindNewPipeAndPassReceiver())); // Unretained is safe because |this| owns |remote_factory_|. remote_factory_.set_disconnect_handler(base::BindOnce( &ForwardingAudioStreamFactory::Core::ResetRemoteFactoryPtr, diff --git a/content/browser/media/forwarding_audio_stream_factory.h b/content/browser/media/forwarding_audio_stream_factory.h index 21fe74694e42e1..fe464f56e62e8d 100644 --- a/content/browser/media/forwarding_audio_stream_factory.h +++ b/content/browser/media/forwarding_audio_stream_factory.h @@ -26,10 +26,6 @@ #include "services/audio/public/mojom/audio_processing.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" -namespace service_manager { -class Connector; -} - namespace media { class AudioParameters; class UserInputMonitorBase; @@ -62,17 +58,11 @@ class CONTENT_EXPORT ForwardingAudioStreamFactory final public: Core(base::WeakPtr owner, media::UserInputMonitorBase* user_input_monitor, - std::unique_ptr connector, std::unique_ptr factory); ~Core() final; const base::UnguessableToken& group_id() const { return group_id_; } - // E.g. to override binder. - service_manager::Connector* get_connector_for_testing() { - return connector_.get(); - } - base::WeakPtr AsWeakPtr(); // TODO(https://crbug.com/787806): Automatically restore streams on audio @@ -149,8 +139,6 @@ class CONTENT_EXPORT ForwardingAudioStreamFactory final // |this|. const base::UnguessableToken group_id_; - const std::unique_ptr connector_; - // Lazily acquired. Reset on connection error and when we no longer have any // streams. Note: we don't want muting to force the connection to be open, // since we want to clean up the service when not in use. If we have active @@ -190,11 +178,9 @@ class CONTENT_EXPORT ForwardingAudioStreamFactory final // |web_contents| is null in the browser-privileged access case, i.e., when // the streams created with this factory will not be consumed by a renderer. - // |connector| will be used on the IO thread. ForwardingAudioStreamFactory( WebContents* web_contents, media::UserInputMonitorBase* user_input_monitor, - std::unique_ptr connector, std::unique_ptr factory); ~ForwardingAudioStreamFactory() final; @@ -220,6 +206,12 @@ class CONTENT_EXPORT ForwardingAudioStreamFactory final Core* core() { return core_.get(); } + // Allows tests to override how StreamFactory interface receivers are bound + // instead of sending them to the Audio Service. + using StreamFactoryBinder = base::RepeatingCallback)>; + static void OverrideStreamFactoryBinderForTesting(StreamFactoryBinder binder); + private: std::unique_ptr core_; bool is_muted_ = false; diff --git a/content/browser/media/forwarding_audio_stream_factory_unittest.cc b/content/browser/media/forwarding_audio_stream_factory_unittest.cc index 88a6726c71334f..9b16ba5598fec4 100644 --- a/content/browser/media/forwarding_audio_stream_factory_unittest.cc +++ b/content/browser/media/forwarding_audio_stream_factory_unittest.cc @@ -23,9 +23,7 @@ #include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "services/audio/public/cpp/fake_stream_factory.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/service_manager/public/cpp/connector.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -187,16 +185,16 @@ class MockLoopbackSink : public AudioStreamBroker::LoopbackSink { class ForwardingAudioStreamFactoryTest : public RenderViewHostTestHarness { public: ForwardingAudioStreamFactoryTest() - : connector_(service_manager::Connector::Create(&connector_receiver_)), - broker_factory_(std::make_unique()) { - connector_->OverrideBinderForTesting( - service_manager::ServiceFilter::ByName(audio::mojom::kServiceName), - audio::mojom::StreamFactory::Name_, + : broker_factory_(std::make_unique()) { + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( base::BindRepeating(&ForwardingAudioStreamFactoryTest::BindFactory, base::Unretained(this))); } - ~ForwardingAudioStreamFactoryTest() override {} + ~ForwardingAudioStreamFactoryTest() override { + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + base::NullCallback()); + } void SetUp() override { RenderViewHostTestHarness::SetUp(); @@ -205,10 +203,9 @@ class ForwardingAudioStreamFactoryTest : public RenderViewHostTestHarness { RenderFrameHostTester::For(main_rfh())->AppendChild("other_rfh"); } - void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { - stream_factory_.receiver_.Bind( - mojo::PendingReceiver( - std::move(factory_receiver))); + void BindFactory( + mojo::PendingReceiver receiver) { + stream_factory_.receiver_.Bind(std::move(receiver)); stream_factory_.receiver_.set_disconnect_handler( base::BindRepeating(&audio::FakeStreamFactory::ResetReceiver, base::Unretained(&stream_factory_))); @@ -246,8 +243,6 @@ class ForwardingAudioStreamFactoryTest : public RenderViewHostTestHarness { static const uint32_t kSharedMemoryCount; static const bool kEnableAgc; MockStreamFactory stream_factory_; - mojo::PendingReceiver connector_receiver_; - std::unique_ptr connector_; RenderFrameHost* other_rfh_; std::unique_ptr broker_factory_; }; @@ -268,9 +263,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, CreateInputStream_CreatesInputStream) { mojo::PendingRemote client; base::WeakPtr broker = ExpectInputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -287,16 +282,13 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr broker = ExpectLoopbackBrokerConstruction(main_rfh()); - std::unique_ptr other_connector = - connector_->Clone(); - - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::move(other_connector), std::make_unique()); + std::make_unique()); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -311,9 +303,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, mojo::PendingRemote client; base::WeakPtr broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -329,9 +321,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectInputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -368,16 +360,13 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectLoopbackBrokerConstruction(other_rfh()); - std::unique_ptr other_connector = - connector_->Clone(); - - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::move(other_connector), std::make_unique()); + std::make_unique()); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -413,9 +402,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectOutputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -460,16 +449,13 @@ TEST_F(ForwardingAudioStreamFactoryTest, DestroyFrame_DestroysRelatedStreams) { base::WeakPtr other_rfh_output_broker = ExpectOutputBrokerConstruction(other_rfh()); - std::unique_ptr other_connector = - connector_->Clone(); - - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::move(other_connector), std::make_unique()); + std::make_unique()); { EXPECT_CALL(*main_rfh_input_broker, CreateStream(NotNull())); @@ -563,9 +549,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, DestroyWebContents_DestroysStreams) { base::WeakPtr output_broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_CALL(*input_broker, CreateStream(NotNull())); ignore_result(input_client.InitWithNewPipeAndPassReceiver()); @@ -600,9 +586,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, LastStreamDeleted_ClearsFactoryPtr) { base::WeakPtr other_rfh_output_broker = ExpectOutputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_input_broker, CreateStream(NotNull())); @@ -671,9 +657,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, LastStreamDeleted_ClearsFactoryPtr) { TEST_F(ForwardingAudioStreamFactoryTest, MuteNoOutputStreams_DoesNotConnectMuter) { - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_FALSE(factory.IsMuted()); factory.SetMuted(true); @@ -692,9 +678,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, TEST_F(ForwardingAudioStreamFactoryTest, MuteWithOutputStream_ConnectsMuter) { mojo::PendingRemote client; base::WeakPtr broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -724,9 +710,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, WhenMuting_ConnectedWhenOutputStreamExists) { mojo::PendingRemote client; base::WeakPtr broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); EXPECT_FALSE(stream_factory_.IsConnected()); EXPECT_FALSE(factory.IsMuted()); @@ -761,9 +747,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr broker = ExpectOutputBrokerConstruction(main_rfh()); base::WeakPtr another_broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); { EXPECT_CALL(*broker, CreateStream(NotNull())); @@ -817,9 +803,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, // called. EXPECT_CALL(sink2, OnSourceGone()); { - ForwardingAudioStreamFactory factory( - web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory(web_contents(), + nullptr /*user_input_monitor*/, + std::move(broker_factory_)); factory.core()->AddLoopbackSink(&sink1); factory.core()->AddLoopbackSink(&sink2); diff --git a/content/browser/media/in_process_audio_loopback_stream_creator.cc b/content/browser/media/in_process_audio_loopback_stream_creator.cc index a56fc100061653..a941bb86c5fc44 100644 --- a/content/browser/media/in_process_audio_loopback_stream_creator.cc +++ b/content/browser/media/in_process_audio_loopback_stream_creator.cc @@ -17,12 +17,10 @@ #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/render_frame_host.h" -#include "content/public/browser/system_connector.h" #include "media/audio/audio_device_description.h" #include "media/base/user_input_monitor.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" -#include "services/service_manager/public/cpp/connector.h" namespace content { @@ -98,7 +96,6 @@ InProcessAudioLoopbackStreamCreator::InProcessAudioLoopbackStreamCreator() ? static_cast( BrowserMainLoop::GetInstance()->user_input_monitor()) : nullptr, - content::GetSystemConnector()->Clone(), AudioStreamBrokerFactory::CreateImpl()) { DCHECK_CURRENTLY_ON(BrowserThread::UI); } diff --git a/content/browser/renderer_host/media/audio_service_listener.cc b/content/browser/renderer_host/media/audio_service_listener.cc index 8daab102edda5b..4e814a06ad7a78 100644 --- a/content/browser/renderer_host/media/audio_service_listener.cc +++ b/content/browser/renderer_host/media/audio_service_listener.cc @@ -10,11 +10,12 @@ #include "base/metrics/histogram_macros.h" #include "base/time/default_tick_clock.h" #include "content/browser/media/audio_log_factory.h" +#include "content/public/browser/audio_service.h" #include "content/public/common/content_features.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" -#include "services/audio/public/mojom/constants.mojom.h" +#include "services/audio/public/mojom/audio_service.mojom.h" #include "services/audio/public/mojom/log_factory_manager.mojom.h" namespace content { @@ -24,12 +25,10 @@ AudioServiceListener::Metrics::Metrics(const base::TickClock* clock) AudioServiceListener::Metrics::~Metrics() = default; -void AudioServiceListener::Metrics::ServiceAlreadyRunning( - service_manager::mojom::InstanceState state) { +void AudioServiceListener::Metrics::ServiceAlreadyRunning() { LogServiceStartStatus(ServiceStartStatus::kAlreadyStarted); initial_downtime_start_ = base::TimeTicks(); - if (state == service_manager::mojom::InstanceState::kStarted) - started_ = clock_->NowTicks(); + started_ = clock_->NowTicks(); } void AudioServiceListener::Metrics::ServiceCreated() { @@ -65,11 +64,6 @@ void AudioServiceListener::Metrics::ServiceStarted() { } } -void AudioServiceListener::Metrics::ServiceFailedToStart() { - LogServiceStartStatus(ServiceStartStatus::kFailure); - created_ = base::TimeTicks(); -} - void AudioServiceListener::Metrics::ServiceStopped() { stopped_ = clock_->NowTicks(); @@ -87,26 +81,15 @@ void AudioServiceListener::Metrics::LogServiceStartStatus( UMA_HISTOGRAM_ENUMERATION("Media.AudioService.ObservedStartStatus", status); } -AudioServiceListener::AudioServiceListener( - std::unique_ptr connector) - : connector_(std::move(connector)), - metrics_(base::DefaultTickClock::GetInstance()) { - DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (!connector_) - return; // Happens in unittests. - - mojo::Remote service_manager; - connector_->Connect(service_manager::mojom::kServiceName, - service_manager.BindNewPipeAndPassReceiver()); - mojo::PendingRemote listener; - mojo::PendingReceiver request( - listener.InitWithNewPipeAndPassReceiver()); - service_manager->AddListener(std::move(listener)); - receiver_.Bind(std::move(request)); +AudioServiceListener::AudioServiceListener() + : metrics_(base::DefaultTickClock::GetInstance()) { + ServiceProcessHost::AddObserver(this); + Init(ServiceProcessHost::GetRunningProcessInfo()); } AudioServiceListener::~AudioServiceListener() { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); + ServiceProcessHost::RemoveObserver(this); } base::ProcessId AudioServiceListener::GetProcessId() const { @@ -114,110 +97,55 @@ base::ProcessId AudioServiceListener::GetProcessId() const { return process_id_; } -void AudioServiceListener::OnInit( - std::vector - running_services) { - DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - for (const service_manager::mojom::RunningServiceInfoPtr& instance : - running_services) { - if (instance->identity.name() == audio::mojom::kServiceName && - instance->state != - service_manager::mojom::InstanceState::kUnreachable) { - current_instance_identity_ = instance->identity; - current_instance_state_ = instance->state; - metrics_.ServiceAlreadyRunning(instance->state); +void AudioServiceListener::Init( + std::vector running_service_processes) { + for (const auto& info : running_service_processes) { + if (info.IsService()) { + process_id_ = info.pid; + metrics_.ServiceAlreadyRunning(); MaybeSetLogFactory(); - - // NOTE: This may not actually be a valid PID yet. If not, we will - // receive OnServicePIDReceived soon. - process_id_ = instance->pid; break; } } } -void AudioServiceListener::OnServiceCreated( - service_manager::mojom::RunningServiceInfoPtr service) { +void AudioServiceListener::OnServiceProcessLaunched( + const ServiceProcessInfo& info) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (service->identity.name() != audio::mojom::kServiceName) + if (!info.IsService()) return; - if (current_instance_identity_) { - // If we were already tracking an instance of the service, it must be dying - // soon. We'll start tracking the new instance instead now, so simulate - // stoppage of the old one. - DCHECK(service->identity != current_instance_identity_); - if (current_instance_state_ == - service_manager::mojom::InstanceState::kCreated) { - OnServiceFailedToStart(*current_instance_identity_); - } else { - DCHECK_EQ(service_manager::mojom::InstanceState::kStarted, - *current_instance_state_); - OnServiceStopped(*current_instance_identity_); - } - } - - current_instance_identity_ = service->identity; - current_instance_state_ = service_manager::mojom::InstanceState::kCreated; + process_id_ = info.pid; metrics_.ServiceCreated(); - MaybeSetLogFactory(); -} - -void AudioServiceListener::OnServiceStarted( - const ::service_manager::Identity& identity, - uint32_t pid) { - DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (identity.name() != audio::mojom::kServiceName) - return; - - DCHECK(identity == current_instance_identity_); - DCHECK(current_instance_state_ == - service_manager::mojom::InstanceState::kCreated); - current_instance_state_ = service_manager::mojom::InstanceState::kStarted; metrics_.ServiceStarted(); } -void AudioServiceListener::OnServicePIDReceived( - const ::service_manager::Identity& identity, - uint32_t pid) { - DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (identity != current_instance_identity_) - return; - - process_id_ = pid; -} - -void AudioServiceListener::OnServiceFailedToStart( - const ::service_manager::Identity& identity) { +void AudioServiceListener::OnServiceProcessTerminatedNormally( + const ServiceProcessInfo& info) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (identity != current_instance_identity_) + if (!info.IsService()) return; - metrics_.ServiceFailedToStart(); - current_instance_identity_.reset(); - current_instance_state_.reset(); + metrics_.ServiceStopped(); process_id_ = base::kNullProcessId; log_factory_is_set_ = false; } -void AudioServiceListener::OnServiceStopped( - const ::service_manager::Identity& identity) { +void AudioServiceListener::OnServiceProcessCrashed( + const ServiceProcessInfo& info) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (identity != current_instance_identity_) + if (!info.IsService()) return; metrics_.ServiceStopped(); - current_instance_identity_.reset(); - current_instance_state_.reset(); process_id_ = base::kNullProcessId; log_factory_is_set_ = false; } void AudioServiceListener::MaybeSetLogFactory() { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - DCHECK(current_instance_identity_); if (!base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess) || - !connector_ || log_factory_is_set_) + log_factory_is_set_) return; mojo::PendingRemote audio_log_factory; @@ -225,8 +153,8 @@ void AudioServiceListener::MaybeSetLogFactory() { std::make_unique(), audio_log_factory.InitWithNewPipeAndPassReceiver()); mojo::Remote log_factory_manager; - connector_->Connect(*current_instance_identity_, - log_factory_manager.BindNewPipeAndPassReceiver()); + GetAudioService().BindLogFactoryManager( + log_factory_manager.BindNewPipeAndPassReceiver()); log_factory_manager->SetLogFactory(std::move(audio_log_factory)); log_factory_is_set_ = true; } diff --git a/content/browser/renderer_host/media/audio_service_listener.h b/content/browser/renderer_host/media/audio_service_listener.h index fa5a88910c8fc8..3c7140c98b805f 100644 --- a/content/browser/renderer_host/media/audio_service_listener.h +++ b/content/browser/renderer_host/media/audio_service_listener.h @@ -13,10 +13,9 @@ #include "base/process/process_handle.h" #include "base/time/time.h" #include "content/common/content_export.h" +#include "content/public/browser/service_process_host.h" +#include "content/public/browser/service_process_info.h" #include "mojo/public/cpp/bindings/receiver.h" -#include "services/service_manager/public/cpp/connector.h" -#include "services/service_manager/public/cpp/identity.h" -#include "services/service_manager/public/mojom/service_manager.mojom.h" namespace base { class TickClock; @@ -26,7 +25,7 @@ namespace content { // Tracks the system's active audio service instance, if any exists. class CONTENT_EXPORT AudioServiceListener - : public service_manager::mojom::ServiceManagerListener { + : public ServiceProcessHost::Observer { public: class CONTENT_EXPORT Metrics { public: @@ -42,9 +41,8 @@ class CONTENT_EXPORT AudioServiceListener explicit Metrics(const base::TickClock* clock); ~Metrics(); - void ServiceAlreadyRunning(service_manager::mojom::InstanceState state); + void ServiceAlreadyRunning(); void ServiceCreated(); - void ServiceFailedToStart(); void ServiceStarted(); void ServiceStopped(); @@ -59,8 +57,7 @@ class CONTENT_EXPORT AudioServiceListener DISALLOW_COPY_AND_ASSIGN(Metrics); }; - explicit AudioServiceListener( - std::unique_ptr connector); + AudioServiceListener(); ~AudioServiceListener() override; base::ProcessId GetProcessId() const; @@ -75,26 +72,17 @@ class CONTENT_EXPORT AudioServiceListener FRIEND_TEST_ALL_PREFIXES(AudioServiceListenerTest, StartService_LogStartStatus); - // service_manager::mojom::ServiceManagerListener implementation. - void OnInit(std::vector - running_services) override; - void OnServiceCreated( - service_manager::mojom::RunningServiceInfoPtr service) override; - void OnServiceStarted(const service_manager::Identity& identity, - uint32_t pid) override; - void OnServicePIDReceived(const service_manager::Identity& identity, - uint32_t pid) override; - void OnServiceFailedToStart( - const service_manager::Identity& identity) override; - void OnServiceStopped(const service_manager::Identity& identity) override; + // Called by the constructor, or by tests to inject fake process info. + void Init(std::vector running_service_processes); + + // ServiceProcessHost::Observer implementation: + void OnServiceProcessLaunched(const ServiceProcessInfo& info) override; + void OnServiceProcessTerminatedNormally( + const ServiceProcessInfo& info) override; + void OnServiceProcessCrashed(const ServiceProcessInfo& info) override; void MaybeSetLogFactory(); - mojo::Receiver receiver_{ - this}; - std::unique_ptr connector_; - base::Optional current_instance_identity_; - base::Optional current_instance_state_; base::ProcessId process_id_ = base::kNullProcessId; Metrics metrics_; bool log_factory_is_set_ = false; diff --git a/content/browser/renderer_host/media/audio_service_listener_unittest.cc b/content/browser/renderer_host/media/audio_service_listener_unittest.cc index 3989d9c98584d0..e69f80ceb4e011 100644 --- a/content/browser/renderer_host/media/audio_service_listener_unittest.cc +++ b/content/browser/renderer_host/media/audio_service_listener_unittest.cc @@ -7,49 +7,45 @@ #include #include "base/test/metrics/histogram_tester.h" +#include "base/test/scoped_feature_list.h" #include "base/test/simple_test_tick_clock.h" #include "base/token.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/service_manager/public/cpp/identity.h" -#include "services/service_manager/public/mojom/service_manager.mojom.h" +#include "content/public/browser/service_process_host.h" +#include "content/public/common/content_features.h" +#include "content/public/test/browser_task_environment.h" +#include "services/audio/public/mojom/audio_service.mojom.h" #include "testing/gtest/include/gtest/gtest.h" -using RunningServiceInfoPtr = service_manager::mojom::RunningServiceInfoPtr; - namespace content { namespace { -service_manager::Identity MakeFakeId(const std::string& service_name) { - const base::Token kFakeInstanceGroup{1, 1}; - const base::Token kFakeInstanceId{0, 0}; - const base::Token kFakeInstanceGuid{1, 1}; - return service_manager::Identity(service_name, kFakeInstanceGroup, - kFakeInstanceId, kFakeInstanceGuid); -} - -RunningServiceInfoPtr MakeTestServiceInfo( - const service_manager::Identity& identity, - uint32_t pid) { - RunningServiceInfoPtr info(service_manager::mojom::RunningServiceInfo::New()); - info->identity = identity; - info->pid = pid; - info->state = service_manager::mojom::InstanceState::kStarted; - return info; +ServiceProcessInfo MakeFakeAudioServiceProcessInfo(base::ProcessId pid) { + ServiceProcessInfo fake_audio_process_info; + fake_audio_process_info.pid = pid; + fake_audio_process_info.service_interface_name = + audio::mojom::AudioService::Name_; + return fake_audio_process_info; } } // namespace -struct AudioServiceListenerMetricsTest : public testing::Test { - AudioServiceListenerMetricsTest() { +struct AudioServiceListenerTest : public testing::Test { + AudioServiceListenerTest() { + // This test environment is not set up to support out-of-process services. + feature_list_.InitWithFeatures( + /*enabled_features=*/{}, + /*disabled_features=*/{features::kAudioServiceOutOfProcess}); test_clock.SetNowTicks(base::TimeTicks::Now()); } + base::test::ScopedFeatureList feature_list_; + BrowserTaskEnvironment task_environment_; base::SimpleTestTickClock test_clock; base::HistogramTester histogram_tester; }; -TEST_F(AudioServiceListenerMetricsTest, +TEST_F(AudioServiceListenerTest, ServiceCreatedStartedStopped_LogsStartupTime_LogsUptime) { AudioServiceListener::Metrics metrics(&test_clock); metrics.ServiceCreated(); @@ -70,7 +66,7 @@ TEST_F(AudioServiceListenerMetricsTest, base::TimeDelta::FromHours(5), 1); } -TEST_F(AudioServiceListenerMetricsTest, +TEST_F(AudioServiceListenerTest, CreateMetricsStartService_LogsInitialDowntime) { AudioServiceListener::Metrics metrics(&test_clock); test_clock.Advance(base::TimeDelta::FromHours(12)); @@ -81,22 +77,19 @@ TEST_F(AudioServiceListenerMetricsTest, base::TimeDelta::FromHours(12), 1); } -TEST_F(AudioServiceListenerMetricsTest, - ServiceAlreadyRunningStopService_LogsUptime) { +TEST_F(AudioServiceListenerTest, ServiceAlreadyRunningStopService_LogsUptime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning( - service_manager::mojom::InstanceState::kStarted); + metrics.ServiceAlreadyRunning(); test_clock.Advance(base::TimeDelta::FromMinutes(42)); metrics.ServiceStopped(); histogram_tester.ExpectTimeBucketCount("Media.AudioService.ObservedUptime", base::TimeDelta::FromMinutes(42), 1); } -TEST_F(AudioServiceListenerMetricsTest, +TEST_F(AudioServiceListenerTest, ServiceAlreadyRunningCreateService_LogsStartupTime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning( - service_manager::mojom::InstanceState::kStarted); + metrics.ServiceAlreadyRunning(); test_clock.Advance(base::TimeDelta::FromMilliseconds(2)); metrics.ServiceStopped(); metrics.ServiceCreated(); @@ -110,11 +103,10 @@ TEST_F(AudioServiceListenerMetricsTest, // Check that if service was already created and ServiceStarted() is called, // ObservedStartupTime and ObservedInitialDowntime are not logged and start time // is reset. -TEST_F(AudioServiceListenerMetricsTest, +TEST_F(AudioServiceListenerTest, ServiceAlreadyRunningStartService_ResetStartTime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning( - service_manager::mojom::InstanceState::kCreated); + metrics.ServiceAlreadyRunning(); test_clock.Advance(base::TimeDelta::FromMilliseconds(20)); metrics.ServiceStarted(); histogram_tester.ExpectTotalCount("Media.AudioService.ObservedStartupTime", @@ -128,61 +120,33 @@ TEST_F(AudioServiceListenerMetricsTest, 1); } -TEST(AudioServiceListenerTest, StartService_LogStartStatus) { +TEST_F(AudioServiceListenerTest, StartService_LogStartStatus) { base::HistogramTester histogram_tester; - AudioServiceListener audio_service_listener(nullptr); - service_manager::Identity audio_service_identity = - MakeFakeId(audio::mojom::kServiceName); + AudioServiceListener audio_service_listener; constexpr base::ProcessId pid(42); - - std::vector instances; - instances.push_back(MakeTestServiceInfo(audio_service_identity, pid)); - audio_service_listener.OnInit(std::move(instances)); + ServiceProcessInfo audio_process_info = MakeFakeAudioServiceProcessInfo(pid); + audio_service_listener.Init({audio_process_info}); histogram_tester.ExpectBucketCount( "Media.AudioService.ObservedStartStatus", static_cast( AudioServiceListener::Metrics::ServiceStartStatus::kAlreadyStarted), 1); - audio_service_listener.OnServiceStopped(audio_service_identity); + audio_service_listener.OnServiceProcessTerminatedNormally(audio_process_info); - audio_service_listener.OnServiceCreated( - MakeTestServiceInfo(audio_service_identity, pid)); - audio_service_listener.OnServiceStarted(audio_service_identity, pid); + audio_service_listener.OnServiceProcessLaunched(audio_process_info); histogram_tester.ExpectBucketCount( "Media.AudioService.ObservedStartStatus", static_cast( AudioServiceListener::Metrics::ServiceStartStatus::kSuccess), 1); - audio_service_listener.OnServiceStopped(audio_service_identity); - - audio_service_listener.OnServiceCreated( - MakeTestServiceInfo(audio_service_identity, pid)); - audio_service_listener.OnServiceFailedToStart(audio_service_identity); - histogram_tester.ExpectBucketCount( - "Media.AudioService.ObservedStartStatus", - static_cast( - AudioServiceListener::Metrics::ServiceStartStatus::kFailure), - 1); -} - -TEST(AudioServiceListenerTest, OnInitWithoutAudioService_ProcessIdNull) { - AudioServiceListener audio_service_listener(nullptr); - service_manager::Identity id = MakeFakeId("id1"); - constexpr base::ProcessId pid(42); - std::vector instances; - instances.push_back(MakeTestServiceInfo(id, pid)); - audio_service_listener.OnInit(std::move(instances)); - EXPECT_EQ(base::kNullProcessId, audio_service_listener.GetProcessId()); + audio_service_listener.OnServiceProcessTerminatedNormally(audio_process_info); } -TEST(AudioServiceListenerTest, OnInitWithAudioService_ProcessIdNotNull) { - AudioServiceListener audio_service_listener(nullptr); - service_manager::Identity audio_service_identity = - MakeFakeId(audio::mojom::kServiceName); +TEST_F(AudioServiceListenerTest, OnInitWithAudioService_ProcessIdNotNull) { + AudioServiceListener audio_service_listener; constexpr base::ProcessId pid(42); - std::vector instances; - instances.push_back(MakeTestServiceInfo(audio_service_identity, pid)); - audio_service_listener.OnInit(std::move(instances)); + ServiceProcessInfo audio_process_info = MakeFakeAudioServiceProcessInfo(pid); + audio_service_listener.Init({audio_process_info}); EXPECT_EQ(pid, audio_service_listener.GetProcessId()); } diff --git a/content/browser/renderer_host/media/media_devices_manager.cc b/content/browser/renderer_host/media/media_devices_manager.cc index 1847e34d7e4c50..98db846e03284e 100644 --- a/content/browser/renderer_host/media/media_devices_manager.cc +++ b/content/browser/renderer_host/media/media_devices_manager.cc @@ -27,6 +27,7 @@ #include "content/browser/renderer_host/media/media_stream_manager.h" #include "content/browser/renderer_host/media/video_capture_manager.h" #include "content/browser/service_manager/service_manager_context.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/common/content_features.h" @@ -35,11 +36,7 @@ #include "media/base/media_switches.h" #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/device_notifications.mojom.h" -#include "services/service_manager/public/cpp/connector.h" -#include "services/service_manager/public/cpp/identity.h" -#include "services/service_manager/public/mojom/connector.mojom-shared.h" #if defined(OS_MACOSX) #include "base/bind_helpers.h" @@ -152,6 +149,12 @@ void ReplaceInvalidFrameRatesWithFallback(media::VideoCaptureFormats* formats) { } } +void BindDeviceNotifierFromUIThread( + mojo::PendingReceiver receiver) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + GetAudioService().BindDeviceNotifier(std::move(receiver)); +} + } // namespace std::string GuessVideoGroupID(const blink::WebMediaDeviceInfoArray& audio_infos, @@ -314,9 +317,7 @@ MediaDevicesManager::SubscriptionRequest::operator=(SubscriptionRequest&&) = class MediaDevicesManager::AudioServiceDeviceListener : public audio::mojom::DeviceListener { public: - explicit AudioServiceDeviceListener(service_manager::Connector* connector) { - TryConnectToService(connector); - } + AudioServiceDeviceListener() { ConnectToService(); } ~AudioServiceDeviceListener() override = default; void DevicesChanged() override { @@ -326,43 +327,23 @@ class MediaDevicesManager::AudioServiceDeviceListener } private: - void TryConnectToService(service_manager::Connector* connector) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - // Check if the service manager is managing the audio service. - // - // TODO: Is this necessary? We should know at build how this will respond. - connector->QueryService( - audio::mojom::kServiceName, - base::BindOnce(&AudioServiceDeviceListener::ServiceQueried, - weak_factory_.GetWeakPtr(), connector)); - } - - void ServiceQueried(service_manager::Connector* connector, - service_manager::mojom::ServiceInfoPtr info) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - // Do not connect if the service manager is not managing the audio service. - if (!info) { - LOG(WARNING) << "Audio service not available."; - return; - } - DoConnectToService(connector); - } - - void DoConnectToService(service_manager::Connector* connector) { + void ConnectToService() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(!mojo_audio_device_notifier_); DCHECK(!receiver_.is_bound()); - connector->Connect( - audio::mojom::kServiceName, - mojo_audio_device_notifier_.BindNewPipeAndPassReceiver()); + base::PostTask( + FROM_HERE, {BrowserThread::UI}, + base::BindOnce( + &BindDeviceNotifierFromUIThread, + mojo_audio_device_notifier_.BindNewPipeAndPassReceiver())); mojo_audio_device_notifier_.set_disconnect_handler(base::BindOnce( &MediaDevicesManager::AudioServiceDeviceListener::OnConnectionError, - weak_factory_.GetWeakPtr(), connector)); + weak_factory_.GetWeakPtr())); mojo_audio_device_notifier_->RegisterListener( receiver_.BindNewPipeAndPassRemote()); } - void OnConnectionError(service_manager::Connector* connector) { + void OnConnectionError() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); mojo_audio_device_notifier_.reset(); receiver_.reset(); @@ -370,9 +351,8 @@ class MediaDevicesManager::AudioServiceDeviceListener // Resetting the error handler in a posted task since doing it synchronously // results in a browser crash. See https://crbug.com/845142. base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::BindOnce(&AudioServiceDeviceListener::TryConnectToService, - weak_factory_.GetWeakPtr(), connector)); + FROM_HERE, base::BindOnce(&AudioServiceDeviceListener::ConnectToService, + weak_factory_.GetWeakPtr())); } mojo::Receiver receiver_{this}; @@ -518,17 +498,8 @@ void MediaDevicesManager::StartMonitoring() { #if defined(OS_WIN) || defined(OS_MACOSX) if (base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) { DCHECK(!audio_service_device_listener_); - if (!connector_) { - auto* connector = ServiceManagerContext::GetConnectorForIOThread(); - // |connector| can be null on unit tests. - if (!connector) - return; - - connector_ = connector->Clone(); - } - audio_service_device_listener_ = - std::make_unique(connector_.get()); + std::make_unique(); } #endif monitoring_started_ = true; diff --git a/content/browser/renderer_host/media/media_devices_manager.h b/content/browser/renderer_host/media/media_devices_manager.h index 883aa8a650217b..01b08fd55d28a6 100644 --- a/content/browser/renderer_host/media/media_devices_manager.h +++ b/content/browser/renderer_host/media/media_devices_manager.h @@ -34,10 +34,6 @@ namespace media { class AudioSystem; } -namespace service_manager { -class Connector; -} - namespace content { class MediaDevicesPermissionChecker; @@ -326,8 +322,6 @@ class CONTENT_EXPORT MediaDevicesManager // Callback used to obtain the current device ID salt and security origin. MediaDeviceSaltAndOriginCallback salt_and_origin_callback_; - std::unique_ptr connector_; - class AudioServiceDeviceListener; std::unique_ptr audio_service_device_listener_; diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc index d2b7c4a48df7d0..688c1880681e01 100644 --- a/content/browser/renderer_host/media/media_stream_manager.cc +++ b/content/browser/renderer_host/media/media_stream_manager.cc @@ -52,7 +52,6 @@ #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" -#include "content/public/browser/system_connector.h" #include "content/public/browser/web_contents_media_capture_id.h" #include "content/public/common/content_client.h" #include "content/public/common/content_features.h" @@ -581,11 +580,7 @@ MediaStreamManager::MediaStreamManager( } InitializeMaybeAsync(std::move(video_capture_provider)); - // May be null in tests. - if (GetSystemConnector()) { - audio_service_listener_ = - std::make_unique(GetSystemConnector()->Clone()); - } + audio_service_listener_ = std::make_unique(); base::PowerMonitor::AddObserver(this); } diff --git a/content/browser/renderer_host/media/render_frame_audio_input_stream_factory_unittest.cc b/content/browser/renderer_host/media/render_frame_audio_input_stream_factory_unittest.cc index 3e5a16bb127743..230a03b230b144 100644 --- a/content/browser/renderer_host/media/render_frame_audio_input_stream_factory_unittest.cc +++ b/content/browser/renderer_host/media/render_frame_audio_input_stream_factory_unittest.cc @@ -39,9 +39,7 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/cpp/fake_stream_factory.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/service_manager/public/cpp/connector.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -77,13 +75,7 @@ class MAYBE_RenderFrameAudioInputStreamFactoryTest RenderFrameHostTester::For(main_rfh())->InitializeRenderFrameIfNeeded(); // Set up the ForwardingAudioStreamFactory. - service_manager::Connector* connector = - ForwardingAudioStreamFactory::ForFrame(main_rfh()) - ->core() - ->get_connector_for_testing(); - connector->OverrideBinderForTesting( - service_manager::ServiceFilter::ByName(audio::mojom::kServiceName), - audio::mojom::StreamFactory::Name_, + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( base::BindRepeating( &MAYBE_RenderFrameAudioInputStreamFactoryTest::BindFactory, base::Unretained(this))); @@ -92,15 +84,16 @@ class MAYBE_RenderFrameAudioInputStreamFactoryTest } void TearDown() override { + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + base::NullCallback()); audio_manager_.Shutdown(); test_service_manager_context_.reset(); RenderViewHostTestHarness::TearDown(); } - void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { - audio_service_stream_factory_.receiver_.Bind( - mojo::PendingReceiver( - std::move(factory_receiver))); + void BindFactory( + mojo::PendingReceiver receiver) { + audio_service_stream_factory_.receiver_.Bind(std::move(receiver)); } class MockStreamFactory : public audio::FakeStreamFactory { diff --git a/content/browser/renderer_host/media/render_frame_audio_output_stream_factory_unittest.cc b/content/browser/renderer_host/media/render_frame_audio_output_stream_factory_unittest.cc index 61dd63ea79c249..e06a1a7eae2d19 100644 --- a/content/browser/renderer_host/media/render_frame_audio_output_stream_factory_unittest.cc +++ b/content/browser/renderer_host/media/render_frame_audio_output_stream_factory_unittest.cc @@ -35,9 +35,7 @@ #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/cpp/fake_stream_factory.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/service_manager/public/cpp/connector.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -69,13 +67,7 @@ class RenderFrameAudioOutputStreamFactoryTest RenderFrameHostTester::For(main_rfh())->InitializeRenderFrameIfNeeded(); // Set up the ForwardingAudioStreamFactory. - service_manager::Connector* connector = - ForwardingAudioStreamFactory::ForFrame(main_rfh()) - ->core() - ->get_connector_for_testing(); - connector->OverrideBinderForTesting( - service_manager::ServiceFilter::ByName(audio::mojom::kServiceName), - audio::mojom::StreamFactory::Name_, + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( base::BindRepeating( &RenderFrameAudioOutputStreamFactoryTest::BindFactory, base::Unretained(this))); @@ -84,15 +76,16 @@ class RenderFrameAudioOutputStreamFactoryTest } void TearDown() override { + ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + base::NullCallback()); audio_manager_.Shutdown(); test_service_manager_context_.reset(); RenderViewHostTestHarness::TearDown(); } - void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { - audio_service_stream_factory_.receiver_.Bind( - mojo::PendingReceiver( - std::move(factory_receiver))); + void BindFactory( + mojo::PendingReceiver receiver) { + audio_service_stream_factory_.receiver_.Bind(std::move(receiver)); } class MockStreamFactory : public audio::FakeStreamFactory { diff --git a/content/browser/service_manager/service_manager_context.cc b/content/browser/service_manager/service_manager_context.cc index 414af7b21698c0..c715f5a6fec8d8 100644 --- a/content/browser/service_manager/service_manager_context.cc +++ b/content/browser/service_manager/service_manager_context.cc @@ -47,7 +47,6 @@ #include "content/public/common/content_switches.h" #include "content/public/common/service_manager_connection.h" #include "content/public/common/service_names.mojom.h" -#include "media/audio/audio_manager.h" #include "media/media_buildflags.h" #include "media/mojo/buildflags.h" #include "media/mojo/mojom/constants.mojom.h" @@ -55,9 +54,6 @@ #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/platform/platform_channel.h" #include "mojo/public/cpp/system/invitation.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/service.h" -#include "services/audio/service_factory.h" #include "services/device/device_service.h" #include "services/device/public/mojom/constants.mojom.h" #include "services/media_session/media_session_service.h" @@ -227,13 +223,6 @@ class DeviceServiceURLLoaderFactory : public network::SharedURLLoaderFactory { DISALLOW_COPY_AND_ASSIGN(DeviceServiceURLLoaderFactory); }; -bool AudioServiceOutOfProcess() { - // Returns true iff kAudioServiceOutOfProcess feature is enabled and if the - // embedder does not provide its own in-process AudioManager. - return base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess) && - !GetContentClient()->browser()->OverridesAudioManager(); -} - using InProcessServiceFactory = base::RepeatingCallback( service_manager::mojom::ServiceRequest request)>; @@ -272,38 +261,12 @@ void RegisterInProcessService( &LaunchInProcessService, std::move(task_runner), factory); } -void CreateInProcessAudioService( - scoped_refptr task_runner, - service_manager::mojom::ServiceRequest request) { - // TODO(https://crbug.com/853254): Remove BrowserMainLoop::GetAudioManager(). - task_runner->PostTask( - FROM_HERE, base::BindOnce( - [](media::AudioManager* audio_manager, - service_manager::mojom::ServiceRequest request) { - service_manager::Service::RunAsyncUntilTermination( - audio::CreateEmbeddedService(audio_manager, - std::move(request))); - }, - BrowserMainLoop::GetAudioManager(), std::move(request))); -} - std::unique_ptr CreateMediaSessionService( service_manager::mojom::ServiceRequest request) { return std::make_unique( std::move(request)); } -void RunServiceInstanceOnIOThread( - const service_manager::Identity& identity, - mojo::PendingReceiver* receiver) { - if (!AudioServiceOutOfProcess() && - identity.name() == audio::mojom::kServiceName) { - CreateInProcessAudioService(audio::Service::GetInProcessTaskRunner(), - std::move(*receiver)); - return; - } -} - // A ServiceProcessHost implementation which uses the Service Manager's builtin // service executable launcher. Not yet intended for use in production Chrome, // hence availability is gated behind a flag. @@ -364,10 +327,6 @@ class BrowserServiceManagerDelegate return true; } - RunServiceInstanceOnIOThread(identity, &receiver); - if (!receiver) - return true; - main_thread_task_runner_->PostTask( FROM_HERE, base::BindOnce(main_thread_request_handler_, identity, std::move(receiver))); diff --git a/content/browser/service_process_host_impl.cc b/content/browser/service_process_host_impl.cc index 1d417b84d888f4..7275d26720ab3a 100644 --- a/content/browser/service_process_host_impl.cc +++ b/content/browser/service_process_host_impl.cc @@ -79,7 +79,9 @@ class ServiceProcessTracker { } void RemoveObserver(ServiceProcessHost::Observer* observer) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); + // NOTE: Some tests may remove observers after BrowserThreads are shut down. + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) || + !BrowserThread::IsThreadInitialized(BrowserThread::UI)); observers_.RemoveObserver(observer); } diff --git a/content/browser/speech/speech_recognizer_impl.cc b/content/browser/speech/speech_recognizer_impl.cc index 5f2f2fb7d7a05f..df50aa846eb629 100644 --- a/content/browser/speech/speech_recognizer_impl.cc +++ b/content/browser/speech/speech_recognizer_impl.cc @@ -16,17 +16,15 @@ #include "build/build_config.h" #include "content/browser/browser_main_loop.h" #include "content/browser/media/media_internals.h" -#include "content/browser/service_manager/service_manager_context.h" #include "content/browser/speech/audio_buffer.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/speech_recognition_event_listener.h" #include "media/audio/audio_system.h" #include "media/base/audio_converter.h" #include "media/mojo/mojom/audio_logging.mojom.h" -#include "services/audio/public/cpp/audio_system_factory.h" #include "services/audio/public/cpp/device_factory.h" -#include "services/service_manager/public/mojom/connector.mojom.h" #if defined(OS_WIN) #include "media/audio/win/core_audio_util_win.h" @@ -874,15 +872,14 @@ media::AudioSystem* SpeechRecognizerImpl::GetAudioSystem() { } void SpeechRecognizerImpl::CreateAudioCapturerSource() { - service_manager::Connector* connector = - ServiceManagerContext::GetConnectorForIOThread(); - if (connector) { - audio_capturer_source_ = audio::CreateInputDevice( - connector->Clone(), device_id_, - MediaInternals::GetInstance()->CreateMojoAudioLog( - media::AudioLogFactory::AUDIO_INPUT_CONTROLLER, - 0 /* component_id */)); - } + mojo::PendingRemote stream_factory; + GetAudioServiceStreamFactoryBinder().Run( + stream_factory.InitWithNewPipeAndPassReceiver()); + audio_capturer_source_ = audio::CreateInputDevice( + std::move(stream_factory), device_id_, + MediaInternals::GetInstance()->CreateMojoAudioLog( + media::AudioLogFactory::AUDIO_INPUT_CONTROLLER, + 0 /* component_id */)); } media::AudioCapturerSource* SpeechRecognizerImpl::GetAudioCapturerSource() { diff --git a/content/browser/speech/speech_recognizer_impl_unittest.cc b/content/browser/speech/speech_recognizer_impl_unittest.cc index 45e8eb7166a092..3384e064b05a8a 100644 --- a/content/browser/speech/speech_recognizer_impl_unittest.cc +++ b/content/browser/speech/speech_recognizer_impl_unittest.cc @@ -15,12 +15,14 @@ #include "base/stl_util.h" #include "base/synchronization/waitable_event.h" #include "base/sys_byteorder.h" +#include "base/test/scoped_feature_list.h" #include "base/threading/thread.h" #include "base/threading/thread_task_runner_handle.h" #include "content/browser/speech/proto/google_streaming_api.pb.h" #include "content/browser/speech/speech_recognition_engine.h" #include "content/browser/speech/speech_recognizer_impl.h" #include "content/public/browser/speech_recognition_event_listener.h" +#include "content/public/common/content_features.h" #include "content/public/test/browser_task_environment.h" #include "media/audio/audio_device_description.h" #include "media/audio/audio_system_impl.h" @@ -83,6 +85,11 @@ class SpeechRecognizerImplTest : public SpeechRecognitionEventListener, sound_ended_(false), error_(blink::mojom::SpeechRecognitionErrorCode::kNone), volume_(-1.0f) { + // This test environment is not set up to support out-of-process services. + feature_list_.InitWithFeatures( + /*enabled_features=*/{}, + /*disabled_features=*/{features::kAudioServiceOutOfProcess}); + // SpeechRecognizer takes ownership of sr_engine. SpeechRecognitionEngine* sr_engine = new SpeechRecognitionEngine( base::MakeRefCounted( @@ -276,6 +283,7 @@ class SpeechRecognizerImplTest : public SpeechRecognitionEventListener, } protected: + base::test::ScopedFeatureList feature_list_; BrowserTaskEnvironment task_environment_; network::TestURLLoaderFactory url_loader_factory_; scoped_refptr recognizer_; diff --git a/content/browser/utility_process_host.cc b/content/browser/utility_process_host.cc index a0786b3ca9fe27..cda2c509519912 100644 --- a/content/browser/utility_process_host.cc +++ b/content/browser/utility_process_host.cc @@ -442,12 +442,6 @@ bool UtilityProcessHost::StartProcess() { switches::kUtilityCmdPrefix)); } - const bool is_service = service_identity_.has_value(); - if (is_service) { - GetContentClient()->browser()->AdjustUtilityServiceProcessCommandLine( - *service_identity_, cmd_line.get()); - } - for (const auto& extra_switch : extra_switches_) cmd_line->AppendSwitch(extra_switch); diff --git a/content/browser/video_capture_service.cc b/content/browser/video_capture_service.cc index a60a4d18511ded..ffd7575e6214c7 100644 --- a/content/browser/video_capture_service.cc +++ b/content/browser/video_capture_service.cc @@ -41,10 +41,13 @@ void BindInProcessInstance( } mojo::Remote& GetUIThreadRemote() { - static base::NoDestructor< - mojo::Remote> - remote; - return *remote; + // NOTE: This use of sequence-local storage is only to ensure that the Remote + // only lives as long as the UI-thread sequence, since the UI-thread sequence + // may be torn down and reinitialized e.g. between unit tests. + static base::NoDestructor>> + remote_slot; + return remote_slot->GetOrCreateValue(); } // This is a custom traits type we use in conjunction with mojo::ReceiverSetBase diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 6f2ee557fb41ec..a9e3c21efa1a74 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -7136,7 +7136,7 @@ ForwardingAudioStreamFactory* WebContentsImpl::GetAudioStreamFactory() { ? static_cast( BrowserMainLoop::GetInstance()->user_input_monitor()) : nullptr, - GetSystemConnector()->Clone(), AudioStreamBrokerFactory::CreateImpl()); + AudioStreamBrokerFactory::CreateImpl()); } return &*audio_stream_factory_; diff --git a/content/browser/webrtc/webrtc_content_browsertest_base.cc b/content/browser/webrtc/webrtc_content_browsertest_base.cc index f96640da7aa5cd..db0626ea36bae4 100644 --- a/content/browser/webrtc/webrtc_content_browsertest_base.cc +++ b/content/browser/webrtc/webrtc_content_browsertest_base.cc @@ -11,7 +11,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "content/browser/web_contents/web_contents_impl.h" -#include "content/public/browser/system_connector.h" +#include "content/public/browser/audio_service.h" #include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h" @@ -19,8 +19,6 @@ #include "media/audio/audio_system.h" #include "media/base/media_switches.h" #include "net/test/embedded_test_server/embedded_test_server.h" -#include "services/audio/public/cpp/audio_system_factory.h" -#include "services/service_manager/public/cpp/connector.h" #if defined(OS_CHROMEOS) #include "chromeos/audio/cras_audio_handler.h" @@ -121,7 +119,7 @@ void WebRtcContentBrowserTestBase::ExecuteJavascriptAndWaitForOk( bool WebRtcContentBrowserTestBase::HasAudioOutputDevices() { bool has_devices = false; base::RunLoop run_loop; - auto audio_system = audio::CreateAudioSystem(GetSystemConnector()->Clone()); + auto audio_system = CreateAudioSystemForAudioService(); audio_system->HasOutputDevices(base::BindOnce( [](base::OnceClosure finished_callback, bool* result, bool received) { *result = received; diff --git a/content/browser/webrtc/webrtc_getusermedia_browsertest.cc b/content/browser/webrtc/webrtc_getusermedia_browsertest.cc index 7f722e269a7d88..34c7223de8e226 100644 --- a/content/browser/webrtc/webrtc_getusermedia_browsertest.cc +++ b/content/browser/webrtc/webrtc_getusermedia_browsertest.cc @@ -17,7 +17,7 @@ #include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/webrtc/webrtc_content_browsertest_base.h" #include "content/browser/webrtc/webrtc_internals.h" -#include "content/public/browser/system_connector.h" +#include "content/public/browser/audio_service.h" #include "content/public/common/content_features.h" #include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" @@ -28,7 +28,6 @@ #include "media/audio/fake_audio_input_stream.h" #include "media/base/media_switches.h" #include "net/test/embedded_test_server/embedded_test_server.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/testing_api.mojom.h" #include "services/service_manager/public/cpp/connector.h" @@ -803,8 +802,7 @@ IN_PROC_BROWSER_TEST_F(WebRtcGetUserMediaBrowserTest, // Crash the audio service process. mojo::Remote service_testing_api; - GetSystemConnector()->Connect( - audio::mojom::kServiceName, + GetAudioService().BindTestingApi( service_testing_api.BindNewPipeAndPassReceiver()); service_testing_api->Crash(); diff --git a/content/browser/webrtc/webrtc_internals.cc b/content/browser/webrtc/webrtc_internals.cc index c13dde3e0560b6..fd6a3eb33bd729 100644 --- a/content/browser/webrtc/webrtc_internals.cc +++ b/content/browser/webrtc/webrtc_internals.cc @@ -18,6 +18,7 @@ #include "content/browser/web_contents/web_contents_view.h" #include "content/browser/webrtc/webrtc_internals_connections_observer.h" #include "content/browser/webrtc/webrtc_internals_ui_observer.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/content_browser_client.h" @@ -567,8 +568,11 @@ void WebRTCInternals::OnRendererExit(int render_process_id) { void WebRTCInternals::EnableAudioDebugRecordingsOnAllRenderProcessHosts() { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(!audio_debug_recording_session_); + mojo::PendingRemote debug_recording; + content::GetAudioService().BindDebugRecording( + debug_recording.InitWithNewPipeAndPassReceiver()); audio_debug_recording_session_ = audio::CreateAudioDebugRecordingSession( - audio_debug_recordings_file_path_, GetSystemConnector()->Clone()); + audio_debug_recordings_file_path_, std::move(debug_recording)); for (RenderProcessHost::iterator i( content::RenderProcessHost::AllHostsIterator()); diff --git a/content/public/browser/BUILD.gn b/content/public/browser/BUILD.gn index 77524595cdeeb4..0c789957ee9f30 100644 --- a/content/public/browser/BUILD.gn +++ b/content/public/browser/BUILD.gn @@ -46,6 +46,7 @@ jumbo_source_set("browser_sources") { "appcache_service.h", "audio_loopback_stream_creator.cc", "audio_loopback_stream_creator.h", + "audio_service.h", "audio_service_info.cc", "audio_service_info.h", "authenticator_environment.h", @@ -416,6 +417,7 @@ jumbo_source_set("browser_sources") { "//media/mojo/mojom:remoting", "//mojo/public/cpp/bindings", "//mojo/public/cpp/system", + "//services/audio/public/mojom", "//services/content/public/mojom", "//services/data_decoder/public/mojom", "//services/device/public/mojom:usb", diff --git a/content/public/browser/DEPS b/content/public/browser/DEPS index 8ae3a073444360..e61bb1d3070db3 100644 --- a/content/public/browser/DEPS +++ b/content/public/browser/DEPS @@ -5,6 +5,7 @@ include_rules = [ "+components/viz/common", "+components/viz/host", "+device/fido", + "+services/audio/public", "+services/content/public", "+services/data_decoder/public", "+services/device/public", diff --git a/content/public/browser/audio_service.h b/content/public/browser/audio_service.h new file mode 100644 index 00000000000000..6fc86df3637b75 --- /dev/null +++ b/content/public/browser/audio_service.h @@ -0,0 +1,39 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_PUBLIC_BROWSER_AUDIO_SERVICE_H_ +#define CONTENT_PUBLIC_BROWSER_AUDIO_SERVICE_H_ + +#include "base/callback.h" +#include "content/common/content_export.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" +#include "services/audio/public/mojom/audio_service.mojom.h" +#include "services/audio/public/mojom/stream_factory.mojom.h" + +namespace media { +class AudioSystem; +} + +namespace content { + +// Returns the browser's main control interface into the Audio Service, which +// is started lazily and may run either in-process or in a dedicated sandboxed +// subprocess. +CONTENT_EXPORT audio::mojom::AudioService& GetAudioService(); + +// Creates an instance of AudioSystem for use with the Audio Service, bound to +// the thread it's used on for the first time. +CONTENT_EXPORT std::unique_ptr +CreateAudioSystemForAudioService(); + +// Returns a callback that can be invoked from any sequence to safely bind a +// StreamFactory interface receiver in the Audio Service. +using AudioServiceStreamFactoryBinder = base::RepeatingCallback)>; +CONTENT_EXPORT AudioServiceStreamFactoryBinder +GetAudioServiceStreamFactoryBinder(); + +} // namespace content + +#endif // CONTENT_PUBLIC_BROWSER_AUDIO_SERVICE_H_ diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h index b70473936db044..24fadd6db92e68 100644 --- a/content/public/browser/content_browser_client.h +++ b/content/public/browser/content_browser_client.h @@ -534,13 +534,6 @@ class CONTENT_EXPORT ContentBrowserClient { virtual void AppendExtraCommandLineSwitches(base::CommandLine* command_line, int child_process_id) {} - // Allows the content embedder to adjust the command line arguments for - // a utility process started to run a service. This is called on a background - // thread. - virtual void AdjustUtilityServiceProcessCommandLine( - const service_manager::Identity& identity, - base::CommandLine* command_line) {} - // Returns a client GUID used for virus scanning. virtual std::string GetApplicationClientGUIDForQuarantineCheck(); diff --git a/content/public/test/DEPS b/content/public/test/DEPS index 2c732b6dfd5d8f..b6ef2b044152e6 100644 --- a/content/public/test/DEPS +++ b/content/public/test/DEPS @@ -8,7 +8,7 @@ include_rules = [ "+components/viz/host", "+components/viz/service", "+components/viz/test", - "+services/audio/public/mojom", + "+services/audio", "+services/network", "+services/service_manager", "+services/tracing/public/cpp", diff --git a/content/public/test/audio_service_test_helper.cc b/content/public/test/audio_service_test_helper.cc index 79fdb7b188133b..5d9bffd7a0fdba 100644 --- a/content/public/test/audio_service_test_helper.cc +++ b/content/public/test/audio_service_test_helper.cc @@ -12,6 +12,7 @@ #include "base/process/process.h" #include "content/public/common/content_features.h" #include "mojo/public/cpp/bindings/receiver_set.h" +#include "services/audio/service.h" namespace content { @@ -39,17 +40,17 @@ class AudioServiceTestHelper::TestingApi : public audio::mojom::TestingApi { }; AudioServiceTestHelper::AudioServiceTestHelper() - : testing_api_(new TestingApi) {} - -AudioServiceTestHelper::~AudioServiceTestHelper() = default; - -void AudioServiceTestHelper::RegisterAudioBinders( - service_manager::BinderMap* binders) { - if (!base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) - return; + : testing_api_(new TestingApi) { + if (base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) { + audio::Service::SetTestingApiBinderForTesting( + base::BindRepeating(&AudioServiceTestHelper::BindTestingApiReceiver, + base::Unretained(this))); + } +} - binders->Add(base::BindRepeating( - &AudioServiceTestHelper::BindTestingApiReceiver, base::Unretained(this))); +AudioServiceTestHelper::~AudioServiceTestHelper() { + if (base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) + audio::Service::SetTestingApiBinderForTesting(base::NullCallback()); } void AudioServiceTestHelper::BindTestingApiReceiver( diff --git a/content/public/test/audio_service_test_helper.h b/content/public/test/audio_service_test_helper.h index 28ef8471b5cbad..3d165108ee4a20 100644 --- a/content/public/test/audio_service_test_helper.h +++ b/content/public/test/audio_service_test_helper.h @@ -14,19 +14,15 @@ namespace content { -// Used by testing environments to inject test-only interface binders into an +// Used by testing environments to inject test-only interface support into an // audio service instance. Test suites should create a long-lived instance of -// this class and call RegisterAudioBinders() on a BinderMap which will be used -// to fulfill interface requests within the audio service. +// this class to ensure that instances of the Audio service in the process are +// able to fulfill test API binding requests. class AudioServiceTestHelper { public: AudioServiceTestHelper(); ~AudioServiceTestHelper(); - // Registers the helper's interfaces on |binders|. Note that this object must - // must outlive |binders|. - void RegisterAudioBinders(service_manager::BinderMap* binders); - private: class TestingApi; diff --git a/content/public/utility/content_utility_client.h b/content/public/utility/content_utility_client.h index f64495949b7082..793d821272cd1a 100644 --- a/content/public/utility/content_utility_client.h +++ b/content/public/utility/content_utility_client.h @@ -70,8 +70,6 @@ class CONTENT_EXPORT ContentUtilityClient { virtual void RegisterNetworkBinders( service_manager::BinderRegistry* registry) {} - - virtual void RegisterAudioBinders(service_manager::BinderMap* binders) {} }; } // namespace content diff --git a/content/shell/utility/shell_content_utility_client.cc b/content/shell/utility/shell_content_utility_client.cc index c0a106554c11e8..7b197a63bc954c 100644 --- a/content/shell/utility/shell_content_utility_client.cc +++ b/content/shell/utility/shell_content_utility_client.cc @@ -139,9 +139,4 @@ void ShellContentUtilityClient::RegisterNetworkBinders( network_service_test_helper_->RegisterNetworkBinders(registry); } -void ShellContentUtilityClient::RegisterAudioBinders( - service_manager::BinderMap* binders) { - audio_service_test_helper_->RegisterAudioBinders(binders); -} - } // namespace content diff --git a/content/shell/utility/shell_content_utility_client.h b/content/shell/utility/shell_content_utility_client.h index b7da4aa7afe57e..12be6332129a7a 100644 --- a/content/shell/utility/shell_content_utility_client.h +++ b/content/shell/utility/shell_content_utility_client.h @@ -25,7 +25,6 @@ class ShellContentUtilityClient : public ContentUtilityClient { mojo::ServiceFactory* GetIOThreadServiceFactory() override; void RegisterNetworkBinders( service_manager::BinderRegistry* registry) override; - void RegisterAudioBinders(service_manager::BinderMap* binders) override; private: std::unique_ptr network_service_test_helper_; diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index ac72ecb08d649e..b88b84fa45842d 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -384,6 +384,7 @@ jumbo_static_library("test_support") { "//mojo/core/embedder", "//net:test_support", "//ppapi/c:c", + "//services/audio", "//services/audio/public/mojom", "//services/data_decoder/public/cpp:test_support", "//services/data_decoder/public/mojom", diff --git a/content/utility/BUILD.gn b/content/utility/BUILD.gn index 6cd7c287076a59..69dc3f3897ae77 100644 --- a/content/utility/BUILD.gn +++ b/content/utility/BUILD.gn @@ -42,8 +42,7 @@ jumbo_source_set("utility") { "//mojo/public/cpp/bindings", "//net", "//sandbox", - "//services/audio:lib", - "//services/audio/public/mojom:constants", + "//services/audio", "//services/data_decoder:lib", "//services/data_decoder/public/cpp", "//services/network:network_service", diff --git a/content/utility/services.cc b/content/utility/services.cc index fb5a0728794132..32c6e8a8117a95 100644 --- a/content/utility/services.cc +++ b/content/utility/services.cc @@ -8,10 +8,12 @@ #include "base/no_destructor.h" #include "base/threading/thread_task_runner_handle.h" +#include "build/build_config.h" #include "content/public/utility/content_utility_client.h" #include "content/public/utility/utility_thread.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" #include "mojo/public/cpp/bindings/service_factory.h" +#include "services/audio/service_factory.h" #include "services/data_decoder/data_decoder_service.h" #include "services/network/network_service.h" #include "services/service_manager/public/cpp/binder_registry.h" @@ -20,6 +22,13 @@ #include "services/video_capture/public/mojom/video_capture_service.mojom.h" #include "services/video_capture/video_capture_service_impl.h" +#if defined(OS_MACOSX) +#include "base/mac/mach_logging.h" +#include "sandbox/mac/system_services.h" +#include "services/service_manager/sandbox/features.h" +#include "services/service_manager/sandbox/sandbox_type.h" +#endif + namespace content { namespace { @@ -33,6 +42,39 @@ auto RunNetworkService( /*delay_initialization_until_set_client=*/true); } +auto RunAudio(mojo::PendingReceiver receiver) { +#if defined(OS_MACOSX) + // Don't connect to launch services when running sandboxed + // (https://crbug.com/874785). + if (service_manager::IsAudioSandboxEnabled()) { + sandbox::DisableLaunchServices(); + } + + // Set the audio process to run with similar scheduling parameters as the + // browser process. + task_category_policy category; + category.role = TASK_FOREGROUND_APPLICATION; + kern_return_t result = task_policy_set( + mach_task_self(), TASK_CATEGORY_POLICY, + reinterpret_cast(&category), TASK_CATEGORY_POLICY_COUNT); + + MACH_LOG_IF(ERROR, result != KERN_SUCCESS, result) + << "task_policy_set TASK_CATEGORY_POLICY"; + + task_qos_policy qos; + qos.task_latency_qos_tier = LATENCY_QOS_TIER_0; + qos.task_throughput_qos_tier = THROUGHPUT_QOS_TIER_0; + result = task_policy_set(mach_task_self(), TASK_BASE_QOS_POLICY, + reinterpret_cast(&qos), + TASK_QOS_POLICY_COUNT); + + MACH_LOG_IF(ERROR, result != KERN_SUCCESS, result) + << "task_policy_set TASK_QOS_POLICY"; +#endif + + return audio::CreateStandaloneService(std::move(receiver)); +} + auto RunDataDecoder( mojo::PendingReceiver receiver) { UtilityThread::Get()->EnsureBlinkInitialized(); @@ -60,6 +102,7 @@ mojo::ServiceFactory& GetIOThreadServiceFactory() { mojo::ServiceFactory& GetMainThreadServiceFactory() { static base::NoDestructor factory{ + RunAudio, RunDataDecoder, RunTracing, RunVideoCapture, diff --git a/content/utility/utility_service_factory.cc b/content/utility/utility_service_factory.cc index 327a490eca8f73..8a03b9283fd67e 100644 --- a/content/utility/utility_service_factory.cc +++ b/content/utility/utility_service_factory.cc @@ -22,9 +22,6 @@ #include "content/public/utility/utility_thread.h" #include "content/utility/utility_thread_impl.h" #include "media/media_buildflags.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/service_factory.h" -#include "services/network/network_service.h" #include "services/service_manager/public/mojom/service.mojom.h" #if BUILDFLAG(ENABLE_LIBRARY_CDMS) @@ -38,13 +35,6 @@ #endif // BUILDFLAG(ENABLE_CDM_HOST_VERIFICATION) #endif -#if defined(OS_MACOSX) -#include "base/mac/mach_logging.h" -#include "sandbox/mac/system_services.h" -#include "services/service_manager/sandbox/features.h" -#include "services/service_manager/sandbox/sandbox_type.h" -#endif - #if defined(OS_WIN) #include "sandbox/win/src/sandbox.h" @@ -94,13 +84,9 @@ class ContentCdmServiceClient final : public media::CdmService::Client { } // namespace -UtilityServiceFactory::UtilityServiceFactory() - : network_registry_(std::make_unique()), - audio_binders_(std::make_unique()) { - GetContentClient()->utility()->RegisterAudioBinders(audio_binders_.get()); -} +UtilityServiceFactory::UtilityServiceFactory() = default; -UtilityServiceFactory::~UtilityServiceFactory() {} +UtilityServiceFactory::~UtilityServiceFactory() = default; void UtilityServiceFactory::RunService( const std::string& service_name, @@ -115,11 +101,8 @@ void UtilityServiceFactory::RunService( base::debug::SetCrashKeyString(service_name_crash_key, service_name); std::unique_ptr service; - if (service_name == audio::mojom::kServiceName) { - service = CreateAudioService(std::move(request)); - } #if BUILDFLAG(ENABLE_LIBRARY_CDMS) - else if (service_name == media::mojom::kCdmServiceName) { + if (service_name == media::mojom::kCdmServiceName) { service = std::make_unique( std::make_unique(), std::move(request)); } @@ -146,41 +129,4 @@ void UtilityServiceFactory::RunService( utility_thread->ReleaseProcess(); } -std::unique_ptr -UtilityServiceFactory::CreateAudioService( - mojo::PendingReceiver receiver) { -#if defined(OS_MACOSX) - // Don't connect to launch services when running sandboxed - // (https://crbug.com/874785). - if (service_manager::IsAudioSandboxEnabled()) { - sandbox::DisableLaunchServices(); - } - - // Set the audio process to run with similar scheduling parameters as the - // browser process. - task_category_policy category; - category.role = TASK_FOREGROUND_APPLICATION; - kern_return_t result = task_policy_set( - mach_task_self(), TASK_CATEGORY_POLICY, - reinterpret_cast(&category), TASK_CATEGORY_POLICY_COUNT); - - MACH_LOG_IF(ERROR, result != KERN_SUCCESS, result) - << "task_policy_set TASK_CATEGORY_POLICY"; - - task_qos_policy qos; - qos.task_latency_qos_tier = LATENCY_QOS_TIER_0; - qos.task_throughput_qos_tier = THROUGHPUT_QOS_TIER_0; - result = task_policy_set(mach_task_self(), TASK_BASE_QOS_POLICY, - reinterpret_cast(&qos), - TASK_QOS_POLICY_COUNT); - - MACH_LOG_IF(ERROR, result != KERN_SUCCESS, result) - << "task_policy_set TASK_QOS_POLICY"; - -#endif - - return audio::CreateStandaloneService(std::move(audio_binders_), - std::move(receiver)); -} - } // namespace content diff --git a/content/utility/utility_service_factory.h b/content/utility/utility_service_factory.h index 280ee0ee9ba51f..654aa3553aa1a2 100644 --- a/content/utility/utility_service_factory.h +++ b/content/utility/utility_service_factory.h @@ -12,7 +12,6 @@ #include "base/memory/scoped_refptr.h" #include "base/sequenced_task_runner.h" #include "mojo/public/cpp/bindings/pending_receiver.h" -#include "services/service_manager/public/cpp/binder_map.h" #include "services/service_manager/public/cpp/binder_registry.h" #include "services/service_manager/public/cpp/service.h" #include "services/service_manager/public/mojom/service.mojom.h" @@ -30,14 +29,6 @@ class UtilityServiceFactory { mojo::PendingReceiver receiver); private: - std::unique_ptr CreateAudioService( - mojo::PendingReceiver receiver); - - // Allows embedders to register their interface implementations before the - // network or audio services are created. Used for testing. - std::unique_ptr network_registry_; - std::unique_ptr audio_binders_; - DISALLOW_COPY_AND_ASSIGN(UtilityServiceFactory); }; diff --git a/services/audio/BUILD.gn b/services/audio/BUILD.gn index 2ba73b51dc3002..27e8425ca92d86 100644 --- a/services/audio/BUILD.gn +++ b/services/audio/BUILD.gn @@ -4,25 +4,9 @@ import("//build/config/chromecast_build.gni") import("//media/webrtc/audio_processing.gni") -import("//services/service_manager/public/cpp/service_executable.gni") import("//testing/test.gni") -# Currently standalone service binaries are not supported on Android or iOS. -standalone_supported = !(is_android || is_ios) - -service_executable("audio") { - sources = [ - "service_main.cc", - ] - - deps = [ - ":lib", - "//services/audio/public/cpp", - "//services/audio/public/mojom", - ] -} - -source_set("lib") { +source_set("audio") { sources = [ "debug_recording.cc", "debug_recording.h", @@ -76,13 +60,12 @@ source_set("lib") { "sync_reader.h", "system_info.cc", "system_info.h", - "traced_service_ref.cc", - "traced_service_ref.h", "user_input_monitor.cc", "user_input_monitor.h", ] deps = [ + ":testing_api_support", "//components/crash/core/common:crash_key", # Temporary: crbug.com/888478 ] @@ -95,8 +78,7 @@ source_set("lib") { "//media", "//media/webrtc", "//services/audio/public/mojom", - "//services/service_manager/public/cpp", - "//services/service_manager/sandbox:sandbox", + "//services/service_manager/sandbox", ] if (is_linux) { @@ -125,6 +107,26 @@ source_set("lib") { } } +# NOTE: This is its own component target because it exposes static storage +# consumed by multiple binary targets that get linked together (e.g. +# content/utility and content_browsertests in a component build). Consider +# making the entire ":audio" target a component library and merging this in. +component("testing_api_support") { + visibility = [ ":audio" ] + + sources = [ + "testing_api_binder.cc", + "testing_api_binder.h", + ] + + public_deps = [ + "//base", + "//services/audio/public/mojom", + ] + + defines = [ "IS_AUDIO_SERVICE_TESTING_API_SUPPORT_IMPL" ] +} + source_set("tests") { testonly = true @@ -145,7 +147,6 @@ source_set("tests") { "public/cpp/output_device_unittest.cc", "service_metrics_unittest.cc", "snooper_node_unittest.cc", - "stream_factory_unittest.cc", "sync_reader_unittest.cc", "test/audio_system_to_service_adapter_test.cc", "test/debug_recording_session_unittest.cc", @@ -160,42 +161,25 @@ source_set("tests") { "test/mock_group_member.h", "test/mock_log.cc", "test/mock_log.h", - "test/service_lifetime_connector_test.cc", - "test/service_lifetime_test_template.h", - "test/service_observer_mock.cc", - "test/service_observer_mock.h", "user_input_monitor_unittest.cc", ] deps = [ ":audio", - ":lib", "//base/test:test_support", "//media:test_support", "//mojo/core/embedder", "//services/audio/public/cpp", - "//services/audio/public/cpp:manifest", "//services/audio/public/cpp:test_support", "//services/audio/public/mojom", - "//services/service_manager/public/cpp", - "//services/service_manager/public/cpp/test:test_support", - "//services/service_manager/public/mojom", "//testing/gmock", "//testing/gtest", ] - if (standalone_supported) { - sources += [ "test/standalone_service_test.cc" ] - } - if (is_chromeos || is_chromecast) { sources += [ "public/cpp/sounds/audio_stream_handler_unittest.cc", "public/cpp/sounds/sounds_manager_unittest.cc", ] } - - data_deps = [ - ":audio", - ] } diff --git a/services/audio/debug_recording.cc b/services/audio/debug_recording.cc index 12b2fd3d3b68a9..15d0b1ee3c4829 100644 --- a/services/audio/debug_recording.cc +++ b/services/audio/debug_recording.cc @@ -15,11 +15,8 @@ namespace audio { DebugRecording::DebugRecording( mojo::PendingReceiver receiver, - media::AudioManager* audio_manager, - TracedServiceRef service_ref) - : audio_manager_(audio_manager), - receiver_(this, std::move(receiver)), - service_ref_(std::move(service_ref)) { + media::AudioManager* audio_manager) + : audio_manager_(audio_manager), receiver_(this, std::move(receiver)) { DCHECK(audio_manager_ != nullptr); DCHECK(audio_manager_->GetTaskRunner()->BelongsToCurrentThread()); @@ -50,8 +47,6 @@ void DebugRecording::Enable( void DebugRecording::Disable() { DCHECK(audio_manager_->GetTaskRunner()->BelongsToCurrentThread()); - // Client connection is lost, resetting the reference. - service_ref_ = TracedServiceRef(); if (!IsEnabled()) return; file_provider_.reset(); diff --git a/services/audio/debug_recording.h b/services/audio/debug_recording.h index 9a13b78b3701a9..59a1f47344045d 100644 --- a/services/audio/debug_recording.h +++ b/services/audio/debug_recording.h @@ -13,7 +13,6 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/mojom/debug_recording.mojom.h" -#include "services/audio/traced_service_ref.h" namespace media { class AudioManager; @@ -26,8 +25,7 @@ namespace audio { class DebugRecording : public mojom::DebugRecording { public: DebugRecording(mojo::PendingReceiver receiver, - media::AudioManager* audio_manager, - TracedServiceRef service_ref); + media::AudioManager* audio_manager); // Disables audio debug recording if Enable() was called before. ~DebugRecording() override; @@ -51,7 +49,6 @@ class DebugRecording : public mojom::DebugRecording { media::AudioManager* const audio_manager_; mojo::Receiver receiver_; mojo::Remote file_provider_; - TracedServiceRef service_ref_; base::WeakPtrFactory weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(DebugRecording); diff --git a/services/audio/debug_recording_unittest.cc b/services/audio/debug_recording_unittest.cc index dfe738da0205ad..a07b7235bd2202 100644 --- a/services/audio/debug_recording_unittest.cc +++ b/services/audio/debug_recording_unittest.cc @@ -17,8 +17,6 @@ #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/cpp/debug_recording_session.h" #include "services/audio/public/mojom/debug_recording.mojom.h" -#include "services/audio/traced_service_ref.h" -#include "services/service_manager/public/cpp/service_keepalive.h" #include "testing/gmock/include/gmock/gmock.h" using testing::_; @@ -59,13 +57,9 @@ class MockFileProvider : public mojom::DebugRecordingFileProvider { DISALLOW_COPY_AND_ASSIGN(MockFileProvider); }; -class DebugRecordingTest : public media::AudioDebugRecordingTest, - public service_manager::ServiceKeepalive::Observer { +class DebugRecordingTest : public media::AudioDebugRecordingTest { public: - DebugRecordingTest() : service_keepalive_(nullptr, base::TimeDelta()) { - service_keepalive_.AddObserver(this); - } - + DebugRecordingTest() = default; ~DebugRecordingTest() override = default; void SetUp() override { @@ -76,17 +70,12 @@ class DebugRecordingTest : public media::AudioDebugRecordingTest, void TearDown() override { ShutdownAudioManager(); } protected: - MOCK_METHOD0(OnNoServiceRefs, void()); - void CreateDebugRecording() { if (remote_debug_recording_) remote_debug_recording_.reset(); debug_recording_ = std::make_unique( remote_debug_recording_.BindNewPipeAndPassReceiver(), - static_cast(mock_audio_manager_.get()), - TracedServiceRef(service_keepalive_.CreateRef(), - "audio::DebugRecording Binding")); - EXPECT_FALSE(service_keepalive_.HasNoRefs()); + static_cast(mock_audio_manager_.get())); } void EnableDebugRecording() { @@ -95,28 +84,21 @@ class DebugRecordingTest : public media::AudioDebugRecordingTest, remote_file_provider.InitWithNewPipeAndPassReceiver(), base::FilePath(kBaseFileName)); remote_debug_recording_->Enable(std::move(remote_file_provider)); - EXPECT_FALSE(service_keepalive_.HasNoRefs()); } void DestroyDebugRecording() { remote_debug_recording_.reset(); task_environment_.RunUntilIdle(); - EXPECT_TRUE(service_keepalive_.HasNoRefs()); } - // service_manager::ServiceKeepalive::Observer: - void OnIdleTimeout() override { OnNoServiceRefs(); } - std::unique_ptr debug_recording_; mojo::Remote remote_debug_recording_; - service_manager::ServiceKeepalive service_keepalive_; private: DISALLOW_COPY_AND_ASSIGN(DebugRecordingTest); }; TEST_F(DebugRecordingTest, EnableResetEnablesDisablesDebugRecording) { - EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(1)); CreateDebugRecording(); EXPECT_CALL(*mock_debug_recording_manager_, EnableDebugRecording(_)); @@ -127,7 +109,6 @@ TEST_F(DebugRecordingTest, EnableResetEnablesDisablesDebugRecording) { } TEST_F(DebugRecordingTest, ResetWithoutEnableDoesNotDisableDebugRecording) { - EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(1)); CreateDebugRecording(); EXPECT_CALL(*mock_debug_recording_manager_, DisableDebugRecording()).Times(0); @@ -135,7 +116,6 @@ TEST_F(DebugRecordingTest, ResetWithoutEnableDoesNotDisableDebugRecording) { } TEST_F(DebugRecordingTest, CreateWavFileCallsFileProviderCreateWavFile) { - EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(1)); CreateDebugRecording(); mojo::PendingRemote remote_file_provider; @@ -160,7 +140,6 @@ TEST_F(DebugRecordingTest, CreateWavFileCallsFileProviderCreateWavFile) { } TEST_F(DebugRecordingTest, SequencialCreate) { - EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(2)); CreateDebugRecording(); DestroyDebugRecording(); CreateDebugRecording(); @@ -170,7 +149,6 @@ TEST_F(DebugRecordingTest, SequencialCreate) { TEST_F(DebugRecordingTest, ConcurrentCreate) { CreateDebugRecording(); CreateDebugRecording(); - EXPECT_CALL(*this, OnNoServiceRefs()); DestroyDebugRecording(); } diff --git a/services/audio/device_notifier.cc b/services/audio/device_notifier.cc index 4b5aef24ffc3f2..7a127dbc849174 100644 --- a/services/audio/device_notifier.cc +++ b/services/audio/device_notifier.cc @@ -23,10 +23,10 @@ DeviceNotifier::~DeviceNotifier() { base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this); } -void DeviceNotifier::Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref) { +void DeviceNotifier::Bind( + mojo::PendingReceiver receiver) { DCHECK(task_runner_->RunsTasksInCurrentSequence()); - receivers_.Add(this, std::move(receiver), std::move(context_ref)); + receivers_.Add(this, std::move(receiver)); } void DeviceNotifier::RegisterListener( diff --git a/services/audio/device_notifier.h b/services/audio/device_notifier.h index 42a5c1714f6b3d..b031ecc2ff3f96 100644 --- a/services/audio/device_notifier.h +++ b/services/audio/device_notifier.h @@ -13,7 +13,6 @@ #include "mojo/public/cpp/bindings/receiver_set.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/mojom/device_notifications.mojom.h" -#include "services/audio/traced_service_ref.h" namespace base { class SequencedTaskRunner; @@ -29,8 +28,7 @@ class DeviceNotifier final : public base::SystemMonitor::DevicesChangedObserver, DeviceNotifier(); ~DeviceNotifier() final; - void Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref); + void Bind(mojo::PendingReceiver receiver); // mojom::DeviceNotifier implementation. void RegisterListener( @@ -45,7 +43,7 @@ class DeviceNotifier final : public base::SystemMonitor::DevicesChangedObserver, int next_listener_id_ = 0; base::flat_map> listeners_; - mojo::ReceiverSet receivers_; + mojo::ReceiverSet receivers_; const scoped_refptr task_runner_; base::WeakPtrFactory weak_factory_{this}; diff --git a/services/audio/device_notifier_unittest.cc b/services/audio/device_notifier_unittest.cc index 84b662f4f5f7e9..910fb8086223c9 100644 --- a/services/audio/device_notifier_unittest.cc +++ b/services/audio/device_notifier_unittest.cc @@ -13,8 +13,6 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/mojom/device_notifications.mojom.h" -#include "services/audio/traced_service_ref.h" -#include "services/service_manager/public/cpp/service_keepalive.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -37,48 +35,34 @@ class MockDeviceListener : public mojom::DeviceListener { } // namespace -class DeviceNotifierTest : public ::testing::Test, - public service_manager::ServiceKeepalive::Observer { +class DeviceNotifierTest : public ::testing::Test { public: DeviceNotifierTest() - : system_monitor_(std::make_unique()), - service_keepalive_(nullptr, base::TimeDelta()) { - service_keepalive_.AddObserver(this); - } + : system_monitor_(std::make_unique()) {} protected: - MOCK_METHOD0(OnNoServiceRefs, void()); - void CreateDeviceNotifier() { device_notifier_ = std::make_unique(); - device_notifier_->Bind(remote_device_notifier_.BindNewPipeAndPassReceiver(), - TracedServiceRef(service_keepalive_.CreateRef(), - "audio::DeviceNotifier Binding")); - EXPECT_FALSE(service_keepalive_.HasNoRefs()); + device_notifier_->Bind( + remote_device_notifier_.BindNewPipeAndPassReceiver()); } void DestroyDeviceNotifier() { remote_device_notifier_.reset(); task_environment_.RunUntilIdle(); - EXPECT_TRUE(service_keepalive_.HasNoRefs()); } - // service_manager::ServiceKeepalive::Observer: - void OnIdleTimeout() override { OnNoServiceRefs(); } - base::test::TaskEnvironment task_environment_; mojo::Remote remote_device_notifier_; private: std::unique_ptr system_monitor_; std::unique_ptr device_notifier_; - service_manager::ServiceKeepalive service_keepalive_; DISALLOW_COPY_AND_ASSIGN(DeviceNotifierTest); }; TEST_F(DeviceNotifierTest, DeviceNotifierNotifies) { - EXPECT_CALL(*this, OnNoServiceRefs()); CreateDeviceNotifier(); mojo::PendingRemote remote_device_listener; diff --git a/services/audio/log_factory_manager.cc b/services/audio/log_factory_manager.cc index 7c3afc39d16f7e..75bc3f4534d0bc 100644 --- a/services/audio/log_factory_manager.cc +++ b/services/audio/log_factory_manager.cc @@ -6,8 +6,6 @@ #include -#include "services/service_manager/public/cpp/service_context_ref.h" - namespace audio { LogFactoryManager::LogFactoryManager() = default; @@ -17,10 +15,9 @@ LogFactoryManager::~LogFactoryManager() { } void LogFactoryManager::Bind( - mojo::PendingReceiver receiver, - TracedServiceRef context_ref) { + mojo::PendingReceiver receiver) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - receivers_.Add(this, std::move(receiver), std::move(context_ref)); + receivers_.Add(this, std::move(receiver)); } void LogFactoryManager::SetLogFactory( diff --git a/services/audio/log_factory_manager.h b/services/audio/log_factory_manager.h index fec947f969e393..4fca633a0990a6 100644 --- a/services/audio/log_factory_manager.h +++ b/services/audio/log_factory_manager.h @@ -11,7 +11,6 @@ #include "mojo/public/cpp/bindings/receiver_set.h" #include "services/audio/log_factory_adapter.h" #include "services/audio/public/mojom/log_factory_manager.mojom.h" -#include "services/audio/traced_service_ref.h" namespace media { class AudioLogFactory; @@ -27,8 +26,7 @@ class LogFactoryManager final : public mojom::LogFactoryManager { LogFactoryManager(); ~LogFactoryManager() final; - void Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref); + void Bind(mojo::PendingReceiver receiver); // LogFactoryManager implementation. void SetLogFactory( @@ -36,7 +34,7 @@ class LogFactoryManager final : public mojom::LogFactoryManager { media::AudioLogFactory* GetLogFactory(); private: - mojo::ReceiverSet receivers_; + mojo::ReceiverSet receivers_; LogFactoryAdapter log_factory_adapter_; SEQUENCE_CHECKER(owning_sequence_); diff --git a/services/audio/log_factory_manager_unittest.cc b/services/audio/log_factory_manager_unittest.cc index a174965bf1392c..57db26002ba870 100644 --- a/services/audio/log_factory_manager_unittest.cc +++ b/services/audio/log_factory_manager_unittest.cc @@ -16,8 +16,6 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" -#include "services/audio/traced_service_ref.h" -#include "services/service_manager/public/cpp/service_keepalive.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -78,47 +76,32 @@ class MockAudioLogFactory : public media::mojom::AudioLogFactory { } // namespace -class LogFactoryManagerTest - : public ::testing::Test, - public service_manager::ServiceKeepalive::Observer { +class LogFactoryManagerTest : public ::testing::Test { public: - LogFactoryManagerTest() : service_keepalive_(nullptr, base::TimeDelta()) { - service_keepalive_.AddObserver(this); - } + LogFactoryManagerTest() = default; protected: - MOCK_METHOD0(OnNoServiceRefs, void()); - void CreateLogFactoryManager() { log_factory_manager_ = std::make_unique(); log_factory_manager_->Bind( - remote_log_factory_manager_.BindNewPipeAndPassReceiver(), - TracedServiceRef(service_keepalive_.CreateRef(), - "audio::LogFactoryManager Binding")); - EXPECT_FALSE(service_keepalive_.HasNoRefs()); + remote_log_factory_manager_.BindNewPipeAndPassReceiver()); } void DestroyLogFactoryManager() { remote_log_factory_manager_.reset(); task_environment_.RunUntilIdle(); - EXPECT_TRUE(service_keepalive_.HasNoRefs()); } - // service_manager::ServiceKeepalive::Observer: - void OnIdleTimeout() override { OnNoServiceRefs(); } - base::test::TaskEnvironment task_environment_; mojo::Remote remote_log_factory_manager_; std::unique_ptr log_factory_manager_; private: - service_manager::ServiceKeepalive service_keepalive_; DISALLOW_COPY_AND_ASSIGN(LogFactoryManagerTest); }; TEST_F(LogFactoryManagerTest, LogFactoryManagerQueuesRequestsAndSetsFactory) { - EXPECT_CALL(*this, OnNoServiceRefs()); CreateLogFactoryManager(); // Create a log before setting the log factory. diff --git a/services/audio/public/cpp/BUILD.gn b/services/audio/public/cpp/BUILD.gn index eb6adddfc5d683..43563aac5deda5 100644 --- a/services/audio/public/cpp/BUILD.gn +++ b/services/audio/public/cpp/BUILD.gn @@ -4,8 +4,6 @@ source_set("cpp") { sources = [ - "audio_system_factory.cc", - "audio_system_factory.h", "audio_system_to_service_adapter.cc", "audio_system_to_service_adapter.h", "debug_recording_session.cc", @@ -32,19 +30,6 @@ source_set("cpp") { "//base", "//media", "//services/audio/public/mojom", - "//services/service_manager/public/cpp", - ] -} - -source_set("manifest") { - sources = [ - "manifest.cc", - "manifest.h", - ] - deps = [ - "//base", - "//services/audio/public/mojom", - "//services/service_manager/public/cpp", ] } @@ -65,7 +50,7 @@ source_set("test_support") { public_deps = [ "//base", "//media", + "//services/audio", "//services/audio/public/mojom", - "//services/service_manager/public/cpp", ] } diff --git a/services/audio/public/cpp/OWNERS b/services/audio/public/cpp/OWNERS index 3e93e664999e99..a3060033221a84 100644 --- a/services/audio/public/cpp/OWNERS +++ b/services/audio/public/cpp/OWNERS @@ -1,7 +1,3 @@ -per-file manifest.cc=set noparent -per-file manifest.cc=file://ipc/SECURITY_OWNERS -per-file manifest.h=set noparent -per-file manifest.h=file://ipc/SECURITY_OWNERS per-file *_mojom_traits*.*=set noparent per-file *_mojom_traits*.*=file://ipc/SECURITY_OWNERS per-file *.typemap=set noparent diff --git a/services/audio/public/cpp/audio_system_factory.cc b/services/audio/public/cpp/audio_system_factory.cc deleted file mode 100644 index 3ef5966a9522b4..00000000000000 --- a/services/audio/public/cpp/audio_system_factory.cc +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/audio/public/cpp/audio_system_factory.h" - -#include "services/audio/public/cpp/audio_system_to_service_adapter.h" -#include "services/service_manager/public/cpp/connector.h" - -namespace audio { - -std::unique_ptr CreateAudioSystem( - std::unique_ptr connector) { - constexpr auto service_disconnect_timeout = base::TimeDelta::FromSeconds(1); - return std::make_unique( - std::move(connector), service_disconnect_timeout); -} - -} // namespace audio diff --git a/services/audio/public/cpp/audio_system_factory.h b/services/audio/public/cpp/audio_system_factory.h deleted file mode 100644 index cb9beb10b0404b..00000000000000 --- a/services/audio/public/cpp/audio_system_factory.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SERVICES_AUDIO_PUBLIC_CPP_AUDIO_SYSTEM_FACTORY_H_ -#define SERVICES_AUDIO_PUBLIC_CPP_AUDIO_SYSTEM_FACTORY_H_ - -#include - -namespace media { -class AudioSystem; -} - -namespace service_manager { -class Connector; -} - -namespace audio { - -// Creates an instance of AudioSystem which will be bound to the thread it's -// used on for the first time. -std::unique_ptr CreateAudioSystem( - std::unique_ptr connector); - -} // namespace audio - -#endif // SERVICES_AUDIO_PUBLIC_CPP_AUDIO_SYSTEM_FACTORY_H_ diff --git a/services/audio/public/cpp/audio_system_to_service_adapter.cc b/services/audio/public/cpp/audio_system_to_service_adapter.cc index 82a5d28197422d..486dfb5ae71e6c 100644 --- a/services/audio/public/cpp/audio_system_to_service_adapter.cc +++ b/services/audio/public/cpp/audio_system_to_service_adapter.cc @@ -12,8 +12,6 @@ #include "base/trace_event/trace_event.h" #include "media/audio/audio_device_description.h" #include "mojo/public/cpp/bindings/callback_helpers.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/service_manager/public/cpp/connector.h" namespace audio { @@ -225,21 +223,22 @@ OnInputDeviceInfoCallback WrapGetInputDeviceInfoReply( } // namespace AudioSystemToServiceAdapter::AudioSystemToServiceAdapter( - std::unique_ptr connector, + SystemInfoBinder system_info_binder, base::TimeDelta disconnect_timeout) - : connector_(std::move(connector)), + : system_info_binder_(std::move(system_info_binder)), disconnect_timeout_(disconnect_timeout) { - DCHECK(connector_); + DCHECK(system_info_binder_); DETACH_FROM_THREAD(thread_checker_); } AudioSystemToServiceAdapter::AudioSystemToServiceAdapter( - std::unique_ptr connector) - : AudioSystemToServiceAdapter(std::move(connector), base::TimeDelta()) {} + SystemInfoBinder system_info_binder) + : AudioSystemToServiceAdapter(std::move(system_info_binder), + base::TimeDelta()) {} AudioSystemToServiceAdapter::~AudioSystemToServiceAdapter() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - if (!!system_info_) { + if (system_info_.is_bound()) { TRACE_EVENT_NESTABLE_ASYNC_END1("audio", "AudioSystemToServiceAdapter bound", this, "disconnect reason", "destroyed"); @@ -320,14 +319,12 @@ mojom::SystemInfo* AudioSystemToServiceAdapter::GetSystemInfo() { if (!system_info_) { TRACE_EVENT_NESTABLE_ASYNC_BEGIN0( "audio", "AudioSystemToServiceAdapter bound", this); - connector_->Connect(mojom::kServiceName, - system_info_.BindNewPipeAndPassReceiver()); + system_info_binder_.Run(system_info_.BindNewPipeAndPassReceiver()); system_info_.set_disconnect_handler( base::BindOnce(&AudioSystemToServiceAdapter::OnConnectionError, base::Unretained(this))); if (!disconnect_timeout_.is_zero()) system_info_.reset_on_idle_timeout(disconnect_timeout_); - DCHECK(system_info_); } return system_info_.get(); diff --git a/services/audio/public/cpp/audio_system_to_service_adapter.h b/services/audio/public/cpp/audio_system_to_service_adapter.h index 3dbdd19b9e5812..a0729d64f840a8 100644 --- a/services/audio/public/cpp/audio_system_to_service_adapter.h +++ b/services/audio/public/cpp/audio_system_to_service_adapter.h @@ -8,18 +8,16 @@ #include #include +#include "base/callback.h" #include "base/optional.h" #include "base/threading/thread_checker.h" #include "base/time/time.h" #include "base/timer/timer.h" #include "media/audio/audio_system.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/mojom/system_info.mojom.h" -namespace service_manager { -class Connector; -} - namespace audio { // Provides media::AudioSystem implementation on top of Audio service. @@ -27,14 +25,17 @@ namespace audio { // empty optionals / false booleans. class AudioSystemToServiceAdapter : public media::AudioSystem { public: + // A callback which can be used to acquire a new SystemInfo interface pipe + // lazily as needed. + using SystemInfoBinder = + base::RepeatingCallback)>; + // If |disconnect_timeout| is positive, the instance will disconnect from // Audio service upon |disconnect_timeout| if not in use, and reconnect on the // next attempt to use it. - AudioSystemToServiceAdapter( - std::unique_ptr connector, - base::TimeDelta disconnect_timeout); - explicit AudioSystemToServiceAdapter( - std::unique_ptr connector); + AudioSystemToServiceAdapter(SystemInfoBinder system_info_binder, + base::TimeDelta disconnect_timeout); + explicit AudioSystemToServiceAdapter(SystemInfoBinder system_info_binder); ~AudioSystemToServiceAdapter() override; // AudioSystem implementation. @@ -61,7 +62,7 @@ class AudioSystemToServiceAdapter : public media::AudioSystem { void OnConnectionError(); // Will be bound to the thread AudioSystemToServiceAdapter is used on. - const std::unique_ptr connector_; + const SystemInfoBinder system_info_binder_; mojo::Remote system_info_; // To disconnect from the audio service when not in use. diff --git a/services/audio/public/cpp/debug_recording_session.cc b/services/audio/public/cpp/debug_recording_session.cc index 9307d6e0782082..40d38902b124e2 100644 --- a/services/audio/public/cpp/debug_recording_session.cc +++ b/services/audio/public/cpp/debug_recording_session.cc @@ -11,8 +11,6 @@ #include "base/strings/string_number_conversions.h" #include "build/build_config.h" #include "media/audio/audio_debug_recording_manager.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/service_manager/public/cpp/connector.h" namespace audio { @@ -67,17 +65,14 @@ void DebugRecordingSession::DebugRecordingFileProvider::CreateWavFile( DebugRecordingSession::DebugRecordingSession( const base::FilePath& file_name_base, - std::unique_ptr connector) { - DCHECK(connector); - + mojo::PendingRemote debug_recording) { mojo::PendingRemote remote_file_provider; file_provider_ = std::make_unique( remote_file_provider.InitWithNewPipeAndPassReceiver(), file_name_base); - - connector->Connect(audio::mojom::kServiceName, - debug_recording_.BindNewPipeAndPassReceiver()); - if (debug_recording_.is_bound()) + if (debug_recording) { + debug_recording_.Bind(std::move(debug_recording)); debug_recording_->Enable(std::move(remote_file_provider)); + } } DebugRecordingSession::~DebugRecordingSession() {} diff --git a/services/audio/public/cpp/debug_recording_session.h b/services/audio/public/cpp/debug_recording_session.h index 9cb25e0c60b20a..4d6e55d8d06277 100644 --- a/services/audio/public/cpp/debug_recording_session.h +++ b/services/audio/public/cpp/debug_recording_session.h @@ -10,14 +10,11 @@ #include "media/audio/audio_debug_recording_helper.h" #include "media/audio/audio_debug_recording_session.h" #include "mojo/public/cpp/bindings/pending_receiver.h" +#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/remote.h" #include "services/audio/public/mojom/debug_recording.mojom.h" -namespace service_manager { -class Connector; -} - namespace base { class FilePath; } @@ -54,8 +51,9 @@ class DebugRecordingSession : public media::AudioDebugRecordingSession { DISALLOW_COPY_AND_ASSIGN(DebugRecordingFileProvider); }; - DebugRecordingSession(const base::FilePath& file_name_base, - std::unique_ptr connector); + DebugRecordingSession( + const base::FilePath& file_name_base, + mojo::PendingRemote debug_recording); ~DebugRecordingSession() override; private: diff --git a/services/audio/public/cpp/debug_recording_session_factory.cc b/services/audio/public/cpp/debug_recording_session_factory.cc index 1be75c7be4dee4..ad283713a063af 100644 --- a/services/audio/public/cpp/debug_recording_session_factory.cc +++ b/services/audio/public/cpp/debug_recording_session_factory.cc @@ -8,18 +8,16 @@ #include "base/files/file_path.h" #include "services/audio/public/cpp/debug_recording_session.h" -#include "services/service_manager/public/cpp/connector.h" namespace audio { std::unique_ptr CreateAudioDebugRecordingSession( const base::FilePath& debug_recording_file_path, - std::unique_ptr connector) { - DCHECK(connector); - + mojo::PendingRemote debug_recording) { + DCHECK(debug_recording); return std::make_unique( - debug_recording_file_path, std::move(connector)); + debug_recording_file_path, std::move(debug_recording)); } } // namespace audio diff --git a/services/audio/public/cpp/debug_recording_session_factory.h b/services/audio/public/cpp/debug_recording_session_factory.h index fae9da88e19750..0732b76310d82c 100644 --- a/services/audio/public/cpp/debug_recording_session_factory.h +++ b/services/audio/public/cpp/debug_recording_session_factory.h @@ -7,6 +7,8 @@ #include +#include "services/audio/public/mojom/debug_recording.mojom.h" + namespace base { class FilePath; } @@ -15,16 +17,12 @@ namespace media { class AudioDebugRecordingSession; } -namespace service_manager { -class Connector; -} - namespace audio { std::unique_ptr CreateAudioDebugRecordingSession( const base::FilePath& debug_recording_file_path, - std::unique_ptr connector); + mojo::PendingRemote debug_recording); } // namespace audio diff --git a/services/audio/public/cpp/device_factory.cc b/services/audio/public/cpp/device_factory.cc index acb8ed396f5d1c..3ecbe054180d9a 100644 --- a/services/audio/public/cpp/device_factory.cc +++ b/services/audio/public/cpp/device_factory.cc @@ -10,22 +10,15 @@ #include "base/bind.h" #include "base/threading/platform_thread.h" #include "services/audio/public/cpp/input_ipc.h" -#include "services/audio/public/mojom/constants.mojom.h" namespace audio { scoped_refptr CreateInputDevice( - std::unique_ptr connector, + mojo::PendingRemote stream_factory, const std::string& device_id, mojo::PendingRemote log) { - DCHECK(connector); - mojo::PendingRemote stream_factory; - connector->Connect(audio::mojom::kServiceName, - stream_factory.InitWithNewPipeAndPassReceiver()); - std::unique_ptr ipc = std::make_unique( std::move(stream_factory), device_id, std::move(log)); - return base::MakeRefCounted( std::move(ipc), media::AudioInputDevice::Purpose::kUserInput); } @@ -33,11 +26,8 @@ scoped_refptr CreateInputDevice( scoped_refptr CreateInputDevice( mojo::PendingRemote stream_factory, const std::string& device_id) { - std::unique_ptr ipc = std::make_unique( - std::move(stream_factory), device_id, mojo::NullRemote()); - - return base::MakeRefCounted( - std::move(ipc), media::AudioInputDevice::Purpose::kUserInput); + return CreateInputDevice(std::move(stream_factory), device_id, + mojo::NullRemote()); } } // namespace audio diff --git a/services/audio/public/cpp/device_factory.h b/services/audio/public/cpp/device_factory.h index 1d7525ade84ccb..a079a470fbccce 100644 --- a/services/audio/public/cpp/device_factory.h +++ b/services/audio/public/cpp/device_factory.h @@ -11,7 +11,6 @@ #include "media/audio/audio_input_device.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/service_manager/public/cpp/connector.h" namespace audio { @@ -20,7 +19,7 @@ scoped_refptr CreateInputDevice( const std::string& device_id); scoped_refptr CreateInputDevice( - std::unique_ptr connector, + mojo::PendingRemote stream_factory, const std::string& device_id, mojo::PendingRemote); diff --git a/services/audio/public/cpp/fake_system_info.cc b/services/audio/public/cpp/fake_system_info.cc index 5e778bfbab1553..e7a25e24abd12a 100644 --- a/services/audio/public/cpp/fake_system_info.cc +++ b/services/audio/public/cpp/fake_system_info.cc @@ -5,8 +5,7 @@ #include "services/audio/public/cpp/fake_system_info.h" #include "base/bind.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/service_manager/public/cpp/service_binding.h" +#include "services/audio/service.h" namespace audio { @@ -17,16 +16,13 @@ FakeSystemInfo::~FakeSystemInfo() {} // static void FakeSystemInfo::OverrideGlobalBinderForAudioService( FakeSystemInfo* fake_system_info) { - service_manager::ServiceBinding::OverrideInterfaceBinderForTesting( - mojom::kServiceName, - base::BindRepeating(&FakeSystemInfo::Bind, - base::Unretained(fake_system_info))); + Service::SetSystemInfoBinderForTesting(base::BindRepeating( + &FakeSystemInfo::Bind, base::Unretained(fake_system_info))); } // static void FakeSystemInfo::ClearGlobalBinderForAudioService() { - service_manager::ServiceBinding ::ClearInterfaceBinderOverrideForTesting< - mojom::SystemInfo>(mojom::kServiceName); + Service::SetSystemInfoBinderForTesting(base::NullCallback()); } void FakeSystemInfo::GetInputStreamParameters( diff --git a/services/audio/public/cpp/input_ipc_unittest.cc b/services/audio/public/cpp/input_ipc_unittest.cc index f723c6f1ba442e..2799503d8d479b 100644 --- a/services/audio/public/cpp/input_ipc_unittest.cc +++ b/services/audio/public/cpp/input_ipc_unittest.cc @@ -17,7 +17,6 @@ #include "services/audio/public/cpp/device_factory.h" #include "services/audio/public/cpp/fake_stream_factory.h" #include "services/audio/public/mojom/audio_processing.mojom.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/services/audio/public/cpp/manifest.cc b/services/audio/public/cpp/manifest.cc deleted file mode 100644 index c6ce7bbaec6865..00000000000000 --- a/services/audio/public/cpp/manifest.cc +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/audio/public/cpp/manifest.h" - -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/public/mojom/debug_recording.mojom.h" -#include "services/audio/public/mojom/device_notifications.mojom.h" -#include "services/audio/public/mojom/log_factory_manager.mojom.h" -#include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/audio/public/mojom/system_info.mojom.h" -#include "services/audio/public/mojom/testing_api.mojom.h" -#include "services/service_manager/public/cpp/manifest_builder.h" - -namespace audio { - -service_manager::Manifest GetManifest( - service_manager::Manifest::ExecutionMode execution_mode) { - return service_manager::Manifest{ - service_manager::ManifestBuilder() - .WithServiceName(mojom::kServiceName) - .WithDisplayName("Audio Service") - .WithOptions(service_manager::ManifestOptionsBuilder() - .WithExecutionMode(execution_mode) - .WithSandboxType("audio") - .WithInstanceSharingPolicy( - service_manager::Manifest:: - InstanceSharingPolicy::kSharedAcrossGroups) - .Build()) - .ExposeCapability( - "debug_recording", - service_manager::Manifest::InterfaceList()) - .ExposeCapability( - "device_notifier", - service_manager::Manifest::InterfaceList()) - .ExposeCapability( - "info", - service_manager::Manifest::InterfaceList()) - .ExposeCapability("log_factory_manager", - service_manager::Manifest::InterfaceList< - mojom::LogFactoryManager>()) - .ExposeCapability( - "stream_factory", - service_manager::Manifest::InterfaceList()) - .ExposeCapability( - "testing_api", - service_manager::Manifest::InterfaceList()) - .Build()}; -} - -} // namespace audio diff --git a/services/audio/public/cpp/manifest.h b/services/audio/public/cpp/manifest.h deleted file mode 100644 index 50b7096276ba40..00000000000000 --- a/services/audio/public/cpp/manifest.h +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SERVICES_AUDIO_PUBLIC_CPP_MANIFEST_H_ -#define SERVICES_AUDIO_PUBLIC_CPP_MANIFEST_H_ - -#include "services/service_manager/public/cpp/manifest.h" - -namespace audio { - -service_manager::Manifest GetManifest( - service_manager::Manifest::ExecutionMode execution_mode); - -} // namespace audio - -#endif // SERVICES_AUDIO_PUBLIC_CPP_MANIFEST_H_ diff --git a/services/audio/public/cpp/output_device_unittest.cc b/services/audio/public/cpp/output_device_unittest.cc index 38169c9d4497ae..b336e350d989ca 100644 --- a/services/audio/public/cpp/output_device_unittest.cc +++ b/services/audio/public/cpp/output_device_unittest.cc @@ -16,7 +16,6 @@ #include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/system/platform_handle.h" #include "services/audio/public/cpp/fake_stream_factory.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/sync_reader.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/services/audio/public/cpp/sounds/audio_stream_handler.cc b/services/audio/public/cpp/sounds/audio_stream_handler.cc index 1b2d21967a9535..847ea18ae063d9 100644 --- a/services/audio/public/cpp/sounds/audio_stream_handler.cc +++ b/services/audio/public/cpp/sounds/audio_stream_handler.cc @@ -20,7 +20,6 @@ #include "media/base/channel_layout.h" #include "media/mojo/mojom/audio_output_stream.mojom.h" #include "services/audio/public/cpp/output_device.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" namespace audio { @@ -44,10 +43,10 @@ class AudioStreamHandler::AudioStreamContainer : public media::AudioRendererSink::RenderCallback { public: explicit AudioStreamContainer( - std::unique_ptr connector, + SoundsManager::StreamFactoryBinder stream_factory_binder, std::unique_ptr wav_audio) : started_(false), - connector_(std::move(connector)), + stream_factory_binder_(std::move(stream_factory_binder)), cursor_(0), delayed_stop_posted_(false), wav_audio_(std::move(wav_audio)) { @@ -72,8 +71,8 @@ class AudioStreamHandler::AudioStreamContainer g_observer_for_testing->Initialize(this, params); } else { mojo::PendingRemote stream_factory; - connector_->Connect(audio::mojom::kServiceName, - stream_factory.InitWithNewPipeAndPassReceiver()); + stream_factory_binder_.Run( + stream_factory.InitWithNewPipeAndPassReceiver()); device_ = std::make_unique( std::move(stream_factory), params, this, std::string()); } @@ -156,7 +155,7 @@ class AudioStreamHandler::AudioStreamContainer } bool started_; - std::unique_ptr connector_; + const SoundsManager::StreamFactoryBinder stream_factory_binder_; std::unique_ptr device_; scoped_refptr task_runner_; @@ -171,7 +170,7 @@ class AudioStreamHandler::AudioStreamContainer }; AudioStreamHandler::AudioStreamHandler( - std::unique_ptr connector, + SoundsManager::StreamFactoryBinder stream_factory_binder, const base::StringPiece& wav_data) { task_runner_ = base::SequencedTaskRunnerHandle::Get(); std::unique_ptr wav_audio = @@ -192,8 +191,8 @@ AudioStreamHandler::AudioStreamHandler( // Store the duration of the WAV data then pass the handler to |stream_|. duration_ = wav_audio->GetDuration(); - stream_.reset( - new AudioStreamContainer(std::move(connector), std::move(wav_audio))); + stream_.reset(new AudioStreamContainer(std::move(stream_factory_binder), + std::move(wav_audio))); } AudioStreamHandler::~AudioStreamHandler() { diff --git a/services/audio/public/cpp/sounds/audio_stream_handler.h b/services/audio/public/cpp/sounds/audio_stream_handler.h index 8d98fe3377882a..b8e6d7fe855804 100644 --- a/services/audio/public/cpp/sounds/audio_stream_handler.h +++ b/services/audio/public/cpp/sounds/audio_stream_handler.h @@ -18,7 +18,7 @@ #include "media/base/audio_parameters.h" #include "media/base/audio_renderer_sink.h" #include "media/base/media_export.h" -#include "services/service_manager/public/cpp/connector.h" +#include "services/audio/public/cpp/sounds/sounds_manager.h" namespace audio { @@ -45,7 +45,7 @@ class AudioStreamHandler { // C-tor for AudioStreamHandler. |wav_data| should be a raw // uncompressed WAVE data which will be sent to the audio output device. explicit AudioStreamHandler( - std::unique_ptr connector, + SoundsManager::StreamFactoryBinder stream_factory_binder, const base::StringPiece& wav_data); virtual ~AudioStreamHandler(); diff --git a/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc b/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc index 17a7d851f46f78..493e552ea35bd9 100644 --- a/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc +++ b/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc @@ -32,17 +32,10 @@ class AudioStreamHandlerTest : public testing::Test { AudioStreamHandlerTest() = default; ~AudioStreamHandlerTest() override = default; - void SetUp() override { - mojo::PendingReceiver connector_receiver; - connector_ = service_manager::Connector::Create(&connector_receiver); - } - void SetObserverForTesting(AudioStreamHandler::TestObserver* observer) { AudioStreamHandler::SetObserverForTesting(observer); } - std::unique_ptr connector_; - private: base::TestMessageLoop message_loop_; }; @@ -54,8 +47,7 @@ TEST_F(AudioStreamHandlerTest, Play) { std::unique_ptr audio_stream_handler; SetObserverForTesting(&observer); - audio_stream_handler.reset( - new AudioStreamHandler(std::move(connector_), data)); + audio_stream_handler.reset(new AudioStreamHandler(base::DoNothing(), data)); ASSERT_TRUE(audio_stream_handler->IsInitialized()); EXPECT_EQ(base::TimeDelta::FromMicroseconds(20u), @@ -79,8 +71,7 @@ TEST_F(AudioStreamHandlerTest, ConsecutivePlayRequests) { std::unique_ptr audio_stream_handler; SetObserverForTesting(&observer); - audio_stream_handler.reset( - new AudioStreamHandler(std::move(connector_), data)); + audio_stream_handler.reset(new AudioStreamHandler(base::DoNothing(), data)); ASSERT_TRUE(audio_stream_handler->IsInitialized()); EXPECT_EQ(base::TimeDelta::FromMicroseconds(20u), @@ -109,8 +100,7 @@ TEST_F(AudioStreamHandlerTest, ConsecutivePlayRequests) { TEST_F(AudioStreamHandlerTest, BadWavDataDoesNotInitialize) { // The class members and SetUp() will be ignored for this test. Create a // handler on the stack with some bad WAV data. - AudioStreamHandler handler(std::move(connector_), - "RIFF1234WAVEjunkjunkjunkjunk"); + AudioStreamHandler handler(base::DoNothing(), "RIFF1234WAVEjunkjunkjunkjunk"); EXPECT_FALSE(handler.IsInitialized()); EXPECT_FALSE(handler.Play()); EXPECT_EQ(base::TimeDelta(), handler.duration()); diff --git a/services/audio/public/cpp/sounds/sounds_manager.cc b/services/audio/public/cpp/sounds/sounds_manager.cc index 520968e6d0c25c..3d08eb89309ffc 100644 --- a/services/audio/public/cpp/sounds/sounds_manager.cc +++ b/services/audio/public/cpp/sounds/sounds_manager.cc @@ -23,8 +23,8 @@ bool g_initialized_for_testing = false; class SoundsManagerImpl : public SoundsManager { public: - SoundsManagerImpl(std::unique_ptr connector) - : connector_(std::move(connector)) {} + SoundsManagerImpl(StreamFactoryBinder stream_factory_binder) + : stream_factory_binder_(std::move(stream_factory_binder)) {} ~SoundsManagerImpl() override { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); } @@ -44,7 +44,7 @@ class SoundsManagerImpl : public SoundsManager { std::unique_ptr handler; }; std::vector handlers_; - std::unique_ptr connector_; + StreamFactoryBinder stream_factory_binder_; DISALLOW_COPY_AND_ASSIGN(SoundsManagerImpl); }; @@ -57,7 +57,7 @@ bool SoundsManagerImpl::Initialize(SoundKey key, } std::unique_ptr handler( - new AudioStreamHandler(connector_->Clone(), data)); + new AudioStreamHandler(stream_factory_binder_, data)); if (!handler->IsInitialized()) { LOG(WARNING) << "Can't initialize AudioStreamHandler for key=" << key; return false; @@ -105,13 +105,12 @@ SoundsManager::~SoundsManager() { } // static -void SoundsManager::Create( - std::unique_ptr connector) { +void SoundsManager::Create(StreamFactoryBinder stream_factory_binder) { CHECK(!g_instance || g_initialized_for_testing) << "SoundsManager::Create() is called twice"; if (g_initialized_for_testing) return; - g_instance = new SoundsManagerImpl(std::move(connector)); + g_instance = new SoundsManagerImpl(std::move(stream_factory_binder)); } // static diff --git a/services/audio/public/cpp/sounds/sounds_manager.h b/services/audio/public/cpp/sounds/sounds_manager.h index 2975ad3f10c714..7128cb5f1aaef2 100644 --- a/services/audio/public/cpp/sounds/sounds_manager.h +++ b/services/audio/public/cpp/sounds/sounds_manager.h @@ -7,12 +7,14 @@ #include +#include "base/callback.h" #include "base/macros.h" #include "base/sequence_checker.h" #include "base/strings/string_piece.h" #include "base/time/time.h" #include "media/base/media_export.h" -#include "services/service_manager/public/cpp/connector.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" +#include "services/audio/public/mojom/stream_factory.mojom.h" namespace audio { @@ -23,7 +25,9 @@ class SoundsManager { typedef int SoundKey; // Creates a singleton instance of the SoundsManager. - static void Create(std::unique_ptr connector); + using StreamFactoryBinder = base::RepeatingCallback)>; + static void Create(StreamFactoryBinder stream_factory_binder); // Removes a singleton instance of the SoundsManager. static void Shutdown(); diff --git a/services/audio/public/cpp/sounds/sounds_manager_unittest.cc b/services/audio/public/cpp/sounds/sounds_manager_unittest.cc index cfe1769d66add2..95ec58fb835f69 100644 --- a/services/audio/public/cpp/sounds/sounds_manager_unittest.cc +++ b/services/audio/public/cpp/sounds/sounds_manager_unittest.cc @@ -29,9 +29,7 @@ class SoundsManagerTest : public testing::Test { ~SoundsManagerTest() override = default; void SetUp() override { - mojo::PendingReceiver connector_receiver; - connector_ = service_manager::Connector::Create(&connector_receiver); - SoundsManager::Create(connector_->Clone()); + SoundsManager::Create(base::DoNothing()); base::RunLoop().RunUntilIdle(); } @@ -44,8 +42,6 @@ class SoundsManagerTest : public testing::Test { AudioStreamHandler::SetObserverForTesting(observer); } - std::unique_ptr connector_; - private: base::TestMessageLoop message_loop_; }; diff --git a/services/audio/public/mojom/BUILD.gn b/services/audio/public/mojom/BUILD.gn index 78ae7f19f1f338..5808a33db1f7f7 100644 --- a/services/audio/public/mojom/BUILD.gn +++ b/services/audio/public/mojom/BUILD.gn @@ -8,6 +8,7 @@ mojom("mojom") { sources = [ "audio_device_description.mojom", "audio_processing.mojom", + "audio_service.mojom", "debug_recording.mojom", "device_notifications.mojom", "log_factory_manager.mojom", @@ -17,14 +18,7 @@ mojom("mojom") { ] public_deps = [ - ":constants", "//media/mojo/mojom", "//mojo/public/mojom/base", ] } - -mojom("constants") { - sources = [ - "constants.mojom", - ] -} diff --git a/services/audio/public/mojom/audio_service.mojom b/services/audio/public/mojom/audio_service.mojom new file mode 100644 index 00000000000000..53b4505f6465bb --- /dev/null +++ b/services/audio/public/mojom/audio_service.mojom @@ -0,0 +1,36 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +module audio.mojom; + +import "services/audio/public/mojom/debug_recording.mojom"; +import "services/audio/public/mojom/device_notifications.mojom"; +import "services/audio/public/mojom/log_factory_manager.mojom"; +import "services/audio/public/mojom/stream_factory.mojom"; +import "services/audio/public/mojom/system_info.mojom"; +import "services/audio/public/mojom/testing_api.mojom"; + +// The main interface to the Audio service. This is a privileged interface and +// must only be bound by trusted processes, e.g. a browser process. +interface AudioService { + // Binds a SystemInfo interface receiver. + BindSystemInfo(pending_receiver receiver); + + // Binds a DebugRecording interface receiver. + BindDebugRecording(pending_receiver receiver); + + // Binds a StreamFactory interface receiver. + BindStreamFactory(pending_receiver receiver); + + // Binds a DeviceNotifier interface receiver. + BindDeviceNotifier(pending_receiver receiver); + + // Binds a LogFactoryManager interface receiver. + BindLogFactoryManager(pending_receiver receiver); + + // Binds a TestingApi interface receiver. Only callable in some test + // environments. + BindTestingApi(pending_receiver receiver); +}; + diff --git a/services/audio/public/mojom/constants.mojom b/services/audio/public/mojom/constants.mojom deleted file mode 100644 index 0d4b0cc5fdb84a..00000000000000 --- a/services/audio/public/mojom/constants.mojom +++ /dev/null @@ -1,7 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -module audio.mojom; - -const string kServiceName = "audio"; diff --git a/services/audio/service.cc b/services/audio/service.cc index 246dd45a3bc8d9..b86b8812f5d6ff 100644 --- a/services/audio/service.cc +++ b/services/audio/service.cc @@ -32,22 +32,24 @@ namespace audio { namespace { + // TODO(crbug.com/888478): Remove this after diagnosis. crash_reporter::CrashKeyString<64> g_service_state_for_crashing( "audio-service-state"); + +Service::SystemInfoBinder& GetSystemInfoBinder() { + static base::NoDestructor binder; + return *binder; +} + } // namespace -Service::Service( - std::unique_ptr audio_manager_accessor, - base::Optional quit_timeout, - bool enable_remote_client_support, - std::unique_ptr extra_binders, - mojo::PendingReceiver receiver) - : service_binding_(this, std::move(receiver)), - keepalive_(&service_binding_, quit_timeout), +Service::Service(std::unique_ptr audio_manager_accessor, + bool enable_remote_client_support, + mojo::PendingReceiver receiver) + : receiver_(this, std::move(receiver)), audio_manager_accessor_(std::move(audio_manager_accessor)), - enable_remote_client_support_(enable_remote_client_support), - binders_(std::move(extra_binders)) { + enable_remote_client_support_(enable_remote_client_support) { magic_bytes_ = 0x600DC0DEu; g_service_state_for_crashing.Set("constructing"); DCHECK(audio_manager_accessor_); @@ -63,6 +65,13 @@ Service::Service( // created. This is required for in-process device notifications. InitializeDeviceMonitor(); } + TRACE_EVENT0("audio", "audio::Service::OnStart") + + // This will pre-create AudioManager if AudioManagerAccessor owns it. + CHECK(audio_manager_accessor_->GetAudioManager()); + + metrics_ = + std::make_unique(base::DefaultTickClock::GetInstance()); g_service_state_for_crashing.Set("constructed"); } @@ -91,106 +100,64 @@ Service::~Service() { // static base::DeferredSequencedTaskRunner* Service::GetInProcessTaskRunner() { - static base::NoDestructor> + static const base::NoDestructor< + scoped_refptr> instance(base::MakeRefCounted()); return instance->get(); } -void Service::OnStart() { - CHECK_EQ(magic_bytes_, 0x600DC0DEu); - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - g_service_state_for_crashing.Set("starting"); - TRACE_EVENT0("audio", "audio::Service::OnStart") - - // This will pre-create AudioManager if AudioManagerAccessor owns it. - CHECK(audio_manager_accessor_->GetAudioManager()); - - metrics_ = - std::make_unique(base::DefaultTickClock::GetInstance()); - binders_->Add(base::BindRepeating(&Service::BindSystemInfoReceiver, - base::Unretained(this))); - binders_->Add(base::BindRepeating(&Service::BindDebugRecordingReceiver, - base::Unretained(this))); - binders_->Add(base::BindRepeating(&Service::BindStreamFactoryReceiver, - base::Unretained(this))); - if (enable_remote_client_support_) { - binders_->Add(base::BindRepeating(&Service::BindDeviceNotifierReceiver, - base::Unretained(this))); - binders_->Add(base::BindRepeating(&Service::BindLogFactoryManagerReceiver, - base::Unretained(this))); - } - g_service_state_for_crashing.Set("started"); -} - -void Service::OnBindInterface( - const service_manager::BindSourceInfo& source_info, - const std::string& interface_name, - mojo::ScopedMessagePipeHandle receiver_pipe) { - CHECK_EQ(magic_bytes_, 0x600DC0DEu); - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - g_service_state_for_crashing.Set("binding " + interface_name); - TRACE_EVENT1("audio", "audio::Service::OnBindInterface", "interface", - interface_name); - - if (keepalive_.HasNoRefs()) - metrics_->HasConnections(); - - binders_->TryBind(interface_name, &receiver_pipe); - DCHECK(!keepalive_.HasNoRefs()); - - g_service_state_for_crashing.Set("bound " + interface_name); +// static +void Service::SetSystemInfoBinderForTesting(SystemInfoBinder binder) { + GetSystemInfoBinder() = std::move(binder); } -void Service::OnDisconnected() { - CHECK_EQ(magic_bytes_, 0x600DC0DEu); - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - g_service_state_for_crashing.Set("connection lost"); - Terminate(); +// static +void Service::SetTestingApiBinderForTesting(TestingApiBinder binder) { + GetTestingApiBinder() = std::move(binder); } -void Service::BindSystemInfoReceiver( +void Service::BindSystemInfo( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + auto& binder_override = GetSystemInfoBinder(); + if (binder_override) { + binder_override.Run(std::move(receiver)); + return; + } + if (!system_info_) { system_info_ = std::make_unique( audio_manager_accessor_->GetAudioManager()); } - system_info_->Bind( - std::move(receiver), - TracedServiceRef(keepalive_.CreateRef(), "audio::SystemInfo Binding")); + system_info_->Bind(std::move(receiver)); } -void Service::BindDebugRecordingReceiver( +void Service::BindDebugRecording( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - TracedServiceRef service_ref(keepalive_.CreateRef(), - "audio::DebugRecording Binding"); // Accept only one bind request at a time. Old receiver is overwritten. // |debug_recording_| must be reset first to disable debug recording, and then // create a new DebugRecording instance to enable debug recording. debug_recording_.reset(); debug_recording_ = std::make_unique( - std::move(receiver), audio_manager_accessor_->GetAudioManager(), - std::move(service_ref)); + std::move(receiver), audio_manager_accessor_->GetAudioManager()); } -void Service::BindStreamFactoryReceiver( +void Service::BindStreamFactory( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); if (!stream_factory_) stream_factory_.emplace(audio_manager_accessor_->GetAudioManager()); - stream_factory_->Bind( - std::move(receiver), - TracedServiceRef(keepalive_.CreateRef(), "audio::StreamFactory Binding")); + stream_factory_->Bind(std::move(receiver)); } -void Service::BindDeviceNotifierReceiver( +void Service::BindDeviceNotifier( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -199,21 +166,23 @@ void Service::BindDeviceNotifierReceiver( InitializeDeviceMonitor(); if (!device_notifier_) device_notifier_ = std::make_unique(); - device_notifier_->Bind(std::move(receiver), - TracedServiceRef(keepalive_.CreateRef(), - "audio::DeviceNotifier Binding")); + device_notifier_->Bind(std::move(receiver)); } -void Service::BindLogFactoryManagerReceiver( +void Service::BindLogFactoryManager( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK(log_factory_manager_); DCHECK(enable_remote_client_support_); - log_factory_manager_->Bind( - std::move(receiver), - TracedServiceRef(keepalive_.CreateRef(), - "audio::LogFactoryManager Binding")); + log_factory_manager_->Bind(std::move(receiver)); +} + +void Service::BindTestingApi( + mojo::PendingReceiver receiver) { + auto& binder = GetTestingApiBinder(); + if (binder) + binder.Run(std::move(receiver)); } void Service::InitializeDeviceMonitor() { diff --git a/services/audio/service.h b/services/audio/service.h index b37b5393755849..cc99501b4e7bd5 100644 --- a/services/audio/service.h +++ b/services/audio/service.h @@ -14,17 +14,15 @@ #include "base/threading/thread_checker.h" #include "build/build_config.h" #include "mojo/public/cpp/bindings/pending_receiver.h" +#include "services/audio/public/mojom/audio_service.mojom.h" #include "services/audio/public/mojom/debug_recording.mojom.h" #include "services/audio/public/mojom/device_notifications.mojom.h" #include "services/audio/public/mojom/log_factory_manager.mojom.h" #include "services/audio/public/mojom/stream_factory.mojom.h" #include "services/audio/public/mojom/system_info.mojom.h" +#include "services/audio/public/mojom/testing_api.mojom.h" #include "services/audio/stream_factory.h" -#include "services/service_manager/public/cpp/binder_map.h" -#include "services/service_manager/public/cpp/service.h" -#include "services/service_manager/public/cpp/service_binding.h" -#include "services/service_manager/public/cpp/service_keepalive.h" -#include "services/service_manager/public/mojom/service.mojom.h" +#include "services/audio/testing_api_binder.h" namespace base { class DeferredSequencedTaskRunner; @@ -44,7 +42,7 @@ class LogFactoryManager; class ServiceMetrics; class SystemInfo; -class Service : public service_manager::Service { +class Service : public mojom::AudioService { public: // Abstracts AudioManager ownership. Lives and must be accessed on a thread // its created on, and that thread must be AudioManager main thread. @@ -65,41 +63,43 @@ class Service : public service_manager::Service { virtual void SetAudioLogFactory(media::AudioLogFactory* factory) = 0; }; - // Service will attempt to quit if there are no connections to it within - // |quit_timeout| interval. If |quit_timeout| is null the - // service never quits. If |enable_remote_client_support| is true, the service - // will make available a DeviceNotifier object that allows clients to - // subscribe to notifications about device changes and a LogFactoryManager - // object that allows clients to set a factory for audio logs. + // If |enable_remote_client_support| is true, the service will make available + // a DeviceNotifier object that allows clients to/ subscribe to notifications + // about device changes and a LogFactoryManager object that allows clients to + // set a factory for audio logs. Service(std::unique_ptr audio_manager_accessor, - base::Optional quit_timeout, bool enable_remote_client_support, - std::unique_ptr extra_binders, - mojo::PendingReceiver receiver); + mojo::PendingReceiver receiver); ~Service() final; // Returns a DeferredSequencedTaskRunner to be used to run the audio service // when launched in the browser process. static base::DeferredSequencedTaskRunner* GetInProcessTaskRunner(); - // service_manager::Service implementation. - void OnStart() final; - void OnBindInterface(const service_manager::BindSourceInfo& source_info, - const std::string& interface_name, - mojo::ScopedMessagePipeHandle receiver_pipe) final; - void OnDisconnected() final; + // Allows tests to override how SystemInfo interface receivers are bound. + // Used by FakeSystemInfo. + using SystemInfoBinder = + base::RepeatingCallback)>; + static void SetSystemInfoBinderForTesting(SystemInfoBinder binder); + + // Allows tests to inject support for TestingApi binding, which is normally + // unsupported by the service. + static void SetTestingApiBinderForTesting(TestingApiBinder binder); private: - void BindSystemInfoReceiver( - mojo::PendingReceiver receiver); - void BindDebugRecordingReceiver( - mojo::PendingReceiver receiver); - void BindStreamFactoryReceiver( - mojo::PendingReceiver receiver); - void BindDeviceNotifierReceiver( - mojo::PendingReceiver receiver); - void BindLogFactoryManagerReceiver( - mojo::PendingReceiver receiver); + // mojom::AudioService implementation: + void BindSystemInfo( + mojo::PendingReceiver receiver) override; + void BindDebugRecording( + mojo::PendingReceiver receiver) override; + void BindStreamFactory( + mojo::PendingReceiver receiver) override; + void BindDeviceNotifier( + mojo::PendingReceiver receiver) override; + void BindLogFactoryManager( + mojo::PendingReceiver receiver) override; + void BindTestingApi( + mojo::PendingReceiver receiver) override; // Initializes a platform-specific device monitor for device-change // notifications. If the client uses the DeviceNotifier interface to get @@ -112,11 +112,9 @@ class Service : public service_manager::Service { // AudioManager provided by AudioManagerAccessor. THREAD_CHECKER(thread_checker_); - service_manager::ServiceBinding service_binding_; - service_manager::ServiceKeepalive keepalive_; - base::RepeatingClosure quit_closure_; + mojo::Receiver receiver_; std::unique_ptr audio_manager_accessor_; const bool enable_remote_client_support_; std::unique_ptr system_monitor_; @@ -130,8 +128,6 @@ class Service : public service_manager::Service { std::unique_ptr log_factory_manager_; std::unique_ptr metrics_; - std::unique_ptr binders_; - // TODO(crbug.com/888478): Remove this after diagnosis. volatile uint32_t magic_bytes_; diff --git a/services/audio/service_factory.cc b/services/audio/service_factory.cc index 083cebc25e15b8..827167b9cc63b4 100644 --- a/services/audio/service_factory.cc +++ b/services/audio/service_factory.cc @@ -4,16 +4,9 @@ #include "services/audio/service_factory.h" -#include #include #include "base/bind.h" -#include "base/command_line.h" -#include "base/metrics/field_trial_params.h" -#include "base/optional.h" -#include "base/strings/string_number_conversions.h" -#include "base/time/time.h" -#include "build/build_config.h" #include "media/audio/audio_manager.h" #include "media/base/media_switches.h" #include "services/audio/in_process_audio_manager_accessor.h" @@ -22,60 +15,20 @@ namespace audio { -namespace { - -base::Optional GetExperimentalQuitTimeout() { - std::string timeout_str( - base::GetFieldTrialParamValue("AudioService", "teardown_timeout_s")); - int timeout_s = 0; - if (!base::StringToInt(timeout_str, &timeout_s)) - return base::nullopt; // Ill-formed value provided, fall back to default. - return base::TimeDelta::FromSeconds(timeout_s); -} - -base::Optional GetCommandLineQuitTimeout() { - const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess(); - if (!cmd_line->HasSwitch(switches::kAudioServiceQuitTimeoutMs)) - return base::nullopt; - std::string timeout_str( - cmd_line->GetSwitchValueASCII(switches::kAudioServiceQuitTimeoutMs)); - int timeout_ms = 0; - if (!base::StringToInt(timeout_str, &timeout_ms)) - return base::nullopt; // Ill-formed value provided, fall back to default. - return base::TimeDelta::FromMilliseconds(timeout_ms); -} - -base::Optional GetQuitTimeout() { - if (auto timeout = GetCommandLineQuitTimeout()) - return *timeout >= base::TimeDelta() ? timeout : base::nullopt; - - if (auto timeout = GetExperimentalQuitTimeout()) - return *timeout >= base::TimeDelta() ? timeout : base::nullopt; - - // Default timeout. - return base::TimeDelta::FromMinutes(15); -} - -} // namespace - std::unique_ptr CreateEmbeddedService( media::AudioManager* audio_manager, - mojo::PendingReceiver receiver) { + mojo::PendingReceiver receiver) { return std::make_unique( std::make_unique(audio_manager), - base::nullopt /* do not quit if all clients disconnected */, - false /* enable_device_notifications */, - std::make_unique(), std::move(receiver)); + /*enable_remote_client_support=*/false, std::move(receiver)); } std::unique_ptr CreateStandaloneService( - std::unique_ptr extra_binders, - mojo::PendingReceiver receiver) { + mojo::PendingReceiver receiver) { return std::make_unique( std::make_unique( base::BindOnce(&media::AudioManager::Create)), - GetQuitTimeout(), true /* enable_remote_client_support */, - std::move(extra_binders), std::move(receiver)); + /*enable_remote_client_support=*/true, std::move(receiver)); } } // namespace audio diff --git a/services/audio/service_factory.h b/services/audio/service_factory.h index c00990eb4b65e1..7690b7ff90c83d 100644 --- a/services/audio/service_factory.h +++ b/services/audio/service_factory.h @@ -8,9 +8,8 @@ #include #include "mojo/public/cpp/bindings/pending_receiver.h" +#include "services/audio/public/mojom/audio_service.mojom.h" #include "services/audio/service.h" -#include "services/service_manager/public/cpp/binder_map.h" -#include "services/service_manager/public/mojom/service.mojom.h" namespace media { class AudioManager; @@ -23,15 +22,14 @@ namespace audio { // the device thread of AudioManager. std::unique_ptr CreateEmbeddedService( media::AudioManager* audio_manager, - mojo::PendingReceiver receiver); + mojo::PendingReceiver receiver); // Creates an instance of Audio service which will live in the current process // and will create and own an AudioManager instance. |extra_binders| can provide // additional interface binders for the service to include. Useful for e.g. // test-only environments. std::unique_ptr CreateStandaloneService( - std::unique_ptr extra_binders, - mojo::PendingReceiver receiver); + mojo::PendingReceiver receiver); } // namespace audio diff --git a/services/audio/service_main.cc b/services/audio/service_main.cc deleted file mode 100644 index 9f3c96398c3d36..00000000000000 --- a/services/audio/service_main.cc +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include - -#include "base/task/single_thread_task_executor.h" -#include "services/audio/service.h" -#include "services/audio/service_factory.h" -#include "services/service_manager/public/cpp/binder_map.h" -#include "services/service_manager/public/cpp/service_executable/service_main.h" - -void ServiceMain(service_manager::mojom::ServiceRequest request) { - base::SingleThreadTaskExecutor main_thread_task_executor; - audio::CreateStandaloneService(std::make_unique(), - std::move(request)) - ->RunUntilTermination(); -} diff --git a/services/audio/stream_factory.cc b/services/audio/stream_factory.cc index 4f2076f5c14e2b..b9adb5d90a2f05 100644 --- a/services/audio/stream_factory.cc +++ b/services/audio/stream_factory.cc @@ -34,11 +34,10 @@ StreamFactory::~StreamFactory() { magic_bytes_ = 0xDEADBEEFu; } -void StreamFactory::Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref) { +void StreamFactory::Bind(mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - receivers_.Add(this, std::move(receiver), std::move(context_ref)); + receivers_.Add(this, std::move(receiver)); } void StreamFactory::CreateInputStream( @@ -56,9 +55,9 @@ void StreamFactory::CreateInputStream( CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); SetStateForCrashing("creating input stream"); - TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( - "audio", "CreateInputStream", receivers_.current_context().id_for_trace(), - "device id", device_id, "params", params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2("audio", "CreateInputStream", this, + "device id", device_id, "params", + params.AsHumanReadableString()); if (processing_config && processing_config->settings.requires_apm() && params.GetBufferDuration() != base::TimeDelta::FromMilliseconds(10)) { @@ -116,10 +115,9 @@ void StreamFactory::CreateOutputStream( CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); SetStateForCrashing("creating output stream"); - TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( - "audio", "CreateOutputStream", - receivers_.current_context().id_for_trace(), "device id", - output_device_id, "params", params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2("audio", "CreateOutputStream", this, + "device id", output_device_id, "params", + params.AsHumanReadableString()); // Unretained is safe since |this| indirectly owns the OutputStream. auto deleter_callback = base::BindOnce(&StreamFactory::DestroyOutputStream, @@ -153,9 +151,8 @@ void StreamFactory::BindMuter( CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); SetStateForCrashing("binding muter"); - TRACE_EVENT_NESTABLE_ASYNC_INSTANT1( - "audio", "BindMuter", receivers_.current_context().id_for_trace(), - "group id", group_id.GetLowForSerialization()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT1("audio", "BindMuter", this, "group id", + group_id.GetLowForSerialization()); // Find the existing LocalMuter for this group, or create one on-demand. auto it = std::find_if(muters_.begin(), muters_.end(), @@ -189,11 +186,10 @@ void StreamFactory::CreateLoopbackStream( CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); SetStateForCrashing("creating loopback stream"); - TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( - "audio", "CreateLoopbackStream", - receivers_.current_context().id_for_trace(), "group id", - group_id.GetLowForSerialization(), "params", - params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2("audio", "CreateLoopbackStream", this, + "group id", + group_id.GetLowForSerialization(), + "params", params.AsHumanReadableString()); // All LoopbackStreams share a single realtime worker thread. This is because // the execution timing of scheduled tasks must be precise, and top priority diff --git a/services/audio/stream_factory.h b/services/audio/stream_factory.h index 2480950e769c14..f2ca6ee2695f04 100644 --- a/services/audio/stream_factory.h +++ b/services/audio/stream_factory.h @@ -22,7 +22,6 @@ #include "services/audio/loopback_coordinator.h" #include "services/audio/public/mojom/stream_factory.mojom.h" #include "services/audio/stream_monitor_coordinator.h" -#include "services/audio/traced_service_ref.h" namespace base { class UnguessableToken; @@ -49,8 +48,7 @@ class StreamFactory final : public mojom::StreamFactory { explicit StreamFactory(media::AudioManager* audio_manager); ~StreamFactory() final; - void Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref); + void Bind(mojo::PendingReceiver receiver); // StreamFactory implementation. void CreateInputStream( @@ -109,7 +107,7 @@ class StreamFactory final : public mojom::StreamFactory { media::AudioManager* const audio_manager_; - mojo::ReceiverSet receivers_; + mojo::ReceiverSet receivers_; // Order of the following members is important for a clean shutdown. LoopbackCoordinator coordinator_; diff --git a/services/audio/stream_factory_unittest.cc b/services/audio/stream_factory_unittest.cc deleted file mode 100644 index 06dff3e0408344..00000000000000 --- a/services/audio/stream_factory_unittest.cc +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/audio/stream_factory.h" - -#include - -#include "base/test/task_environment.h" -#include "media/audio/mock_audio_manager.h" -#include "media/audio/test_audio_thread.h" -#include "mojo/public/cpp/bindings/remote.h" -#include "services/audio/public/mojom/stream_factory.mojom.h" -#include "services/audio/traced_service_ref.h" -#include "services/service_manager/public/cpp/service_keepalive.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace audio { - -// Stream creation is tested as part of the stream unit tests. -TEST(AudioServiceStreamFactoryTest, TakesServiceRef) { - base::test::TaskEnvironment env; - service_manager::ServiceKeepalive keepalive{nullptr, base::nullopt}; - media::MockAudioManager audio_manager( - std::make_unique()); - - StreamFactory factory(&audio_manager); - - mojo::Remote remote_factory; - - factory.Bind( - remote_factory.BindNewPipeAndPassReceiver(), - TracedServiceRef(keepalive.CreateRef(), "audio::StreamFactory binding")); - EXPECT_FALSE(keepalive.HasNoRefs()); - remote_factory.reset(); - env.RunUntilIdle(); - EXPECT_TRUE(keepalive.HasNoRefs()); - audio_manager.Shutdown(); -} - -} // namespace audio diff --git a/services/audio/system_info.cc b/services/audio/system_info.cc index 4f6c624885cfcc..04ce14562283ff 100644 --- a/services/audio/system_info.cc +++ b/services/audio/system_info.cc @@ -7,7 +7,6 @@ #include #include "base/trace_event/trace_event.h" -#include "services/service_manager/public/cpp/service_context_ref.h" namespace audio { @@ -20,10 +19,9 @@ SystemInfo::~SystemInfo() { DCHECK_CALLED_ON_VALID_SEQUENCE(binding_sequence_checker_); } -void SystemInfo::Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref) { +void SystemInfo::Bind(mojo::PendingReceiver receiver) { DCHECK_CALLED_ON_VALID_SEQUENCE(binding_sequence_checker_); - receivers_.Add(this, std::move(receiver), std::move(context_ref)); + receivers_.Add(this, std::move(receiver)); } void SystemInfo::GetInputStreamParameters( diff --git a/services/audio/system_info.h b/services/audio/system_info.h index 504881bcbdd3d9..a99714346cec32 100644 --- a/services/audio/system_info.h +++ b/services/audio/system_info.h @@ -14,7 +14,6 @@ #include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/receiver_set.h" #include "services/audio/public/mojom/system_info.mojom.h" -#include "services/audio/traced_service_ref.h" namespace media { class AudioManager; @@ -27,8 +26,7 @@ class SystemInfo : public mojom::SystemInfo { explicit SystemInfo(media::AudioManager* audio_manager); ~SystemInfo() override; - void Bind(mojo::PendingReceiver receiver, - TracedServiceRef context_ref); + void Bind(mojo::PendingReceiver receiver); private: // audio::mojom::SystemInfo implementation. @@ -52,9 +50,7 @@ class SystemInfo : public mojom::SystemInfo { media::AudioSystemHelper helper_; - // Each receiver increases ref count of the service context, so that the - // service knows when it is in use. - mojo::ReceiverSet receivers_; + mojo::ReceiverSet receivers_; // Validates thread-safe access to |bindings_| only. |helper_| takes care of // its thread safety/affinity itself. diff --git a/services/audio/test/audio_system_to_service_adapter_test.cc b/services/audio/test/audio_system_to_service_adapter_test.cc index 8ec249d9dad484..e157694a42b8de 100644 --- a/services/audio/test/audio_system_to_service_adapter_test.cc +++ b/services/audio/test/audio_system_to_service_adapter_test.cc @@ -12,10 +12,7 @@ #include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/receiver.h" #include "services/audio/in_process_audio_manager_accessor.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/system_info.h" -#include "services/service_manager/public/cpp/connector.h" -#include "services/service_manager/public/mojom/connector.mojom.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -41,18 +38,10 @@ class AudioSystemToServiceAdapterTestBase : public testing::Test { std::make_unique(audio_manager_.get()); system_info_receiver_ = std::make_unique>( system_info_impl_.get()); - - mojo::PendingReceiver ignored_receiver; - auto connector = service_manager::Connector::Create(&ignored_receiver); - connector->OverrideBinderForTesting( - service_manager::ServiceFilter::ByName(mojom::kServiceName), - mojom::SystemInfo::Name_, - base::BindRepeating( + audio_system_ = + std::make_unique(base::BindRepeating( &AudioSystemToServiceAdapterTestBase::BindSystemInfoReceiver, base::Unretained(this))); - - audio_system_ = - std::make_unique(std::move(connector)); } void TearDown() override { @@ -77,14 +66,14 @@ class AudioSystemToServiceAdapterTestBase : public testing::Test { std::unique_ptr audio_system_; private: - void BindSystemInfoReceiver(mojo::ScopedMessagePipeHandle handle) { + void BindSystemInfoReceiver( + mojo::PendingReceiver receiver) { EXPECT_TRUE(system_info_receiver_) << "AudioSystemToServiceAdapter should " "not request AudioSysteInfo during " "construction"; - EXPECT_FALSE(system_info_receiver_->is_bound()); + ASSERT_FALSE(system_info_receiver_->is_bound()); system_info_bind_requested_.Call(); - system_info_receiver_->Bind( - mojo::PendingReceiver(std::move(handle))); + system_info_receiver_->Bind(std::move(receiver)); } DISALLOW_COPY_AND_ASSIGN(AudioSystemToServiceAdapterTestBase); @@ -405,22 +394,16 @@ class AudioSystemToServiceAdapterDisconnectTest : public testing::Test { } }; // class MockSystemInfo - std::unique_ptr GetConnector() { - mojo::PendingReceiver ignored_receiver; - auto connector = service_manager::Connector::Create(&ignored_receiver); - connector->OverrideBinderForTesting( - service_manager::ServiceFilter::ByName(mojom::kServiceName), - mojom::SystemInfo::Name_, - base::BindRepeating( - &AudioSystemToServiceAdapterDisconnectTest::BindSystemInfoReceiver, - base::Unretained(this))); - return connector; + AudioSystemToServiceAdapter::SystemInfoBinder GetSystemInfoBinder() { + return base::BindRepeating( + &AudioSystemToServiceAdapterDisconnectTest::BindSystemInfoReceiver, + base::Unretained(this)); } - void BindSystemInfoReceiver(mojo::ScopedMessagePipeHandle handle) { + void BindSystemInfoReceiver( + mojo::PendingReceiver receiver) { ClientConnected(); - system_info_receiver_.Bind( - mojo::PendingReceiver(std::move(handle))); + system_info_receiver_.Bind(std::move(receiver)); system_info_receiver_.set_disconnect_handler(base::BindOnce( &AudioSystemToServiceAdapterDisconnectTest::OnConnectionError, base::Unretained(this))); @@ -447,7 +430,8 @@ class AudioSystemToServiceAdapterDisconnectTest : public testing::Test { TEST_F(AudioSystemToServiceAdapterDisconnectTest, ResponseDelayIsShorterThanDisconnectTimeout) { const base::TimeDelta kDisconnectTimeout = kResponseDelay * 2; - AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), + kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -463,7 +447,8 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, ResponseDelayIsLongerThanDisconnectTimeout) { const base::TimeDelta kDisconnectTimeout = kResponseDelay / 2; - AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), + kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -479,7 +464,8 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, DisconnectTimeoutIsResetOnSecondRequest) { const base::TimeDelta kDisconnectTimeout = kResponseDelay * 1.5; - AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), + kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -502,7 +488,8 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, DoesNotDisconnectIfNoTimeout) { - AudioSystemToServiceAdapter audio_system(GetConnector(), base::TimeDelta()); + AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), + base::TimeDelta()); EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); EXPECT_CALL(response_received_, Run(valid_reply_)); diff --git a/services/audio/test/debug_recording_session_unittest.cc b/services/audio/test/debug_recording_session_unittest.cc index 8886144dd320a7..237d2e63f6707c 100644 --- a/services/audio/test/debug_recording_session_unittest.cc +++ b/services/audio/test/debug_recording_session_unittest.cc @@ -19,10 +19,8 @@ #include "media/audio/mock_audio_debug_recording_manager.h" #include "media/audio/mock_audio_manager.h" #include "mojo/public/cpp/bindings/remote.h" -#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/service.h" #include "services/audio/service_factory.h" -#include "services/service_manager/public/cpp/test/test_connector_factory.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -93,7 +91,7 @@ class DebugRecordingSessionTest : public media::AudioDebugRecordingTest { service_ = CreateEmbeddedService( static_cast(mock_audio_manager_.get()), - connector_factory_.RegisterInstance(audio::mojom::kServiceName)); + service_remote_.BindNewPipeAndPassReceiver()); task_environment_.RunUntilIdle(); } @@ -102,10 +100,12 @@ class DebugRecordingSessionTest : public media::AudioDebugRecordingTest { protected: std::unique_ptr CreateDebugRecordingSession() { + mojo::PendingRemote debug_recording; + service_remote_->BindDebugRecording( + debug_recording.InitWithNewPipeAndPassReceiver()); std::unique_ptr session( - std::make_unique( - base::FilePath(kBaseFileName), - connector_factory_.CreateConnector())); + std::make_unique(base::FilePath(kBaseFileName), + std::move(debug_recording))); task_environment_.RunUntilIdle(); return session; } @@ -117,7 +117,7 @@ class DebugRecordingSessionTest : public media::AudioDebugRecordingTest { } private: - service_manager::TestConnectorFactory connector_factory_; + mojo::Remote service_remote_; std::unique_ptr service_; DISALLOW_COPY_AND_ASSIGN(DebugRecordingSessionTest); diff --git a/services/audio/test/in_process_service_test.cc b/services/audio/test/in_process_service_test.cc index 0735fcd9978985..0eba84a326e152 100644 --- a/services/audio/test/in_process_service_test.cc +++ b/services/audio/test/in_process_service_test.cc @@ -10,15 +10,8 @@ #include "services/audio/in_process_audio_manager_accessor.h" #include "services/audio/public/cpp/audio_system_to_service_adapter.h" #include "services/audio/public/cpp/fake_system_info.h" -#include "services/audio/public/cpp/manifest.h" -#include "services/audio/public/mojom/constants.mojom.h" +#include "services/audio/public/mojom/audio_service.mojom.h" #include "services/audio/service.h" -#include "services/audio/test/service_lifetime_test_template.h" -#include "services/service_manager/public/cpp/manifest_builder.h" -#include "services/service_manager/public/cpp/service.h" -#include "services/service_manager/public/cpp/service_binding.h" -#include "services/service_manager/public/cpp/test/test_service_manager.h" -#include "services/service_manager/public/mojom/constants.mojom.h" #include "testing/gtest/include/gtest/gtest.h" using testing::Exactly; @@ -26,32 +19,27 @@ using testing::Invoke; namespace audio { -class ServiceTestHelper : public service_manager::Service { +class ServiceTestHelper { public: class AudioThreadContext : public base::RefCountedThreadSafe { public: - AudioThreadContext(media::AudioManager* audio_manager, - base::TimeDelta service_quit_timeout) - : audio_manager_(audio_manager), - service_quit_timeout_(service_quit_timeout) {} + explicit AudioThreadContext(media::AudioManager* audio_manager) + : audio_manager_(audio_manager) {} void CreateServiceOnAudioThread( - service_manager::mojom::ServiceRequest request) { + mojo::PendingReceiver receiver) { if (!audio_manager_->GetTaskRunner()->BelongsToCurrentThread()) { audio_manager_->GetTaskRunner()->PostTask( FROM_HERE, base::BindOnce(&AudioThreadContext::CreateServiceOnAudioThread, - this, std::move(request))); + this, std::move(receiver))); return; } DCHECK(!service_); service_ = std::make_unique( std::make_unique(audio_manager_), - service_quit_timeout_, false /* device_notifications_enabled */, - std::make_unique(), std::move(request)); - service_->set_termination_closure(base::BindOnce( - &AudioThreadContext::QuitOnAudioThread, base::Unretained(this))); + false /* device_notifications_enabled */, std::move(receiver)); } void QuitOnAudioThread() { @@ -64,21 +52,19 @@ class ServiceTestHelper : public service_manager::Service { virtual ~AudioThreadContext() = default; media::AudioManager* const audio_manager_; - const base::TimeDelta service_quit_timeout_; std::unique_ptr service_; DISALLOW_COPY_AND_ASSIGN(AudioThreadContext); }; - ServiceTestHelper(media::AudioManager* audio_manager, - base::TimeDelta service_quit_timeout, - service_manager::mojom::ServiceRequest request) - : service_binding_(this, std::move(request)), - audio_manager_(audio_manager), - audio_thread_context_( - new AudioThreadContext(audio_manager, service_quit_timeout)) {} + explicit ServiceTestHelper(media::AudioManager* audio_manager) + : audio_manager_(audio_manager), + audio_thread_context_(new AudioThreadContext(audio_manager)) { + audio_thread_context_->CreateServiceOnAudioThread( + service_remote_.BindNewPipeAndPassReceiver()); + } - ~ServiceTestHelper() override { + ~ServiceTestHelper() { // Ensure that the AudioThreadContext is destroyed on the correct thread by // passing our only reference into a task posted there. audio_manager_->GetTaskRunner()->PostTask( @@ -86,34 +72,16 @@ class ServiceTestHelper : public service_manager::Service { std::move(audio_thread_context_))); } - service_manager::Connector* connector() { - return service_binding_.GetConnector(); - } - - protected: - // service_manager::Service: - void CreatePackagedServiceInstance( - const std::string& service_name, - mojo::PendingReceiver receiver, - CreatePackagedServiceInstanceCallback callback) override { - if (service_name == mojom::kServiceName) { - audio_thread_context_->CreateServiceOnAudioThread(std::move(receiver)); - std::move(callback).Run(base::GetCurrentProcId()); - } else { - std::move(callback).Run(base::nullopt); - } - } + mojom::AudioService& service() { return *service_remote_.get(); } private: - service_manager::ServiceBinding service_binding_; media::AudioManager* const audio_manager_; + mojo::Remote service_remote_; scoped_refptr audio_thread_context_; DISALLOW_COPY_AND_ASSIGN(ServiceTestHelper); }; -const char kTestServiceName[] = "audio_unittests"; - // if |use_audio_thread| is true, AudioManager has a dedicated audio thread and // Audio service lives on it; otherwise audio thread is the main thread of the // test fixture, and that's where Service lives. So in the former case the @@ -123,37 +91,16 @@ const char kTestServiceName[] = "audio_unittests"; template class InProcessServiceTest : public testing::Test { public: - explicit InProcessServiceTest(base::TimeDelta service_quit_timeout) - : test_service_manager_( - {service_manager::ManifestBuilder() - .WithServiceName(kTestServiceName) - .RequireCapability(mojom::kServiceName, "info") - .RequireCapability(service_manager::mojom::kServiceName, - "service_manager:service_manager") - .PackageService( - GetManifest(service_manager::Manifest::ExecutionMode :: - kInProcessBuiltin)) - .Build()}), - audio_manager_( + InProcessServiceTest() + : audio_manager_( std::make_unique(use_audio_thread)), - helper_(std::make_unique( - &audio_manager_, - service_quit_timeout, - test_service_manager_.RegisterTestInstance(kTestServiceName))), + helper_(std::make_unique(&audio_manager_)), audio_system_(std::make_unique( - connector()->Clone())) {} - - InProcessServiceTest() - : InProcessServiceTest(base::TimeDelta() /* not timeout */) {} - - ~InProcessServiceTest() override {} + base::BindRepeating(&InProcessServiceTest::BindSystemInfo, + base::Unretained(this)))) {} + ~InProcessServiceTest() override = default; protected: - service_manager::Connector* connector() { - DCHECK(helper_); - return helper_->connector(); - } - void TearDown() override { audio_system_.reset(); @@ -171,8 +118,11 @@ class InProcessServiceTest : public testing::Test { media::AudioSystem* audio_system() { return audio_system_.get(); } private: + void BindSystemInfo(mojo::PendingReceiver receiver) { + helper_->service().BindSystemInfo(std::move(receiver)); + } + base::test::TaskEnvironment task_environment_; - service_manager::TestServiceManager test_service_manager_; media::MockAudioManager audio_manager_; std::unique_ptr helper_; std::unique_ptr audio_system_; @@ -184,8 +134,8 @@ class InProcessServiceTest : public testing::Test { class FakeSystemInfoTest : public InProcessServiceTest, public FakeSystemInfo { public: - FakeSystemInfoTest() {} - ~FakeSystemInfoTest() override {} + FakeSystemInfoTest() = default; + ~FakeSystemInfoTest() override = default; protected: MOCK_METHOD0(MethodCalled, void()); @@ -209,23 +159,6 @@ TEST_F(FakeSystemInfoTest, HasInputDevicesCalledOnGlobalBinderOverride) { FakeSystemInfo::ClearGlobalBinderForAudioService(); } -// Service lifetime tests. -class InProcessServiceLifetimeTestBase : public InProcessServiceTest { - public: - using TestBase = InProcessServiceTest; - - InProcessServiceLifetimeTestBase() - : TestBase(base::TimeDelta::FromMilliseconds(1)) {} - ~InProcessServiceLifetimeTestBase() override {} - - private: - DISALLOW_COPY_AND_ASSIGN(InProcessServiceLifetimeTestBase); -}; - -INSTANTIATE_TYPED_TEST_SUITE_P(InProcessAudioService, - ServiceLifetimeTestTemplate, - InProcessServiceLifetimeTestBase); - } // namespace audio // AudioSystem interface conformance tests. diff --git a/services/audio/test/service_lifetime_connector_test.cc b/services/audio/test/service_lifetime_connector_test.cc deleted file mode 100644 index 57005f9fc0e7bd..00000000000000 --- a/services/audio/test/service_lifetime_connector_test.cc +++ /dev/null @@ -1,277 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/bind.h" -#include "base/run_loop.h" -#include "base/test/mock_callback.h" -#include "base/test/task_environment.h" -#include "build/build_config.h" -#include "media/audio/mock_audio_manager.h" -#include "media/audio/test_audio_thread.h" -#include "mojo/public/cpp/bindings/remote.h" -#include "services/audio/in_process_audio_manager_accessor.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/service.h" -#include "services/audio/service_factory.h" -#include "services/audio/system_info.h" -#include "services/service_manager/public/cpp/binder_map.h" -#include "services/service_manager/public/cpp/test/test_connector_factory.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -using testing::Exactly; -using testing::StrictMock; - -namespace { -constexpr base::TimeDelta kQuitTimeout = base::TimeDelta::FromMilliseconds(100); -} // namespace - -namespace audio { - -// Connector-based tests for Audio service "quit with timeout" logic -class AudioServiceLifetimeConnectorTest : public testing::Test { - public: - AudioServiceLifetimeConnectorTest() {} - ~AudioServiceLifetimeConnectorTest() override {} - - void SetUp() override { - audio_manager_ = std::make_unique( - std::make_unique( - false /*not using a separate audio thread*/)); - service_ = std::make_unique( - std::make_unique(audio_manager_.get()), - kQuitTimeout, false /* device_notifications_enabled */, - std::make_unique(), - connector_factory_.RegisterInstance(mojom::kServiceName)); - service_->set_termination_closure(quit_request_.Get()); - connector_ = connector_factory_.CreateConnector(); - } - - void TearDown() override { - if (audio_manager_) - audio_manager_->Shutdown(); - } - - protected: - base::test::TaskEnvironment task_environment_{ - base::test::TaskEnvironment::TimeSource::MOCK_TIME}; - - StrictMock> quit_request_; - std::unique_ptr audio_manager_; - service_manager::TestConnectorFactory connector_factory_; - std::unique_ptr service_; - std::unique_ptr connector_; - - private: - DISALLOW_COPY_AND_ASSIGN(AudioServiceLifetimeConnectorTest); -}; - -#if defined(OS_WIN) || defined(OS_MACOSX) || \ - (defined(OS_LINUX) && !defined(OS_CHROMEOS)) -// For platforms where the standalone audio service is supported, the -// service should terminate after a default timeout if no specific timeout has -// been set. -// Disabled due to flakiness. -// TODO(crbug.com/976841): Fix the flakiness and re-enable this. -TEST_F(AudioServiceLifetimeConnectorTest, - DISABLED_StandaloneServiceTerminatesWhenNoTimeoutIsSet) { - service_.reset(); - audio_manager_->Shutdown(); - audio_manager_.reset(); - service_manager::TestConnectorFactory connector_factory; - service_ = audio::CreateStandaloneService( - std::make_unique(), - connector_factory.RegisterInstance(mojom::kServiceName)); - service_->set_termination_closure(quit_request_.Get()); - connector_ = connector_factory.CreateConnector(); - task_environment_.RunUntilIdle(); - - mojo::Remote info; - connector_->Connect(mojom::kServiceName, info.BindNewPipeAndPassReceiver()); - - // Make sure |info| is connected. - base::RunLoop loop; - info->HasOutputDevices( - base::BindOnce([](base::OnceClosure cl, bool) { std::move(cl).Run(); }, - loop.QuitClosure())); - loop.Run(); - - const base::TimeDelta default_timeout = base::TimeDelta::FromMinutes(15); - { - // Make sure the service does not disconnect before a timeout. - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - info.reset(); - task_environment_.FastForwardBy(default_timeout / 2); - } - - // Now wait for what is left from of the timeout: the service should - // disconnect. - EXPECT_CALL(quit_request_, Run()).Times(Exactly(1)); - task_environment_.FastForwardBy(default_timeout / 2); - - service_.reset(); -} -#else -// For platforms where the standalone audio service has not been launched, the -// service should never terminate if no specific timeout has been set. -TEST_F(AudioServiceLifetimeConnectorTest, - StandaloneServiceNeverTerminatesWhenNoTimeoutIsSet) { - service_.reset(); - audio_manager_->Shutdown(); - audio_manager_.reset(); - service_manager::TestConnectorFactory connector_factory; - service_ = audio::CreateStandaloneService( - std::make_unique(), - connector_factory.RegisterInstance(mojom::kServiceName)); - service_->set_termination_closure(quit_request_.Get()); - connector_ = connector_factory.CreateConnector(); - task_environment_.RunUntilIdle(); - - mojo::Remote info; - connector_->Connect(mojom::kServiceName, info.BindNewPipeAndPassReceiver()); - - // Make sure |info| is connected. - base::RunLoop loop; - info->HasOutputDevices( - base::BindOnce([](base::OnceClosure cl, bool) { std::move(cl).Run(); }, - loop.QuitClosure())); - loop.Run(); - - info.reset(); - - task_environment_.RunUntilIdle(); - - service_.reset(); -} -#endif - -TEST_F(AudioServiceLifetimeConnectorTest, - EmbeddedServiceNeverTerminatesWhenNoTimeoutIsSet) { - service_.reset(); - service_manager::TestConnectorFactory connector_factory; - service_ = audio::CreateEmbeddedService( - audio_manager_.get(), - connector_factory.RegisterInstance(mojom::kServiceName)); - service_->set_termination_closure(quit_request_.Get()); - connector_ = connector_factory.CreateConnector(); - task_environment_.RunUntilIdle(); - - mojo::Remote info; - connector_->Connect(mojom::kServiceName, info.BindNewPipeAndPassReceiver()); - - // Make sure |info| is connected. - base::RunLoop loop; - info->HasOutputDevices( - base::BindOnce([](base::OnceClosure cl, bool) { std::move(cl).Run(); }, - loop.QuitClosure())); - loop.Run(); - - info.reset(); - - task_environment_.RunUntilIdle(); - - service_.reset(); -} - -TEST_F(AudioServiceLifetimeConnectorTest, ServiceNotQuitWhenClientConnected) { - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - - mojo::Remote info; - connector_->Connect(mojom::kServiceName, info.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info.is_bound()); - - task_environment_.FastForwardBy(kQuitTimeout * 2); - EXPECT_TRUE(info.is_bound()); -} - -TEST_F(AudioServiceLifetimeConnectorTest, - ServiceQuitAfterTimeoutWhenClientDisconnected) { - mojo::Remote info; - connector_->Connect(mojom::kServiceName, info.BindNewPipeAndPassReceiver()); - - { - // Make sure the service does not disconnect before a timeout. - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - info.reset(); - task_environment_.FastForwardBy(kQuitTimeout / 2); - } - // Now wait for what is left from of the timeout: the service should - // disconnect. - EXPECT_CALL(quit_request_, Run()).Times(Exactly(1)); - task_environment_.FastForwardBy(kQuitTimeout / 2); -} - -TEST_F(AudioServiceLifetimeConnectorTest, - ServiceNotQuitWhenAnotherClientQuicklyConnects) { - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - - mojo::Remote info1; - connector_->Connect(mojom::kServiceName, info1.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info1.is_bound()); - - info1.reset(); - - mojo::Remote info2; - connector_->Connect(mojom::kServiceName, info2.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info2.is_bound()); - - task_environment_.FastForwardBy(kQuitTimeout); - EXPECT_TRUE(info2.is_bound()); -} - -TEST_F(AudioServiceLifetimeConnectorTest, - ServiceNotQuitWhenOneClientRemainsConnected) { - mojo::Remote info1; - mojo::Remote info2; - { - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - - connector_->Connect(mojom::kServiceName, - info1.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info1.is_bound()); - connector_->Connect(mojom::kServiceName, - info2.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info2.is_bound()); - - task_environment_.FastForwardBy(kQuitTimeout); - EXPECT_TRUE(info1.is_bound()); - EXPECT_TRUE(info2.is_bound()); - - info1.reset(); - EXPECT_TRUE(info2.is_bound()); - - task_environment_.FastForwardBy(kQuitTimeout); - EXPECT_FALSE(info1.is_bound()); - EXPECT_TRUE(info2.is_bound()); - } - // Now disconnect the last client and wait for service quit request. - EXPECT_CALL(quit_request_, Run()).Times(Exactly(1)); - info2.reset(); - task_environment_.FastForwardBy(kQuitTimeout); -} - -TEST_F(AudioServiceLifetimeConnectorTest, - QuitTimeoutIsNotShortenedAfterDelayedReconnect) { - mojo::Remote info1; - mojo::Remote info2; - { - EXPECT_CALL(quit_request_, Run()).Times(Exactly(0)); - - connector_->Connect(mojom::kServiceName, - info1.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info1.is_bound()); - info1.reset(); - task_environment_.FastForwardBy(kQuitTimeout * 0.75); - - connector_->Connect(mojom::kServiceName, - info2.BindNewPipeAndPassReceiver()); - EXPECT_TRUE(info2.is_bound()); - info2.reset(); - task_environment_.FastForwardBy(kQuitTimeout * 0.75); - } - EXPECT_CALL(quit_request_, Run()).Times(Exactly(1)); - task_environment_.FastForwardBy(kQuitTimeout * 0.25); -} - -} // namespace audio diff --git a/services/audio/test/service_lifetime_test_template.h b/services/audio/test/service_lifetime_test_template.h deleted file mode 100644 index 43d989853b5c4c..00000000000000 --- a/services/audio/test/service_lifetime_test_template.h +++ /dev/null @@ -1,152 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SERVICES_AUDIO_TEST_SERVICE_LIFETIME_TEST_TEMPLATE_H_ -#define SERVICES_AUDIO_TEST_SERVICE_LIFETIME_TEST_TEMPLATE_H_ - -#include "base/run_loop.h" -#include "mojo/public/cpp/bindings/remote.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/public/mojom/system_info.mojom.h" -#include "services/audio/test/service_observer_mock.h" - -namespace audio { - -// Template for Audio service lifetime tests regarding client -// connect/disconnect events. Audio service under test must be configured to -// quit after a timeout if there are no incoming connections. -template -class ServiceLifetimeTestTemplate : public TestBase { - public: - ServiceLifetimeTestTemplate() {} - - ~ServiceLifetimeTestTemplate() override {} - - protected: - void SetUp() override { - TestBase::SetUp(); - - mojo::Remote service_manager; - TestBase::connector()->Connect( - service_manager::mojom::kServiceName, - service_manager.BindNewPipeAndPassReceiver()); - - mojo::PendingRemote - listener; - service_observer_ = std::make_unique( - mojom::kServiceName, listener.InitWithNewPipeAndPassReceiver()); - - base::RunLoop wait_loop; - EXPECT_CALL(*service_observer_, Initialized()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - service_manager->AddListener(std::move(listener)); - wait_loop.Run(); - } - - protected: - std::unique_ptr service_observer_; - - private: - DISALLOW_COPY_AND_ASSIGN(ServiceLifetimeTestTemplate); -}; - -TYPED_TEST_SUITE_P(ServiceLifetimeTestTemplate); - -TYPED_TEST_P(ServiceLifetimeTestTemplate, - DISABLED_ServiceQuitsWhenClientDisconnects) { - mojo::Remote info; - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStarted()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - this->connector()->Connect(mojom::kServiceName, - info.BindNewPipeAndPassReceiver()); - wait_loop.Run(); - } - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStopped()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - info.reset(); - wait_loop.Run(); - } -} - -TYPED_TEST_P(ServiceLifetimeTestTemplate, - DISABLED_ServiceQuitsWhenLastClientDisconnects) { - mojo::Remote info; - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStarted()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - this->connector()->Connect(mojom::kServiceName, - info.BindNewPipeAndPassReceiver()); - wait_loop.Run(); - } - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStopped()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - EXPECT_CALL(*this->service_observer_, ServiceStarted()) - .Times(testing::Exactly(0)); - - mojo::Remote info2; - this->connector()->Connect(mojom::kServiceName, - info2.BindNewPipeAndPassReceiver()); - info2.FlushForTesting(); - - mojo::Remote info3; - this->connector()->Connect(mojom::kServiceName, - info3.BindNewPipeAndPassReceiver()); - info3.FlushForTesting(); - - info.reset(); - info2.reset(); - info3.reset(); - wait_loop.Run(); - } -} - -TYPED_TEST_P(ServiceLifetimeTestTemplate, - DISABLED_ServiceRestartsWhenClientReconnects) { - mojo::Remote info; - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStarted()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - this->connector()->Connect(mojom::kServiceName, - info.BindNewPipeAndPassReceiver()); - wait_loop.Run(); - } - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStopped()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - info.reset(); - wait_loop.Run(); - } - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStarted()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - this->connector()->Connect(mojom::kServiceName, - info.BindNewPipeAndPassReceiver()); - wait_loop.Run(); - } - { - base::RunLoop wait_loop; - EXPECT_CALL(*this->service_observer_, ServiceStopped()) - .WillOnce(testing::Invoke(&wait_loop, &base::RunLoop::Quit)); - info.reset(); - wait_loop.Run(); - } -} - -REGISTER_TYPED_TEST_SUITE_P(ServiceLifetimeTestTemplate, - DISABLED_ServiceQuitsWhenClientDisconnects, - DISABLED_ServiceQuitsWhenLastClientDisconnects, - DISABLED_ServiceRestartsWhenClientReconnects); -} // namespace audio - -#endif // SERVICES_AUDIO_TEST_SERVICE_LIFETIME_TEST_TEMPLATE_H_ diff --git a/services/audio/test/service_observer_mock.cc b/services/audio/test/service_observer_mock.cc deleted file mode 100644 index 7098f5c592d5fa..00000000000000 --- a/services/audio/test/service_observer_mock.cc +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/audio/test/service_observer_mock.h" - -namespace audio { - -ServiceObserverMock::ServiceObserverMock( - const std::string& service_name, - mojo::PendingReceiver - receiver) - : service_name_(service_name), receiver_(this, std::move(receiver)) {} - -ServiceObserverMock::~ServiceObserverMock() = default; - -void ServiceObserverMock::OnInit( - std::vector instances) { - Initialized(); -} - -void ServiceObserverMock::OnServiceStarted( - const service_manager::Identity& identity, - uint32_t pid) { - if (identity.name() == service_name_) - ServiceStarted(); -} - -void ServiceObserverMock::OnServiceStopped( - const service_manager::Identity& identity) { - if (identity.name() == service_name_) - ServiceStopped(); -} - -} // namespace audio diff --git a/services/audio/test/service_observer_mock.h b/services/audio/test/service_observer_mock.h deleted file mode 100644 index a0f8657fdf1342..00000000000000 --- a/services/audio/test/service_observer_mock.h +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SERVICES_AUDIO_TEST_SERVICE_OBSERVER_MOCK_H_ -#define SERVICES_AUDIO_TEST_SERVICE_OBSERVER_MOCK_H_ - -#include - -#include "mojo/public/cpp/bindings/pending_receiver.h" -#include "mojo/public/cpp/bindings/receiver.h" -#include "services/service_manager/public/mojom/service_manager.mojom.h" -#include "testing/gmock/include/gmock/gmock.h" - -namespace audio { - -// Mock listener tracking lifetime events for a specified service. -class ServiceObserverMock - : public service_manager::mojom::ServiceManagerListener { - public: - ServiceObserverMock( - const std::string& service_name, - mojo::PendingReceiver - receiver); - ~ServiceObserverMock() override; - - MOCK_METHOD0(Initialized, void(void)); - MOCK_METHOD0(ServiceStarted, void(void)); - MOCK_METHOD0(ServiceStopped, void(void)); - - // mojom::ServiceManagerListener implementation. - void OnInit(std::vector - instances) override; - void OnServiceCreated( - service_manager::mojom::RunningServiceInfoPtr instance) override {} - void OnServiceStarted(const service_manager::Identity& identity, - uint32_t pid) override; - void OnServiceFailedToStart( - const service_manager::Identity& identity) override {} - void OnServicePIDReceived(const service_manager::Identity& identity, - uint32_t pid) override {} - void OnServiceStopped(const service_manager::Identity& identity) override; - - private: - const std::string service_name_; - mojo::Receiver receiver_; -}; - -} // namespace audio - -#endif // SERVICES_AUDIO_TEST_SERVICE_OBSERVER_MOCK_H_ diff --git a/services/audio/test/standalone_service_test.cc b/services/audio/test/standalone_service_test.cc deleted file mode 100644 index 43d70b8b042805..00000000000000 --- a/services/audio/test/standalone_service_test.cc +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/command_line.h" -#include "base/strings/string_number_conversions.h" -#include "base/test/task_environment.h" -#include "media/base/media_switches.h" -#include "services/audio/public/cpp/manifest.h" -#include "services/audio/public/mojom/constants.mojom.h" -#include "services/audio/service.h" -#include "services/audio/test/service_lifetime_test_template.h" -#include "services/service_manager/public/cpp/manifest_builder.h" -#include "services/service_manager/public/cpp/test/test_service.h" -#include "services/service_manager/public/cpp/test/test_service_manager.h" -#include "services/service_manager/public/mojom/constants.mojom.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace audio { - -const char kTestServiceName[] = "audio_unittests"; - -service_manager::Manifest MakeAudioManifestForUnsandboxedExecutable() { - service_manager::Manifest manifest(GetManifest( - service_manager::Manifest::ExecutionMode::kStandaloneExecutable)); - manifest.options.sandbox_type = "none"; - return manifest; -} - -class StandaloneAudioServiceTest : public testing::Test { - public: - StandaloneAudioServiceTest() - : test_service_manager_( - {MakeAudioManifestForUnsandboxedExecutable(), - service_manager::ManifestBuilder() - .WithServiceName(kTestServiceName) - .RequireCapability(mojom::kServiceName, "info") - .RequireCapability(service_manager::mojom::kServiceName, - "service_manager:service_manager") - .Build()}), - test_service_( - test_service_manager_.RegisterTestInstance("audio_unittests")) {} - - protected: - service_manager::Connector* connector() { return test_service_.connector(); } - - void SetUp() override { - base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess(); - cmd_line->AppendSwitchASCII(switches::kAudioServiceQuitTimeoutMs, - base::NumberToString(10)); - } - - private: - base::test::TaskEnvironment task_environment_; - service_manager::TestServiceManager test_service_manager_; - service_manager::TestService test_service_; - - DISALLOW_COPY_AND_ASSIGN(StandaloneAudioServiceTest); -}; - -INSTANTIATE_TYPED_TEST_SUITE_P(StandaloneAudioService, - ServiceLifetimeTestTemplate, - StandaloneAudioServiceTest); -} // namespace audio diff --git a/services/audio/testing_api_binder.cc b/services/audio/testing_api_binder.cc new file mode 100644 index 00000000000000..91eaa91b4d4bec --- /dev/null +++ b/services/audio/testing_api_binder.cc @@ -0,0 +1,16 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "services/audio/testing_api_binder.h" + +#include "base/no_destructor.h" + +namespace audio { + +TestingApiBinder& GetTestingApiBinder() { + static base::NoDestructor binder; + return *binder; +} + +} // namespace audio diff --git a/services/audio/testing_api_binder.h b/services/audio/testing_api_binder.h new file mode 100644 index 00000000000000..520996739af4ea --- /dev/null +++ b/services/audio/testing_api_binder.h @@ -0,0 +1,24 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SERVICES_AUDIO_TESTING_API_BINDER_H_ +#define SERVICES_AUDIO_TESTING_API_BINDER_H_ + +#include "base/callback.h" +#include "base/component_export.h" +#include "services/audio/public/mojom/testing_api.mojom.h" + +namespace audio { + +// Exposes access to global storage for a callback that can bind a TestingApi +// receiver. Test environments can use this to inject an implementation of +// TestingApi, and the service will use it if available. +using TestingApiBinder = + base::RepeatingCallback)>; +COMPONENT_EXPORT(AUDIO_SERVICE_TESTING_API_SUPPORT) +TestingApiBinder& GetTestingApiBinder(); + +} // namespace audio + +#endif // SERVICES_AUDIO_TESTING_API_BINDER_H_ diff --git a/services/audio/traced_service_ref.cc b/services/audio/traced_service_ref.cc deleted file mode 100644 index 09a0df3b826f9e..00000000000000 --- a/services/audio/traced_service_ref.cc +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/audio/traced_service_ref.h" - -#include - -#include "base/trace_event/trace_event.h" -#include "services/service_manager/public/cpp/service_keepalive.h" - -namespace audio { - -TracedServiceRef::TracedServiceRef() : trace_name_("") {} - -TracedServiceRef::TracedServiceRef( - std::unique_ptr keepalive_ref, - const char* trace_name) - : keepalive_ref_(std::move(keepalive_ref)), trace_name_(trace_name) { - if (keepalive_ref_) - TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("audio", trace_name_, - keepalive_ref_.get()); -} - -TracedServiceRef::TracedServiceRef(TracedServiceRef&& other) { - std::swap(keepalive_ref_, other.keepalive_ref_); - std::swap(trace_name_, other.trace_name_); -} - -TracedServiceRef& TracedServiceRef::operator=(TracedServiceRef&& other) { - std::swap(keepalive_ref_, other.keepalive_ref_); - std::swap(trace_name_, other.trace_name_); - return *this; -} - -TracedServiceRef::~TracedServiceRef() { - if (keepalive_ref_) - TRACE_EVENT_NESTABLE_ASYNC_END0("audio", trace_name_, keepalive_ref_.get()); -} - -} // namespace audio diff --git a/services/audio/traced_service_ref.h b/services/audio/traced_service_ref.h deleted file mode 100644 index 98b691ed7d2300..00000000000000 --- a/services/audio/traced_service_ref.h +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SERVICES_AUDIO_TRACED_SERVICE_REF_H_ -#define SERVICES_AUDIO_TRACED_SERVICE_REF_H_ - -#include - -#include "base/macros.h" - -namespace service_manager { -class ServiceKeepaliveRef; -} - -namespace audio { - -// Wraps a service_manager::ServiceKeepaliveRef, and provides tracing -// information. -class TracedServiceRef final { - public: - TracedServiceRef(); - // |trace_name| must have static lifetime since it is used in trace macros. - // By convention, "audio::InterfaceName Binding" is used, e.g. - // "audio::StreamFactory Binding". - TracedServiceRef( - std::unique_ptr keepalive_ref, - const char* trace_name); - TracedServiceRef(TracedServiceRef&& other); - TracedServiceRef& operator=(TracedServiceRef&& other); - - ~TracedServiceRef(); - - // This id can be used to add TRACE_EVENT_NESTABLE_ASYNC traces to the scope - // of the TracedServiceRef. - const void* id_for_trace() const { return keepalive_ref_.get(); } - - private: - std::unique_ptr keepalive_ref_; - const char* trace_name_; - - DISALLOW_COPY_AND_ASSIGN(TracedServiceRef); -}; - -} // namespace audio - -#endif // SERVICES_AUDIO_TRACED_SERVICE_REF_H_