diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 9834c9f863cfd2..68c6fe29d4c17a 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -347,6 +347,7 @@ #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" @@ -2224,6 +2225,17 @@ 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 db350881f241f6..d6b48bfa43147c 100644 --- a/chrome/browser/chrome_content_browser_client.h +++ b/chrome/browser/chrome_content_browser_client.h @@ -206,6 +206,9 @@ 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 bc15c8f200b511..f7070fb175ac22 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc @@ -176,7 +176,6 @@ #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" @@ -651,7 +650,7 @@ void ChromeBrowserMainPartsChromeos::PreProfileInit() { new default_app_order::ExternalLoader(true /* async */)); } - audio::SoundsManager::Create(content::GetAudioServiceStreamFactoryBinder()); + audio::SoundsManager::Create(content::GetSystemConnector()->Clone()); // |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 78b91589d76442..7eb077835ea769 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,6 +98,7 @@ #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" @@ -2372,7 +2373,7 @@ class KioskVirtualKeyboardTestSoundsManagerTestImpl } auto handler = std::make_unique( - content::GetAudioServiceStreamFactoryBinder(), iter->second); + content::GetSystemConnector()->Clone(), 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 d02c6f6e87eb56..f942b4d18e8691 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/audio_service.h" +#include "content/public/browser/system_connector.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::GetAudioServiceStreamFactoryBinder()); + audio::SoundsManager::Create(content::GetSystemConnector()->Clone()); 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 8606e917ff72fc..060590f6dcc3bf 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,12 +25,14 @@ #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" @@ -170,8 +172,10 @@ ChromeVirtualKeyboardDelegate::~ChromeVirtualKeyboardDelegate() {} void ChromeVirtualKeyboardDelegate::GetKeyboardConfig( OnKeyboardSettingsCallback on_settings_callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (!audio_system_) - audio_system_ = content::CreateAudioSystemForAudioService(); + if (!audio_system_) { + audio_system_ = + audio::CreateAudioSystem(content::GetSystemConnector()->Clone()); + } 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 9e331b4fdb905e..9d920b10a5c4ba 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,16 +16,18 @@ #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" @@ -132,8 +134,10 @@ std::string WebrtcAudioPrivateFunction::device_id_salt() const { media::AudioSystem* WebrtcAudioPrivateFunction::GetAudioSystem() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (!audio_system_) - audio_system_ = content::CreateAudioSystemForAudioService(); + if (!audio_system_) { + audio_system_ = + audio::CreateAudioSystem(content::GetSystemConnector()->Clone()); + } 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 2a399b87e64425..500733126428b2 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,6 +44,8 @@ #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) @@ -70,7 +72,7 @@ void GetAudioDeviceDescriptions(bool for_input, AudioDeviceDescriptions* device_descriptions) { base::RunLoop run_loop; std::unique_ptr audio_system = - content::CreateAudioSystemForAudioService(); + audio::CreateAudioSystem(content::GetSystemConnector()->Clone()); 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 087351469e4d2a..44ee73b6ae7403 100644 --- a/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc +++ b/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc @@ -14,13 +14,14 @@ #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; @@ -111,11 +112,8 @@ 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, std::move(debug_recording)); + prefix_path, content::GetSystemConnector()->Clone()); 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 f544b6ca4624c6..57018b6c9ef1fa 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,7 +183,8 @@ void AssistantClient::RequestWakeLockProvider( void AssistantClient::RequestAudioStreamFactory( mojo::PendingReceiver receiver) { - content::GetAudioService().BindStreamFactory(std::move(receiver)); + content::GetSystemConnector()->Connect(audio::mojom::kServiceName, + 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 758b1de63074a1..80781150162b2a 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/audio_service.h" +#include "content/public/browser/system_connector.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::GetAudioServiceStreamFactoryBinder()); + audio::SoundsManager::Create(content::GetSystemConnector()->Clone()); 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 bee65920076447..a9bed5f362942d 100644 --- a/chromeos/services/assistant/assistant_state_proxy.cc +++ b/chromeos/services/assistant/assistant_state_proxy.cc @@ -10,6 +10,7 @@ #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 98ce1f30d300df..d6a5161173cb4d 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn @@ -143,8 +143,9 @@ jumbo_source_set("browser") { "//net/server:http_server", "//ppapi/buildflags", "//printing/buildflags", - "//services/audio", + "//services/audio:lib", "//services/audio/public/cpp", + "//services/audio/public/cpp:manifest", "//services/audio/public/mojom", "//services/content:impl", "//services/content/public/cpp", @@ -433,7 +434,6 @@ 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 deleted file mode 100644 index 8c158a5248de79..00000000000000 --- a/content/browser/audio/OWNERS +++ /dev/null @@ -1,2 +0,0 @@ -file://services/audio/OWNERS - diff --git a/content/browser/audio/audio_service.cc b/content/browser/audio/audio_service.cc deleted file mode 100644 index 940e420e7c70b2..00000000000000 --- a/content/browser/audio/audio_service.cc +++ /dev/null @@ -1,176 +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 "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 a23f65b9aee04c..9da6475030a796 100644 --- a/content/browser/browser_main_loop.cc +++ b/content/browser/browser_main_loop.cc @@ -99,7 +99,6 @@ #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" @@ -134,6 +133,8 @@ #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" @@ -1598,11 +1599,17 @@ void BrowserMainLoop::InitializeAudio() { {content::BrowserThread::UI, base::TaskPriority::BEST_EFFORT}, base::BindOnce([]() { TRACE_EVENT0("audio", "Starting audio service"); - GetAudioService(); + 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)); + } })); } - audio_system_ = CreateAudioSystemForAudioService(); + audio_system_ = audio::CreateAudioSystem(GetSystemConnector()->Clone()); CHECK(audio_system_); } diff --git a/content/browser/builtin_service_manifests.cc b/content/browser/builtin_service_manifests.cc index 5e026e3516aac2..a924aa79b47fa0 100644 --- a/content/browser/builtin_service_manifests.cc +++ b/content/browser/builtin_service_manifests.cc @@ -15,6 +15,7 @@ #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/metrics/public/cpp/manifest.h" @@ -22,11 +23,26 @@ 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 1ba3ccd16e4df7..de0bec2594d713 100644 --- a/content/browser/media/forwarding_audio_stream_factory.cc +++ b/content/browser/media/forwarding_audio_stream_factory.cc @@ -9,12 +9,10 @@ #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" @@ -22,44 +20,26 @@ #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()) { + group_id_(base::UnguessableToken::Create()), + connector_(std::move(connector)) { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(owner_); DCHECK(broker_factory_); + DCHECK(connector_); } ForwardingAudioStreamFactory::Core::~Core() { @@ -226,11 +206,13 @@ 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(broker_factory)); + core_ = + std::make_unique(weak_ptr_factory_.GetWeakPtr(), user_input_monitor, + std::move(connector), std::move(broker_factory)); } ForwardingAudioStreamFactory::~ForwardingAudioStreamFactory() { @@ -286,11 +268,6 @@ 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) { @@ -340,10 +317,8 @@ audio::mojom::StreamFactory* ForwardingAudioStreamFactory::Core::GetFactory() { TRACE_EVENT_INSTANT1( "audio", "ForwardingAudioStreamFactory: Binding new factory", TRACE_EVENT_SCOPE_THREAD, "group", group_id_.GetLowForSerialization()); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, - base::BindOnce(&BindStreamFactoryFromUIThread, - remote_factory_.BindNewPipeAndPassReceiver())); + connector_->Connect(audio::mojom::kServiceName, + 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 fe464f56e62e8d..21fe74694e42e1 100644 --- a/content/browser/media/forwarding_audio_stream_factory.h +++ b/content/browser/media/forwarding_audio_stream_factory.h @@ -26,6 +26,10 @@ #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; @@ -58,11 +62,17 @@ 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 @@ -139,6 +149,8 @@ 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 @@ -178,9 +190,11 @@ 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; @@ -206,12 +220,6 @@ 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 9b16ba5598fec4..88a6726c71334f 100644 --- a/content/browser/media/forwarding_audio_stream_factory_unittest.cc +++ b/content/browser/media/forwarding_audio_stream_factory_unittest.cc @@ -23,7 +23,9 @@ #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" @@ -185,16 +187,16 @@ class MockLoopbackSink : public AudioStreamBroker::LoopbackSink { class ForwardingAudioStreamFactoryTest : public RenderViewHostTestHarness { public: ForwardingAudioStreamFactoryTest() - : broker_factory_(std::make_unique()) { - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + : 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_, base::BindRepeating(&ForwardingAudioStreamFactoryTest::BindFactory, base::Unretained(this))); } - ~ForwardingAudioStreamFactoryTest() override { - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( - base::NullCallback()); - } + ~ForwardingAudioStreamFactoryTest() override {} void SetUp() override { RenderViewHostTestHarness::SetUp(); @@ -203,9 +205,10 @@ class ForwardingAudioStreamFactoryTest : public RenderViewHostTestHarness { RenderFrameHostTester::For(main_rfh())->AppendChild("other_rfh"); } - void BindFactory( - mojo::PendingReceiver receiver) { - stream_factory_.receiver_.Bind(std::move(receiver)); + void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { + stream_factory_.receiver_.Bind( + mojo::PendingReceiver( + std::move(factory_receiver))); stream_factory_.receiver_.set_disconnect_handler( base::BindRepeating(&audio::FakeStreamFactory::ResetReceiver, base::Unretained(&stream_factory_))); @@ -243,6 +246,8 @@ 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_; }; @@ -263,9 +268,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(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -282,13 +287,16 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr broker = ExpectLoopbackBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + std::unique_ptr other_connector = + connector_->Clone(); + + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::make_unique()); + std::move(other_connector), std::make_unique()); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -303,9 +311,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, mojo::PendingRemote client; base::WeakPtr broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -321,9 +329,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectInputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -360,13 +368,16 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectLoopbackBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + std::unique_ptr other_connector = + connector_->Clone(); + + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::make_unique()); + std::move(other_connector), std::make_unique()); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -402,9 +413,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, base::WeakPtr other_rfh_broker = ExpectOutputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_broker, CreateStream(NotNull())); @@ -449,13 +460,16 @@ TEST_F(ForwardingAudioStreamFactoryTest, DestroyFrame_DestroysRelatedStreams) { base::WeakPtr other_rfh_output_broker = ExpectOutputBrokerConstruction(other_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + std::unique_ptr other_connector = + connector_->Clone(); + + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); ForwardingAudioStreamFactory source_factory( source_contents.get(), nullptr /*user_input_monitor*/, - std::make_unique()); + std::move(other_connector), std::make_unique()); { EXPECT_CALL(*main_rfh_input_broker, CreateStream(NotNull())); @@ -549,9 +563,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, DestroyWebContents_DestroysStreams) { base::WeakPtr output_broker = ExpectOutputBrokerConstruction(main_rfh()); - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_CALL(*input_broker, CreateStream(NotNull())); ignore_result(input_client.InitWithNewPipeAndPassReceiver()); @@ -586,9 +600,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(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); { EXPECT_CALL(*main_rfh_input_broker, CreateStream(NotNull())); @@ -657,9 +671,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, LastStreamDeleted_ClearsFactoryPtr) { TEST_F(ForwardingAudioStreamFactoryTest, MuteNoOutputStreams_DoesNotConnectMuter) { - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_FALSE(factory.IsMuted()); factory.SetMuted(true); @@ -678,9 +692,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(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_CALL(*broker, CreateStream(NotNull())); ignore_result(client.InitWithNewPipeAndPassReceiver()); @@ -710,9 +724,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(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); EXPECT_FALSE(stream_factory_.IsConnected()); EXPECT_FALSE(factory.IsMuted()); @@ -747,9 +761,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(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + std::move(broker_factory_)); { EXPECT_CALL(*broker, CreateStream(NotNull())); @@ -803,9 +817,9 @@ TEST_F(ForwardingAudioStreamFactoryTest, // called. EXPECT_CALL(sink2, OnSourceGone()); { - ForwardingAudioStreamFactory factory(web_contents(), - nullptr /*user_input_monitor*/, - std::move(broker_factory_)); + ForwardingAudioStreamFactory factory( + web_contents(), nullptr /*user_input_monitor*/, std::move(connector_), + 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 a941bb86c5fc44..a56fc100061653 100644 --- a/content/browser/media/in_process_audio_loopback_stream_creator.cc +++ b/content/browser/media/in_process_audio_loopback_stream_creator.cc @@ -17,10 +17,12 @@ #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 { @@ -96,6 +98,7 @@ 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 4e814a06ad7a78..8daab102edda5b 100644 --- a/content/browser/renderer_host/media/audio_service_listener.cc +++ b/content/browser/renderer_host/media/audio_service_listener.cc @@ -10,12 +10,11 @@ #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/audio_service.mojom.h" +#include "services/audio/public/mojom/constants.mojom.h" #include "services/audio/public/mojom/log_factory_manager.mojom.h" namespace content { @@ -25,10 +24,12 @@ AudioServiceListener::Metrics::Metrics(const base::TickClock* clock) AudioServiceListener::Metrics::~Metrics() = default; -void AudioServiceListener::Metrics::ServiceAlreadyRunning() { +void AudioServiceListener::Metrics::ServiceAlreadyRunning( + service_manager::mojom::InstanceState state) { LogServiceStartStatus(ServiceStartStatus::kAlreadyStarted); initial_downtime_start_ = base::TimeTicks(); - started_ = clock_->NowTicks(); + if (state == service_manager::mojom::InstanceState::kStarted) + started_ = clock_->NowTicks(); } void AudioServiceListener::Metrics::ServiceCreated() { @@ -64,6 +65,11 @@ void AudioServiceListener::Metrics::ServiceStarted() { } } +void AudioServiceListener::Metrics::ServiceFailedToStart() { + LogServiceStartStatus(ServiceStartStatus::kFailure); + created_ = base::TimeTicks(); +} + void AudioServiceListener::Metrics::ServiceStopped() { stopped_ = clock_->NowTicks(); @@ -81,15 +87,26 @@ void AudioServiceListener::Metrics::LogServiceStartStatus( UMA_HISTOGRAM_ENUMERATION("Media.AudioService.ObservedStartStatus", status); } -AudioServiceListener::AudioServiceListener() - : metrics_(base::DefaultTickClock::GetInstance()) { - ServiceProcessHost::AddObserver(this); - Init(ServiceProcessHost::GetRunningProcessInfo()); +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() { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - ServiceProcessHost::RemoveObserver(this); } base::ProcessId AudioServiceListener::GetProcessId() const { @@ -97,55 +114,110 @@ base::ProcessId AudioServiceListener::GetProcessId() const { return process_id_; } -void AudioServiceListener::Init( - std::vector running_service_processes) { - for (const auto& info : running_service_processes) { - if (info.IsService()) { - process_id_ = info.pid; - metrics_.ServiceAlreadyRunning(); +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); 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::OnServiceProcessLaunched( - const ServiceProcessInfo& info) { +void AudioServiceListener::OnServiceCreated( + service_manager::mojom::RunningServiceInfoPtr service) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (!info.IsService()) + if (service->identity.name() != audio::mojom::kServiceName) return; - process_id_ = info.pid; + 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; 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::OnServiceProcessTerminatedNormally( - const ServiceProcessInfo& info) { +void AudioServiceListener::OnServicePIDReceived( + const ::service_manager::Identity& identity, + uint32_t pid) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (!info.IsService()) + if (identity != current_instance_identity_) return; - metrics_.ServiceStopped(); + process_id_ = pid; +} + +void AudioServiceListener::OnServiceFailedToStart( + const ::service_manager::Identity& identity) { + DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); + if (identity != current_instance_identity_) + return; + + metrics_.ServiceFailedToStart(); + current_instance_identity_.reset(); + current_instance_state_.reset(); process_id_ = base::kNullProcessId; log_factory_is_set_ = false; } -void AudioServiceListener::OnServiceProcessCrashed( - const ServiceProcessInfo& info) { +void AudioServiceListener::OnServiceStopped( + const ::service_manager::Identity& identity) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - if (!info.IsService()) + if (identity != current_instance_identity_) 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) || - log_factory_is_set_) + !connector_ || log_factory_is_set_) return; mojo::PendingRemote audio_log_factory; @@ -153,8 +225,8 @@ void AudioServiceListener::MaybeSetLogFactory() { std::make_unique(), audio_log_factory.InitWithNewPipeAndPassReceiver()); mojo::Remote log_factory_manager; - GetAudioService().BindLogFactoryManager( - log_factory_manager.BindNewPipeAndPassReceiver()); + connector_->Connect(*current_instance_identity_, + 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 3c7140c98b805f..fa5a88910c8fc8 100644 --- a/content/browser/renderer_host/media/audio_service_listener.h +++ b/content/browser/renderer_host/media/audio_service_listener.h @@ -13,9 +13,10 @@ #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; @@ -25,7 +26,7 @@ namespace content { // Tracks the system's active audio service instance, if any exists. class CONTENT_EXPORT AudioServiceListener - : public ServiceProcessHost::Observer { + : public service_manager::mojom::ServiceManagerListener { public: class CONTENT_EXPORT Metrics { public: @@ -41,8 +42,9 @@ class CONTENT_EXPORT AudioServiceListener explicit Metrics(const base::TickClock* clock); ~Metrics(); - void ServiceAlreadyRunning(); + void ServiceAlreadyRunning(service_manager::mojom::InstanceState state); void ServiceCreated(); + void ServiceFailedToStart(); void ServiceStarted(); void ServiceStopped(); @@ -57,7 +59,8 @@ class CONTENT_EXPORT AudioServiceListener DISALLOW_COPY_AND_ASSIGN(Metrics); }; - AudioServiceListener(); + explicit AudioServiceListener( + std::unique_ptr connector); ~AudioServiceListener() override; base::ProcessId GetProcessId() const; @@ -72,17 +75,26 @@ class CONTENT_EXPORT AudioServiceListener FRIEND_TEST_ALL_PREFIXES(AudioServiceListenerTest, StartService_LogStartStatus); - // 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; + // 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; 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 e69f80ceb4e011..3989d9c98584d0 100644 --- a/content/browser/renderer_host/media/audio_service_listener_unittest.cc +++ b/content/browser/renderer_host/media/audio_service_listener_unittest.cc @@ -7,45 +7,49 @@ #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 "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 "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 "testing/gtest/include/gtest/gtest.h" +using RunningServiceInfoPtr = service_manager::mojom::RunningServiceInfoPtr; + namespace content { namespace { -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; +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; } } // namespace -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}); +struct AudioServiceListenerMetricsTest : public testing::Test { + AudioServiceListenerMetricsTest() { test_clock.SetNowTicks(base::TimeTicks::Now()); } - base::test::ScopedFeatureList feature_list_; - BrowserTaskEnvironment task_environment_; base::SimpleTestTickClock test_clock; base::HistogramTester histogram_tester; }; -TEST_F(AudioServiceListenerTest, +TEST_F(AudioServiceListenerMetricsTest, ServiceCreatedStartedStopped_LogsStartupTime_LogsUptime) { AudioServiceListener::Metrics metrics(&test_clock); metrics.ServiceCreated(); @@ -66,7 +70,7 @@ TEST_F(AudioServiceListenerTest, base::TimeDelta::FromHours(5), 1); } -TEST_F(AudioServiceListenerTest, +TEST_F(AudioServiceListenerMetricsTest, CreateMetricsStartService_LogsInitialDowntime) { AudioServiceListener::Metrics metrics(&test_clock); test_clock.Advance(base::TimeDelta::FromHours(12)); @@ -77,19 +81,22 @@ TEST_F(AudioServiceListenerTest, base::TimeDelta::FromHours(12), 1); } -TEST_F(AudioServiceListenerTest, ServiceAlreadyRunningStopService_LogsUptime) { +TEST_F(AudioServiceListenerMetricsTest, + ServiceAlreadyRunningStopService_LogsUptime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning(); + metrics.ServiceAlreadyRunning( + service_manager::mojom::InstanceState::kStarted); test_clock.Advance(base::TimeDelta::FromMinutes(42)); metrics.ServiceStopped(); histogram_tester.ExpectTimeBucketCount("Media.AudioService.ObservedUptime", base::TimeDelta::FromMinutes(42), 1); } -TEST_F(AudioServiceListenerTest, +TEST_F(AudioServiceListenerMetricsTest, ServiceAlreadyRunningCreateService_LogsStartupTime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning(); + metrics.ServiceAlreadyRunning( + service_manager::mojom::InstanceState::kStarted); test_clock.Advance(base::TimeDelta::FromMilliseconds(2)); metrics.ServiceStopped(); metrics.ServiceCreated(); @@ -103,10 +110,11 @@ TEST_F(AudioServiceListenerTest, // Check that if service was already created and ServiceStarted() is called, // ObservedStartupTime and ObservedInitialDowntime are not logged and start time // is reset. -TEST_F(AudioServiceListenerTest, +TEST_F(AudioServiceListenerMetricsTest, ServiceAlreadyRunningStartService_ResetStartTime) { AudioServiceListener::Metrics metrics(&test_clock); - metrics.ServiceAlreadyRunning(); + metrics.ServiceAlreadyRunning( + service_manager::mojom::InstanceState::kCreated); test_clock.Advance(base::TimeDelta::FromMilliseconds(20)); metrics.ServiceStarted(); histogram_tester.ExpectTotalCount("Media.AudioService.ObservedStartupTime", @@ -120,33 +128,61 @@ TEST_F(AudioServiceListenerTest, 1); } -TEST_F(AudioServiceListenerTest, StartService_LogStartStatus) { +TEST(AudioServiceListenerTest, StartService_LogStartStatus) { base::HistogramTester histogram_tester; - AudioServiceListener audio_service_listener; + AudioServiceListener audio_service_listener(nullptr); + service_manager::Identity audio_service_identity = + MakeFakeId(audio::mojom::kServiceName); constexpr base::ProcessId pid(42); - ServiceProcessInfo audio_process_info = MakeFakeAudioServiceProcessInfo(pid); - audio_service_listener.Init({audio_process_info}); + + std::vector instances; + instances.push_back(MakeTestServiceInfo(audio_service_identity, pid)); + audio_service_listener.OnInit(std::move(instances)); histogram_tester.ExpectBucketCount( "Media.AudioService.ObservedStartStatus", static_cast( AudioServiceListener::Metrics::ServiceStartStatus::kAlreadyStarted), 1); - audio_service_listener.OnServiceProcessTerminatedNormally(audio_process_info); + audio_service_listener.OnServiceStopped(audio_service_identity); - audio_service_listener.OnServiceProcessLaunched(audio_process_info); + audio_service_listener.OnServiceCreated( + MakeTestServiceInfo(audio_service_identity, pid)); + audio_service_listener.OnServiceStarted(audio_service_identity, pid); histogram_tester.ExpectBucketCount( "Media.AudioService.ObservedStartStatus", static_cast( AudioServiceListener::Metrics::ServiceStartStatus::kSuccess), 1); - audio_service_listener.OnServiceProcessTerminatedNormally(audio_process_info); + 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()); } -TEST_F(AudioServiceListenerTest, OnInitWithAudioService_ProcessIdNotNull) { - AudioServiceListener audio_service_listener; +TEST(AudioServiceListenerTest, OnInitWithAudioService_ProcessIdNotNull) { + AudioServiceListener audio_service_listener(nullptr); + service_manager::Identity audio_service_identity = + MakeFakeId(audio::mojom::kServiceName); constexpr base::ProcessId pid(42); - ServiceProcessInfo audio_process_info = MakeFakeAudioServiceProcessInfo(pid); - audio_service_listener.Init({audio_process_info}); + std::vector instances; + instances.push_back(MakeTestServiceInfo(audio_service_identity, pid)); + audio_service_listener.OnInit(std::move(instances)); 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 98db846e03284e..1847e34d7e4c50 100644 --- a/content/browser/renderer_host/media/media_devices_manager.cc +++ b/content/browser/renderer_host/media/media_devices_manager.cc @@ -27,7 +27,6 @@ #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" @@ -36,7 +35,11 @@ #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" @@ -149,12 +152,6 @@ 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, @@ -317,7 +314,9 @@ MediaDevicesManager::SubscriptionRequest::operator=(SubscriptionRequest&&) = class MediaDevicesManager::AudioServiceDeviceListener : public audio::mojom::DeviceListener { public: - AudioServiceDeviceListener() { ConnectToService(); } + explicit AudioServiceDeviceListener(service_manager::Connector* connector) { + TryConnectToService(connector); + } ~AudioServiceDeviceListener() override = default; void DevicesChanged() override { @@ -327,23 +326,43 @@ class MediaDevicesManager::AudioServiceDeviceListener } private: - void ConnectToService() { + 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) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(!mojo_audio_device_notifier_); DCHECK(!receiver_.is_bound()); - base::PostTask( - FROM_HERE, {BrowserThread::UI}, - base::BindOnce( - &BindDeviceNotifierFromUIThread, - mojo_audio_device_notifier_.BindNewPipeAndPassReceiver())); + connector->Connect( + audio::mojom::kServiceName, + mojo_audio_device_notifier_.BindNewPipeAndPassReceiver()); mojo_audio_device_notifier_.set_disconnect_handler(base::BindOnce( &MediaDevicesManager::AudioServiceDeviceListener::OnConnectionError, - weak_factory_.GetWeakPtr())); + weak_factory_.GetWeakPtr(), connector)); mojo_audio_device_notifier_->RegisterListener( receiver_.BindNewPipeAndPassRemote()); } - void OnConnectionError() { + void OnConnectionError(service_manager::Connector* connector) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); mojo_audio_device_notifier_.reset(); receiver_.reset(); @@ -351,8 +370,9 @@ 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::ConnectToService, - weak_factory_.GetWeakPtr())); + FROM_HERE, + base::BindOnce(&AudioServiceDeviceListener::TryConnectToService, + weak_factory_.GetWeakPtr(), connector)); } mojo::Receiver receiver_{this}; @@ -498,8 +518,17 @@ 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(); + std::make_unique(connector_.get()); } #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 01b08fd55d28a6..883aa8a650217b 100644 --- a/content/browser/renderer_host/media/media_devices_manager.h +++ b/content/browser/renderer_host/media/media_devices_manager.h @@ -34,6 +34,10 @@ namespace media { class AudioSystem; } +namespace service_manager { +class Connector; +} + namespace content { class MediaDevicesPermissionChecker; @@ -322,6 +326,8 @@ 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 688c1880681e01..d2b7c4a48df7d0 100644 --- a/content/browser/renderer_host/media/media_stream_manager.cc +++ b/content/browser/renderer_host/media/media_stream_manager.cc @@ -52,6 +52,7 @@ #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" @@ -580,7 +581,11 @@ MediaStreamManager::MediaStreamManager( } InitializeMaybeAsync(std::move(video_capture_provider)); - audio_service_listener_ = std::make_unique(); + // May be null in tests. + if (GetSystemConnector()) { + audio_service_listener_ = + std::make_unique(GetSystemConnector()->Clone()); + } 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 230a03b230b144..3e5a16bb127743 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,7 +39,9 @@ #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" @@ -75,7 +77,13 @@ class MAYBE_RenderFrameAudioInputStreamFactoryTest RenderFrameHostTester::For(main_rfh())->InitializeRenderFrameIfNeeded(); // Set up the ForwardingAudioStreamFactory. - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + 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_, base::BindRepeating( &MAYBE_RenderFrameAudioInputStreamFactoryTest::BindFactory, base::Unretained(this))); @@ -84,16 +92,15 @@ class MAYBE_RenderFrameAudioInputStreamFactoryTest } void TearDown() override { - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( - base::NullCallback()); audio_manager_.Shutdown(); test_service_manager_context_.reset(); RenderViewHostTestHarness::TearDown(); } - void BindFactory( - mojo::PendingReceiver receiver) { - audio_service_stream_factory_.receiver_.Bind(std::move(receiver)); + void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { + audio_service_stream_factory_.receiver_.Bind( + mojo::PendingReceiver( + std::move(factory_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 e06a1a7eae2d19..61dd63ea79c249 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,7 +35,9 @@ #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" @@ -67,7 +69,13 @@ class RenderFrameAudioOutputStreamFactoryTest RenderFrameHostTester::For(main_rfh())->InitializeRenderFrameIfNeeded(); // Set up the ForwardingAudioStreamFactory. - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( + 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_, base::BindRepeating( &RenderFrameAudioOutputStreamFactoryTest::BindFactory, base::Unretained(this))); @@ -76,16 +84,15 @@ class RenderFrameAudioOutputStreamFactoryTest } void TearDown() override { - ForwardingAudioStreamFactory::OverrideStreamFactoryBinderForTesting( - base::NullCallback()); audio_manager_.Shutdown(); test_service_manager_context_.reset(); RenderViewHostTestHarness::TearDown(); } - void BindFactory( - mojo::PendingReceiver receiver) { - audio_service_stream_factory_.receiver_.Bind(std::move(receiver)); + void BindFactory(mojo::ScopedMessagePipeHandle factory_receiver) { + audio_service_stream_factory_.receiver_.Bind( + mojo::PendingReceiver( + std::move(factory_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 e88f9a7825d268..b40811d624ff79 100644 --- a/content/browser/service_manager/service_manager_context.cc +++ b/content/browser/service_manager/service_manager_context.cc @@ -47,6 +47,7 @@ #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" @@ -54,6 +55,9 @@ #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" @@ -225,6 +229,13 @@ 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)>; @@ -263,12 +274,38 @@ 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. @@ -329,6 +366,10 @@ 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 7275d26720ab3a..1d417b84d888f4 100644 --- a/content/browser/service_process_host_impl.cc +++ b/content/browser/service_process_host_impl.cc @@ -79,9 +79,7 @@ class ServiceProcessTracker { } void RemoveObserver(ServiceProcessHost::Observer* observer) { - // NOTE: Some tests may remove observers after BrowserThreads are shut down. - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) || - !BrowserThread::IsThreadInitialized(BrowserThread::UI)); + DCHECK_CURRENTLY_ON(BrowserThread::UI); observers_.RemoveObserver(observer); } diff --git a/content/browser/speech/speech_recognizer_impl.cc b/content/browser/speech/speech_recognizer_impl.cc index df50aa846eb629..5f2f2fb7d7a05f 100644 --- a/content/browser/speech/speech_recognizer_impl.cc +++ b/content/browser/speech/speech_recognizer_impl.cc @@ -16,15 +16,17 @@ #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" @@ -872,14 +874,15 @@ media::AudioSystem* SpeechRecognizerImpl::GetAudioSystem() { } void SpeechRecognizerImpl::CreateAudioCapturerSource() { - 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 */)); + 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 */)); + } } 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 3384e064b05a8a..45e8eb7166a092 100644 --- a/content/browser/speech/speech_recognizer_impl_unittest.cc +++ b/content/browser/speech/speech_recognizer_impl_unittest.cc @@ -15,14 +15,12 @@ #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" @@ -85,11 +83,6 @@ 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( @@ -283,7 +276,6 @@ 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 cda2c509519912..a0786b3ca9fe27 100644 --- a/content/browser/utility_process_host.cc +++ b/content/browser/utility_process_host.cc @@ -442,6 +442,12 @@ 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 ffd7575e6214c7..a60a4d18511ded 100644 --- a/content/browser/video_capture_service.cc +++ b/content/browser/video_capture_service.cc @@ -41,13 +41,10 @@ void BindInProcessInstance( } mojo::Remote& GetUIThreadRemote() { - // 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(); + static base::NoDestructor< + mojo::Remote> + remote; + return *remote; } // 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 5c96df5a854fb5..c116da56b2158f 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -7135,7 +7135,7 @@ ForwardingAudioStreamFactory* WebContentsImpl::GetAudioStreamFactory() { ? static_cast( BrowserMainLoop::GetInstance()->user_input_monitor()) : nullptr, - AudioStreamBrokerFactory::CreateImpl()); + GetSystemConnector()->Clone(), 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 db0626ea36bae4..f96640da7aa5cd 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/audio_service.h" +#include "content/public/browser/system_connector.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,6 +19,8 @@ #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" @@ -119,7 +121,7 @@ void WebRtcContentBrowserTestBase::ExecuteJavascriptAndWaitForOk( bool WebRtcContentBrowserTestBase::HasAudioOutputDevices() { bool has_devices = false; base::RunLoop run_loop; - auto audio_system = CreateAudioSystemForAudioService(); + auto audio_system = audio::CreateAudioSystem(GetSystemConnector()->Clone()); 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 34c7223de8e226..7f722e269a7d88 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/audio_service.h" +#include "content/public/browser/system_connector.h" #include "content/public/common/content_features.h" #include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" @@ -28,6 +28,7 @@ #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" @@ -802,7 +803,8 @@ IN_PROC_BROWSER_TEST_F(WebRtcGetUserMediaBrowserTest, // Crash the audio service process. mojo::Remote service_testing_api; - GetAudioService().BindTestingApi( + GetSystemConnector()->Connect( + audio::mojom::kServiceName, 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 fd6a3eb33bd729..c13dde3e0560b6 100644 --- a/content/browser/webrtc/webrtc_internals.cc +++ b/content/browser/webrtc/webrtc_internals.cc @@ -18,7 +18,6 @@ #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" @@ -568,11 +567,8 @@ 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_, std::move(debug_recording)); + audio_debug_recordings_file_path_, GetSystemConnector()->Clone()); for (RenderProcessHost::iterator i( content::RenderProcessHost::AllHostsIterator()); diff --git a/content/public/browser/BUILD.gn b/content/public/browser/BUILD.gn index 0c789957ee9f30..77524595cdeeb4 100644 --- a/content/public/browser/BUILD.gn +++ b/content/public/browser/BUILD.gn @@ -46,7 +46,6 @@ 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", @@ -417,7 +416,6 @@ 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 e61bb1d3070db3..8ae3a073444360 100644 --- a/content/public/browser/DEPS +++ b/content/public/browser/DEPS @@ -5,7 +5,6 @@ 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 deleted file mode 100644 index 6fc86df3637b75..00000000000000 --- a/content/public/browser/audio_service.h +++ /dev/null @@ -1,39 +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 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 18aef76db7f6a6..0a09d0752726ec 100644 --- a/content/public/browser/content_browser_client.h +++ b/content/public/browser/content_browser_client.h @@ -534,6 +534,13 @@ 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 b6ef2b044152e6..2c732b6dfd5d8f 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", + "+services/audio/public/mojom", "+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 5d9bffd7a0fdba..79fdb7b188133b 100644 --- a/content/public/test/audio_service_test_helper.cc +++ b/content/public/test/audio_service_test_helper.cc @@ -12,7 +12,6 @@ #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 { @@ -40,17 +39,17 @@ class AudioServiceTestHelper::TestingApi : public audio::mojom::TestingApi { }; AudioServiceTestHelper::AudioServiceTestHelper() - : testing_api_(new TestingApi) { - if (base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) { - audio::Service::SetTestingApiBinderForTesting( - base::BindRepeating(&AudioServiceTestHelper::BindTestingApiReceiver, - base::Unretained(this))); - } -} + : testing_api_(new TestingApi) {} + +AudioServiceTestHelper::~AudioServiceTestHelper() = default; + +void AudioServiceTestHelper::RegisterAudioBinders( + service_manager::BinderMap* binders) { + if (!base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) + return; -AudioServiceTestHelper::~AudioServiceTestHelper() { - if (base::FeatureList::IsEnabled(features::kAudioServiceOutOfProcess)) - audio::Service::SetTestingApiBinderForTesting(base::NullCallback()); + binders->Add(base::BindRepeating( + &AudioServiceTestHelper::BindTestingApiReceiver, base::Unretained(this))); } void AudioServiceTestHelper::BindTestingApiReceiver( diff --git a/content/public/test/audio_service_test_helper.h b/content/public/test/audio_service_test_helper.h index 3d165108ee4a20..28ef8471b5cbad 100644 --- a/content/public/test/audio_service_test_helper.h +++ b/content/public/test/audio_service_test_helper.h @@ -14,15 +14,19 @@ namespace content { -// Used by testing environments to inject test-only interface support into an +// Used by testing environments to inject test-only interface binders into an // audio service instance. Test suites should create a long-lived instance of -// this class to ensure that instances of the Audio service in the process are -// able to fulfill test API binding requests. +// this class and call RegisterAudioBinders() on a BinderMap which will be used +// to fulfill interface requests within the audio service. 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 793d821272cd1a..f64495949b7082 100644 --- a/content/public/utility/content_utility_client.h +++ b/content/public/utility/content_utility_client.h @@ -70,6 +70,8 @@ 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 7b197a63bc954c..c0a106554c11e8 100644 --- a/content/shell/utility/shell_content_utility_client.cc +++ b/content/shell/utility/shell_content_utility_client.cc @@ -139,4 +139,9 @@ 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 12be6332129a7a..b7da4aa7afe57e 100644 --- a/content/shell/utility/shell_content_utility_client.h +++ b/content/shell/utility/shell_content_utility_client.h @@ -25,6 +25,7 @@ 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 58a57a3aaf2692..789ae85b2a5124 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -384,7 +384,6 @@ 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 69dc3f3897ae77..6cd7c287076a59 100644 --- a/content/utility/BUILD.gn +++ b/content/utility/BUILD.gn @@ -42,7 +42,8 @@ jumbo_source_set("utility") { "//mojo/public/cpp/bindings", "//net", "//sandbox", - "//services/audio", + "//services/audio:lib", + "//services/audio/public/mojom:constants", "//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 32c6e8a8117a95..fb5a0728794132 100644 --- a/content/utility/services.cc +++ b/content/utility/services.cc @@ -8,12 +8,10 @@ #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" @@ -22,13 +20,6 @@ #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 { @@ -42,39 +33,6 @@ 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(); @@ -102,7 +60,6 @@ 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 8a03b9283fd67e..327a490eca8f73 100644 --- a/content/utility/utility_service_factory.cc +++ b/content/utility/utility_service_factory.cc @@ -22,6 +22,9 @@ #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) @@ -35,6 +38,13 @@ #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" @@ -84,9 +94,13 @@ class ContentCdmServiceClient final : public media::CdmService::Client { } // namespace -UtilityServiceFactory::UtilityServiceFactory() = default; +UtilityServiceFactory::UtilityServiceFactory() + : network_registry_(std::make_unique()), + audio_binders_(std::make_unique()) { + GetContentClient()->utility()->RegisterAudioBinders(audio_binders_.get()); +} -UtilityServiceFactory::~UtilityServiceFactory() = default; +UtilityServiceFactory::~UtilityServiceFactory() {} void UtilityServiceFactory::RunService( const std::string& service_name, @@ -101,8 +115,11 @@ 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) - if (service_name == media::mojom::kCdmServiceName) { + else if (service_name == media::mojom::kCdmServiceName) { service = std::make_unique( std::make_unique(), std::move(request)); } @@ -129,4 +146,41 @@ 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 654aa3553aa1a2..280ee0ee9ba51f 100644 --- a/content/utility/utility_service_factory.h +++ b/content/utility/utility_service_factory.h @@ -12,6 +12,7 @@ #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" @@ -29,6 +30,14 @@ 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 131281546e34fe..2ba73b51dc3002 100644 --- a/services/audio/BUILD.gn +++ b/services/audio/BUILD.gn @@ -4,9 +4,25 @@ 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") -source_set("audio") { +# 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") { sources = [ "debug_recording.cc", "debug_recording.h", @@ -60,6 +76,8 @@ source_set("audio") { "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", ] @@ -77,7 +95,8 @@ source_set("audio") { "//media", "//media/webrtc", "//services/audio/public/mojom", - "//services/service_manager/sandbox", + "//services/service_manager/public/cpp", + "//services/service_manager/sandbox:sandbox", ] if (is_linux) { @@ -126,6 +145,7 @@ 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", @@ -140,25 +160,42 @@ 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 15d0b1ee3c4829..12b2fd3d3b68a9 100644 --- a/services/audio/debug_recording.cc +++ b/services/audio/debug_recording.cc @@ -15,8 +15,11 @@ namespace audio { DebugRecording::DebugRecording( mojo::PendingReceiver receiver, - media::AudioManager* audio_manager) - : audio_manager_(audio_manager), receiver_(this, std::move(receiver)) { + media::AudioManager* audio_manager, + TracedServiceRef service_ref) + : audio_manager_(audio_manager), + receiver_(this, std::move(receiver)), + service_ref_(std::move(service_ref)) { DCHECK(audio_manager_ != nullptr); DCHECK(audio_manager_->GetTaskRunner()->BelongsToCurrentThread()); @@ -47,6 +50,8 @@ 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 59a1f47344045d..9a13b78b3701a9 100644 --- a/services/audio/debug_recording.h +++ b/services/audio/debug_recording.h @@ -13,6 +13,7 @@ #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; @@ -25,7 +26,8 @@ namespace audio { class DebugRecording : public mojom::DebugRecording { public: DebugRecording(mojo::PendingReceiver receiver, - media::AudioManager* audio_manager); + media::AudioManager* audio_manager, + TracedServiceRef service_ref); // Disables audio debug recording if Enable() was called before. ~DebugRecording() override; @@ -49,6 +51,7 @@ 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 a07b7235bd2202..dfe738da0205ad 100644 --- a/services/audio/debug_recording_unittest.cc +++ b/services/audio/debug_recording_unittest.cc @@ -17,6 +17,8 @@ #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::_; @@ -57,9 +59,13 @@ class MockFileProvider : public mojom::DebugRecordingFileProvider { DISALLOW_COPY_AND_ASSIGN(MockFileProvider); }; -class DebugRecordingTest : public media::AudioDebugRecordingTest { +class DebugRecordingTest : public media::AudioDebugRecordingTest, + public service_manager::ServiceKeepalive::Observer { public: - DebugRecordingTest() = default; + DebugRecordingTest() : service_keepalive_(nullptr, base::TimeDelta()) { + service_keepalive_.AddObserver(this); + } + ~DebugRecordingTest() override = default; void SetUp() override { @@ -70,12 +76,17 @@ 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())); + static_cast(mock_audio_manager_.get()), + TracedServiceRef(service_keepalive_.CreateRef(), + "audio::DebugRecording Binding")); + EXPECT_FALSE(service_keepalive_.HasNoRefs()); } void EnableDebugRecording() { @@ -84,21 +95,28 @@ 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(_)); @@ -109,6 +127,7 @@ 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); @@ -116,6 +135,7 @@ TEST_F(DebugRecordingTest, ResetWithoutEnableDoesNotDisableDebugRecording) { } TEST_F(DebugRecordingTest, CreateWavFileCallsFileProviderCreateWavFile) { + EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(1)); CreateDebugRecording(); mojo::PendingRemote remote_file_provider; @@ -140,6 +160,7 @@ TEST_F(DebugRecordingTest, CreateWavFileCallsFileProviderCreateWavFile) { } TEST_F(DebugRecordingTest, SequencialCreate) { + EXPECT_CALL(*this, OnNoServiceRefs()).Times(Exactly(2)); CreateDebugRecording(); DestroyDebugRecording(); CreateDebugRecording(); @@ -149,6 +170,7 @@ 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 7a127dbc849174..4b5aef24ffc3f2 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) { +void DeviceNotifier::Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref) { DCHECK(task_runner_->RunsTasksInCurrentSequence()); - receivers_.Add(this, std::move(receiver)); + receivers_.Add(this, std::move(receiver), std::move(context_ref)); } void DeviceNotifier::RegisterListener( diff --git a/services/audio/device_notifier.h b/services/audio/device_notifier.h index b031ecc2ff3f96..42a5c1714f6b3d 100644 --- a/services/audio/device_notifier.h +++ b/services/audio/device_notifier.h @@ -13,6 +13,7 @@ #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; @@ -28,7 +29,8 @@ class DeviceNotifier final : public base::SystemMonitor::DevicesChangedObserver, DeviceNotifier(); ~DeviceNotifier() final; - void Bind(mojo::PendingReceiver receiver); + void Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref); // mojom::DeviceNotifier implementation. void RegisterListener( @@ -43,7 +45,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 910fb8086223c9..84b662f4f5f7e9 100644 --- a/services/audio/device_notifier_unittest.cc +++ b/services/audio/device_notifier_unittest.cc @@ -13,6 +13,8 @@ #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" @@ -35,34 +37,48 @@ class MockDeviceListener : public mojom::DeviceListener { } // namespace -class DeviceNotifierTest : public ::testing::Test { +class DeviceNotifierTest : public ::testing::Test, + public service_manager::ServiceKeepalive::Observer { public: DeviceNotifierTest() - : system_monitor_(std::make_unique()) {} + : system_monitor_(std::make_unique()), + service_keepalive_(nullptr, base::TimeDelta()) { + service_keepalive_.AddObserver(this); + } protected: + MOCK_METHOD0(OnNoServiceRefs, void()); + void CreateDeviceNotifier() { device_notifier_ = std::make_unique(); - device_notifier_->Bind( - remote_device_notifier_.BindNewPipeAndPassReceiver()); + device_notifier_->Bind(remote_device_notifier_.BindNewPipeAndPassReceiver(), + TracedServiceRef(service_keepalive_.CreateRef(), + "audio::DeviceNotifier Binding")); + EXPECT_FALSE(service_keepalive_.HasNoRefs()); } 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 75bc3f4534d0bc..7c3afc39d16f7e 100644 --- a/services/audio/log_factory_manager.cc +++ b/services/audio/log_factory_manager.cc @@ -6,6 +6,8 @@ #include +#include "services/service_manager/public/cpp/service_context_ref.h" + namespace audio { LogFactoryManager::LogFactoryManager() = default; @@ -15,9 +17,10 @@ LogFactoryManager::~LogFactoryManager() { } void LogFactoryManager::Bind( - mojo::PendingReceiver receiver) { + mojo::PendingReceiver receiver, + TracedServiceRef context_ref) { DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - receivers_.Add(this, std::move(receiver)); + receivers_.Add(this, std::move(receiver), std::move(context_ref)); } void LogFactoryManager::SetLogFactory( diff --git a/services/audio/log_factory_manager.h b/services/audio/log_factory_manager.h index 4fca633a0990a6..fec947f969e393 100644 --- a/services/audio/log_factory_manager.h +++ b/services/audio/log_factory_manager.h @@ -11,6 +11,7 @@ #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; @@ -26,7 +27,8 @@ class LogFactoryManager final : public mojom::LogFactoryManager { LogFactoryManager(); ~LogFactoryManager() final; - void Bind(mojo::PendingReceiver receiver); + void Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref); // LogFactoryManager implementation. void SetLogFactory( @@ -34,7 +36,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 57db26002ba870..a174965bf1392c 100644 --- a/services/audio/log_factory_manager_unittest.cc +++ b/services/audio/log_factory_manager_unittest.cc @@ -16,6 +16,8 @@ #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" @@ -76,32 +78,47 @@ class MockAudioLogFactory : public media::mojom::AudioLogFactory { } // namespace -class LogFactoryManagerTest : public ::testing::Test { +class LogFactoryManagerTest + : public ::testing::Test, + public service_manager::ServiceKeepalive::Observer { public: - LogFactoryManagerTest() = default; + LogFactoryManagerTest() : service_keepalive_(nullptr, base::TimeDelta()) { + service_keepalive_.AddObserver(this); + } protected: + MOCK_METHOD0(OnNoServiceRefs, void()); + void CreateLogFactoryManager() { log_factory_manager_ = std::make_unique(); log_factory_manager_->Bind( - remote_log_factory_manager_.BindNewPipeAndPassReceiver()); + remote_log_factory_manager_.BindNewPipeAndPassReceiver(), + TracedServiceRef(service_keepalive_.CreateRef(), + "audio::LogFactoryManager Binding")); + EXPECT_FALSE(service_keepalive_.HasNoRefs()); } 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 43563aac5deda5..eb6adddfc5d683 100644 --- a/services/audio/public/cpp/BUILD.gn +++ b/services/audio/public/cpp/BUILD.gn @@ -4,6 +4,8 @@ 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", @@ -30,6 +32,19 @@ 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", ] } @@ -50,7 +65,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 a3060033221a84..3e93e664999e99 100644 --- a/services/audio/public/cpp/OWNERS +++ b/services/audio/public/cpp/OWNERS @@ -1,3 +1,7 @@ +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 new file mode 100644 index 00000000000000..3ef5966a9522b4 --- /dev/null +++ b/services/audio/public/cpp/audio_system_factory.cc @@ -0,0 +1,19 @@ +// 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 new file mode 100644 index 00000000000000..cb9beb10b0404b --- /dev/null +++ b/services/audio/public/cpp/audio_system_factory.h @@ -0,0 +1,27 @@ +// 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 486dfb5ae71e6c..82a5d28197422d 100644 --- a/services/audio/public/cpp/audio_system_to_service_adapter.cc +++ b/services/audio/public/cpp/audio_system_to_service_adapter.cc @@ -12,6 +12,8 @@ #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 { @@ -223,22 +225,21 @@ OnInputDeviceInfoCallback WrapGetInputDeviceInfoReply( } // namespace AudioSystemToServiceAdapter::AudioSystemToServiceAdapter( - SystemInfoBinder system_info_binder, + std::unique_ptr connector, base::TimeDelta disconnect_timeout) - : system_info_binder_(std::move(system_info_binder)), + : connector_(std::move(connector)), disconnect_timeout_(disconnect_timeout) { - DCHECK(system_info_binder_); + DCHECK(connector_); DETACH_FROM_THREAD(thread_checker_); } AudioSystemToServiceAdapter::AudioSystemToServiceAdapter( - SystemInfoBinder system_info_binder) - : AudioSystemToServiceAdapter(std::move(system_info_binder), - base::TimeDelta()) {} + std::unique_ptr connector) + : AudioSystemToServiceAdapter(std::move(connector), base::TimeDelta()) {} AudioSystemToServiceAdapter::~AudioSystemToServiceAdapter() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - if (system_info_.is_bound()) { + if (!!system_info_) { TRACE_EVENT_NESTABLE_ASYNC_END1("audio", "AudioSystemToServiceAdapter bound", this, "disconnect reason", "destroyed"); @@ -319,12 +320,14 @@ mojom::SystemInfo* AudioSystemToServiceAdapter::GetSystemInfo() { if (!system_info_) { TRACE_EVENT_NESTABLE_ASYNC_BEGIN0( "audio", "AudioSystemToServiceAdapter bound", this); - system_info_binder_.Run(system_info_.BindNewPipeAndPassReceiver()); + connector_->Connect(mojom::kServiceName, + 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 a0729d64f840a8..3dbdd19b9e5812 100644 --- a/services/audio/public/cpp/audio_system_to_service_adapter.h +++ b/services/audio/public/cpp/audio_system_to_service_adapter.h @@ -8,16 +8,18 @@ #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. @@ -25,17 +27,14 @@ 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(SystemInfoBinder system_info_binder, - base::TimeDelta disconnect_timeout); - explicit AudioSystemToServiceAdapter(SystemInfoBinder system_info_binder); + AudioSystemToServiceAdapter( + std::unique_ptr connector, + base::TimeDelta disconnect_timeout); + explicit AudioSystemToServiceAdapter( + std::unique_ptr connector); ~AudioSystemToServiceAdapter() override; // AudioSystem implementation. @@ -62,7 +61,7 @@ class AudioSystemToServiceAdapter : public media::AudioSystem { void OnConnectionError(); // Will be bound to the thread AudioSystemToServiceAdapter is used on. - const SystemInfoBinder system_info_binder_; + const std::unique_ptr connector_; 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 40d38902b124e2..9307d6e0782082 100644 --- a/services/audio/public/cpp/debug_recording_session.cc +++ b/services/audio/public/cpp/debug_recording_session.cc @@ -11,6 +11,8 @@ #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 { @@ -65,14 +67,17 @@ void DebugRecordingSession::DebugRecordingFileProvider::CreateWavFile( DebugRecordingSession::DebugRecordingSession( const base::FilePath& file_name_base, - mojo::PendingRemote debug_recording) { + std::unique_ptr connector) { + DCHECK(connector); + mojo::PendingRemote remote_file_provider; file_provider_ = std::make_unique( remote_file_provider.InitWithNewPipeAndPassReceiver(), file_name_base); - if (debug_recording) { - debug_recording_.Bind(std::move(debug_recording)); + + connector->Connect(audio::mojom::kServiceName, + debug_recording_.BindNewPipeAndPassReceiver()); + if (debug_recording_.is_bound()) 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 4d6e55d8d06277..9cb25e0c60b20a 100644 --- a/services/audio/public/cpp/debug_recording_session.h +++ b/services/audio/public/cpp/debug_recording_session.h @@ -10,11 +10,14 @@ #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; } @@ -51,9 +54,8 @@ class DebugRecordingSession : public media::AudioDebugRecordingSession { DISALLOW_COPY_AND_ASSIGN(DebugRecordingFileProvider); }; - DebugRecordingSession( - const base::FilePath& file_name_base, - mojo::PendingRemote debug_recording); + DebugRecordingSession(const base::FilePath& file_name_base, + std::unique_ptr connector); ~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 ad283713a063af..1be75c7be4dee4 100644 --- a/services/audio/public/cpp/debug_recording_session_factory.cc +++ b/services/audio/public/cpp/debug_recording_session_factory.cc @@ -8,16 +8,18 @@ #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, - mojo::PendingRemote debug_recording) { - DCHECK(debug_recording); + std::unique_ptr connector) { + DCHECK(connector); + return std::make_unique( - debug_recording_file_path, std::move(debug_recording)); + debug_recording_file_path, std::move(connector)); } } // 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 0732b76310d82c..fae9da88e19750 100644 --- a/services/audio/public/cpp/debug_recording_session_factory.h +++ b/services/audio/public/cpp/debug_recording_session_factory.h @@ -7,8 +7,6 @@ #include -#include "services/audio/public/mojom/debug_recording.mojom.h" - namespace base { class FilePath; } @@ -17,12 +15,16 @@ namespace media { class AudioDebugRecordingSession; } +namespace service_manager { +class Connector; +} + namespace audio { std::unique_ptr CreateAudioDebugRecordingSession( const base::FilePath& debug_recording_file_path, - mojo::PendingRemote debug_recording); + std::unique_ptr connector); } // namespace audio diff --git a/services/audio/public/cpp/device_factory.cc b/services/audio/public/cpp/device_factory.cc index 3ecbe054180d9a..acb8ed396f5d1c 100644 --- a/services/audio/public/cpp/device_factory.cc +++ b/services/audio/public/cpp/device_factory.cc @@ -10,15 +10,22 @@ #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( - mojo::PendingRemote stream_factory, + std::unique_ptr connector, 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); } @@ -26,8 +33,11 @@ scoped_refptr CreateInputDevice( scoped_refptr CreateInputDevice( mojo::PendingRemote stream_factory, const std::string& device_id) { - return CreateInputDevice(std::move(stream_factory), device_id, - mojo::NullRemote()); + 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); } } // namespace audio diff --git a/services/audio/public/cpp/device_factory.h b/services/audio/public/cpp/device_factory.h index a079a470fbccce..1d7525ade84ccb 100644 --- a/services/audio/public/cpp/device_factory.h +++ b/services/audio/public/cpp/device_factory.h @@ -11,6 +11,7 @@ #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 { @@ -19,7 +20,7 @@ scoped_refptr CreateInputDevice( const std::string& device_id); scoped_refptr CreateInputDevice( - mojo::PendingRemote stream_factory, + std::unique_ptr connector, 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 e7a25e24abd12a..5e778bfbab1553 100644 --- a/services/audio/public/cpp/fake_system_info.cc +++ b/services/audio/public/cpp/fake_system_info.cc @@ -5,7 +5,8 @@ #include "services/audio/public/cpp/fake_system_info.h" #include "base/bind.h" -#include "services/audio/service.h" +#include "services/audio/public/mojom/constants.mojom.h" +#include "services/service_manager/public/cpp/service_binding.h" namespace audio { @@ -16,13 +17,16 @@ FakeSystemInfo::~FakeSystemInfo() {} // static void FakeSystemInfo::OverrideGlobalBinderForAudioService( FakeSystemInfo* fake_system_info) { - Service::SetSystemInfoBinderForTesting(base::BindRepeating( - &FakeSystemInfo::Bind, base::Unretained(fake_system_info))); + service_manager::ServiceBinding::OverrideInterfaceBinderForTesting( + mojom::kServiceName, + base::BindRepeating(&FakeSystemInfo::Bind, + base::Unretained(fake_system_info))); } // static void FakeSystemInfo::ClearGlobalBinderForAudioService() { - Service::SetSystemInfoBinderForTesting(base::NullCallback()); + service_manager::ServiceBinding ::ClearInterfaceBinderOverrideForTesting< + mojom::SystemInfo>(mojom::kServiceName); } void FakeSystemInfo::GetInputStreamParameters( diff --git a/services/audio/public/cpp/input_ipc_unittest.cc b/services/audio/public/cpp/input_ipc_unittest.cc index 2799503d8d479b..f723c6f1ba442e 100644 --- a/services/audio/public/cpp/input_ipc_unittest.cc +++ b/services/audio/public/cpp/input_ipc_unittest.cc @@ -17,6 +17,7 @@ #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 new file mode 100644 index 00000000000000..c6ce7bbaec6865 --- /dev/null +++ b/services/audio/public/cpp/manifest.cc @@ -0,0 +1,52 @@ +// 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 new file mode 100644 index 00000000000000..50b7096276ba40 --- /dev/null +++ b/services/audio/public/cpp/manifest.h @@ -0,0 +1,17 @@ +// 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 b336e350d989ca..38169c9d4497ae 100644 --- a/services/audio/public/cpp/output_device_unittest.cc +++ b/services/audio/public/cpp/output_device_unittest.cc @@ -16,6 +16,7 @@ #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 847ea18ae063d9..1b2d21967a9535 100644 --- a/services/audio/public/cpp/sounds/audio_stream_handler.cc +++ b/services/audio/public/cpp/sounds/audio_stream_handler.cc @@ -20,6 +20,7 @@ #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 { @@ -43,10 +44,10 @@ class AudioStreamHandler::AudioStreamContainer : public media::AudioRendererSink::RenderCallback { public: explicit AudioStreamContainer( - SoundsManager::StreamFactoryBinder stream_factory_binder, + std::unique_ptr connector, std::unique_ptr wav_audio) : started_(false), - stream_factory_binder_(std::move(stream_factory_binder)), + connector_(std::move(connector)), cursor_(0), delayed_stop_posted_(false), wav_audio_(std::move(wav_audio)) { @@ -71,8 +72,8 @@ class AudioStreamHandler::AudioStreamContainer g_observer_for_testing->Initialize(this, params); } else { mojo::PendingRemote stream_factory; - stream_factory_binder_.Run( - stream_factory.InitWithNewPipeAndPassReceiver()); + connector_->Connect(audio::mojom::kServiceName, + stream_factory.InitWithNewPipeAndPassReceiver()); device_ = std::make_unique( std::move(stream_factory), params, this, std::string()); } @@ -155,7 +156,7 @@ class AudioStreamHandler::AudioStreamContainer } bool started_; - const SoundsManager::StreamFactoryBinder stream_factory_binder_; + std::unique_ptr connector_; std::unique_ptr device_; scoped_refptr task_runner_; @@ -170,7 +171,7 @@ class AudioStreamHandler::AudioStreamContainer }; AudioStreamHandler::AudioStreamHandler( - SoundsManager::StreamFactoryBinder stream_factory_binder, + std::unique_ptr connector, const base::StringPiece& wav_data) { task_runner_ = base::SequencedTaskRunnerHandle::Get(); std::unique_ptr wav_audio = @@ -191,8 +192,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(stream_factory_binder), - std::move(wav_audio))); + stream_.reset( + new AudioStreamContainer(std::move(connector), 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 b8e6d7fe855804..8d98fe3377882a 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/audio/public/cpp/sounds/sounds_manager.h" +#include "services/service_manager/public/cpp/connector.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( - SoundsManager::StreamFactoryBinder stream_factory_binder, + std::unique_ptr connector, 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 493e552ea35bd9..17a7d851f46f78 100644 --- a/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc +++ b/services/audio/public/cpp/sounds/audio_stream_handler_unittest.cc @@ -32,10 +32,17 @@ 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_; }; @@ -47,7 +54,8 @@ TEST_F(AudioStreamHandlerTest, Play) { std::unique_ptr audio_stream_handler; SetObserverForTesting(&observer); - audio_stream_handler.reset(new AudioStreamHandler(base::DoNothing(), data)); + audio_stream_handler.reset( + new AudioStreamHandler(std::move(connector_), data)); ASSERT_TRUE(audio_stream_handler->IsInitialized()); EXPECT_EQ(base::TimeDelta::FromMicroseconds(20u), @@ -71,7 +79,8 @@ TEST_F(AudioStreamHandlerTest, ConsecutivePlayRequests) { std::unique_ptr audio_stream_handler; SetObserverForTesting(&observer); - audio_stream_handler.reset(new AudioStreamHandler(base::DoNothing(), data)); + audio_stream_handler.reset( + new AudioStreamHandler(std::move(connector_), data)); ASSERT_TRUE(audio_stream_handler->IsInitialized()); EXPECT_EQ(base::TimeDelta::FromMicroseconds(20u), @@ -100,7 +109,8 @@ 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(base::DoNothing(), "RIFF1234WAVEjunkjunkjunkjunk"); + AudioStreamHandler handler(std::move(connector_), + "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 3d08eb89309ffc..520968e6d0c25c 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(StreamFactoryBinder stream_factory_binder) - : stream_factory_binder_(std::move(stream_factory_binder)) {} + SoundsManagerImpl(std::unique_ptr connector) + : connector_(std::move(connector)) {} ~SoundsManagerImpl() override { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); } @@ -44,7 +44,7 @@ class SoundsManagerImpl : public SoundsManager { std::unique_ptr handler; }; std::vector handlers_; - StreamFactoryBinder stream_factory_binder_; + std::unique_ptr connector_; DISALLOW_COPY_AND_ASSIGN(SoundsManagerImpl); }; @@ -57,7 +57,7 @@ bool SoundsManagerImpl::Initialize(SoundKey key, } std::unique_ptr handler( - new AudioStreamHandler(stream_factory_binder_, data)); + new AudioStreamHandler(connector_->Clone(), data)); if (!handler->IsInitialized()) { LOG(WARNING) << "Can't initialize AudioStreamHandler for key=" << key; return false; @@ -105,12 +105,13 @@ SoundsManager::~SoundsManager() { } // static -void SoundsManager::Create(StreamFactoryBinder stream_factory_binder) { +void SoundsManager::Create( + std::unique_ptr connector) { CHECK(!g_instance || g_initialized_for_testing) << "SoundsManager::Create() is called twice"; if (g_initialized_for_testing) return; - g_instance = new SoundsManagerImpl(std::move(stream_factory_binder)); + g_instance = new SoundsManagerImpl(std::move(connector)); } // static diff --git a/services/audio/public/cpp/sounds/sounds_manager.h b/services/audio/public/cpp/sounds/sounds_manager.h index 7128cb5f1aaef2..2975ad3f10c714 100644 --- a/services/audio/public/cpp/sounds/sounds_manager.h +++ b/services/audio/public/cpp/sounds/sounds_manager.h @@ -7,14 +7,12 @@ #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 "mojo/public/cpp/bindings/pending_receiver.h" -#include "services/audio/public/mojom/stream_factory.mojom.h" +#include "services/service_manager/public/cpp/connector.h" namespace audio { @@ -25,9 +23,7 @@ class SoundsManager { typedef int SoundKey; // Creates a singleton instance of the SoundsManager. - using StreamFactoryBinder = base::RepeatingCallback)>; - static void Create(StreamFactoryBinder stream_factory_binder); + static void Create(std::unique_ptr connector); // 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 95ec58fb835f69..cfe1769d66add2 100644 --- a/services/audio/public/cpp/sounds/sounds_manager_unittest.cc +++ b/services/audio/public/cpp/sounds/sounds_manager_unittest.cc @@ -29,7 +29,9 @@ class SoundsManagerTest : public testing::Test { ~SoundsManagerTest() override = default; void SetUp() override { - SoundsManager::Create(base::DoNothing()); + mojo::PendingReceiver connector_receiver; + connector_ = service_manager::Connector::Create(&connector_receiver); + SoundsManager::Create(connector_->Clone()); base::RunLoop().RunUntilIdle(); } @@ -42,6 +44,8 @@ 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 5808a33db1f7f7..78ae7f19f1f338 100644 --- a/services/audio/public/mojom/BUILD.gn +++ b/services/audio/public/mojom/BUILD.gn @@ -8,7 +8,6 @@ mojom("mojom") { sources = [ "audio_device_description.mojom", "audio_processing.mojom", - "audio_service.mojom", "debug_recording.mojom", "device_notifications.mojom", "log_factory_manager.mojom", @@ -18,7 +17,14 @@ 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 deleted file mode 100644 index 53b4505f6465bb..00000000000000 --- a/services/audio/public/mojom/audio_service.mojom +++ /dev/null @@ -1,36 +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. - -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 new file mode 100644 index 00000000000000..0d4b0cc5fdb84a --- /dev/null +++ b/services/audio/public/mojom/constants.mojom @@ -0,0 +1,7 @@ +// 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 f02c4e7ce850d2..246dd45a3bc8d9 100644 --- a/services/audio/service.cc +++ b/services/audio/service.cc @@ -32,29 +32,22 @@ 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; -} - -Service::TestingApiBinder& GetTestingApiBinder() { - static base::NoDestructor binder; - return *binder; -} - } // namespace -Service::Service(std::unique_ptr audio_manager_accessor, - bool enable_remote_client_support, - mojo::PendingReceiver receiver) - : receiver_(this, std::move(receiver)), +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), audio_manager_accessor_(std::move(audio_manager_accessor)), - enable_remote_client_support_(enable_remote_client_support) { + enable_remote_client_support_(enable_remote_client_support), + binders_(std::move(extra_binders)) { magic_bytes_ = 0x600DC0DEu; g_service_state_for_crashing.Set("constructing"); DCHECK(audio_manager_accessor_); @@ -70,13 +63,6 @@ Service::Service(std::unique_ptr audio_manager_accessor, // 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"); } @@ -105,64 +91,106 @@ Service::~Service() { // static base::DeferredSequencedTaskRunner* Service::GetInProcessTaskRunner() { - static const base::NoDestructor< - scoped_refptr> + static base::NoDestructor> instance(base::MakeRefCounted()); return instance->get(); } -// static -void Service::SetSystemInfoBinderForTesting(SystemInfoBinder binder) { - GetSystemInfoBinder() = std::move(binder); +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"); } -// static -void Service::SetTestingApiBinderForTesting(TestingApiBinder binder) { - GetTestingApiBinder() = std::move(binder); +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); } -void Service::BindSystemInfo( - mojo::PendingReceiver receiver) { +void Service::OnDisconnected() { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + g_service_state_for_crashing.Set("connection lost"); + Terminate(); +} - auto& binder_override = GetSystemInfoBinder(); - if (binder_override) { - binder_override.Run(std::move(receiver)); - return; - } +void Service::BindSystemInfoReceiver( + mojo::PendingReceiver receiver) { + CHECK_EQ(magic_bytes_, 0x600DC0DEu); + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); if (!system_info_) { system_info_ = std::make_unique( audio_manager_accessor_->GetAudioManager()); } - system_info_->Bind(std::move(receiver)); + system_info_->Bind( + std::move(receiver), + TracedServiceRef(keepalive_.CreateRef(), "audio::SystemInfo Binding")); } -void Service::BindDebugRecording( +void Service::BindDebugRecordingReceiver( 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(receiver), audio_manager_accessor_->GetAudioManager(), + std::move(service_ref)); } -void Service::BindStreamFactory( +void Service::BindStreamFactoryReceiver( 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)); + stream_factory_->Bind( + std::move(receiver), + TracedServiceRef(keepalive_.CreateRef(), "audio::StreamFactory Binding")); } -void Service::BindDeviceNotifier( +void Service::BindDeviceNotifierReceiver( mojo::PendingReceiver receiver) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -171,23 +199,21 @@ void Service::BindDeviceNotifier( InitializeDeviceMonitor(); if (!device_notifier_) device_notifier_ = std::make_unique(); - device_notifier_->Bind(std::move(receiver)); + device_notifier_->Bind(std::move(receiver), + TracedServiceRef(keepalive_.CreateRef(), + "audio::DeviceNotifier Binding")); } -void Service::BindLogFactoryManager( +void Service::BindLogFactoryManagerReceiver( 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)); -} - -void Service::BindTestingApi( - mojo::PendingReceiver receiver) { - auto& binder = GetTestingApiBinder(); - if (binder) - binder.Run(std::move(receiver)); + log_factory_manager_->Bind( + std::move(receiver), + TracedServiceRef(keepalive_.CreateRef(), + "audio::LogFactoryManager Binding")); } void Service::InitializeDeviceMonitor() { diff --git a/services/audio/service.h b/services/audio/service.h index b127348ec5f407..b37b5393755849 100644 --- a/services/audio/service.h +++ b/services/audio/service.h @@ -14,14 +14,17 @@ #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" namespace base { class DeferredSequencedTaskRunner; @@ -41,7 +44,7 @@ class LogFactoryManager; class ServiceMetrics; class SystemInfo; -class Service : public mojom::AudioService { +class Service : public service_manager::Service { public: // Abstracts AudioManager ownership. Lives and must be accessed on a thread // its created on, and that thread must be AudioManager main thread. @@ -62,45 +65,41 @@ class Service : public mojom::AudioService { virtual void SetAudioLogFactory(media::AudioLogFactory* factory) = 0; }; - // 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 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. Service(std::unique_ptr audio_manager_accessor, + base::Optional quit_timeout, bool enable_remote_client_support, - mojo::PendingReceiver receiver); + std::unique_ptr extra_binders, + 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(); - // 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. - using TestingApiBinder = - base::RepeatingCallback)>; - static void SetTestingApiBinderForTesting(TestingApiBinder binder); + // 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; private: - // 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; + 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); // Initializes a platform-specific device monitor for device-change // notifications. If the client uses the DeviceNotifier interface to get @@ -113,9 +112,11 @@ class Service : public mojom::AudioService { // 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_; @@ -129,6 +130,8 @@ class Service : public mojom::AudioService { 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 827167b9cc63b4..083cebc25e15b8 100644 --- a/services/audio/service_factory.cc +++ b/services/audio/service_factory.cc @@ -4,9 +4,16 @@ #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" @@ -15,20 +22,60 @@ 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), - /*enable_remote_client_support=*/false, std::move(receiver)); + base::nullopt /* do not quit if all clients disconnected */, + false /* enable_device_notifications */, + std::make_unique(), std::move(receiver)); } std::unique_ptr CreateStandaloneService( - mojo::PendingReceiver receiver) { + std::unique_ptr extra_binders, + mojo::PendingReceiver receiver) { return std::make_unique( std::make_unique( base::BindOnce(&media::AudioManager::Create)), - /*enable_remote_client_support=*/true, std::move(receiver)); + GetQuitTimeout(), true /* enable_remote_client_support */, + std::move(extra_binders), std::move(receiver)); } } // namespace audio diff --git a/services/audio/service_factory.h b/services/audio/service_factory.h index 7690b7ff90c83d..c00990eb4b65e1 100644 --- a/services/audio/service_factory.h +++ b/services/audio/service_factory.h @@ -8,8 +8,9 @@ #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; @@ -22,14 +23,15 @@ 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( - mojo::PendingReceiver receiver); + std::unique_ptr extra_binders, + mojo::PendingReceiver receiver); } // namespace audio diff --git a/services/audio/service_main.cc b/services/audio/service_main.cc new file mode 100644 index 00000000000000..9f3c96398c3d36 --- /dev/null +++ b/services/audio/service_main.cc @@ -0,0 +1,18 @@ +// 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 b9adb5d90a2f05..4f2076f5c14e2b 100644 --- a/services/audio/stream_factory.cc +++ b/services/audio/stream_factory.cc @@ -34,10 +34,11 @@ StreamFactory::~StreamFactory() { magic_bytes_ = 0xDEADBEEFu; } -void StreamFactory::Bind(mojo::PendingReceiver receiver) { +void StreamFactory::Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref) { CHECK_EQ(magic_bytes_, 0x600DC0DEu); DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_); - receivers_.Add(this, std::move(receiver)); + receivers_.Add(this, std::move(receiver), std::move(context_ref)); } void StreamFactory::CreateInputStream( @@ -55,9 +56,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", this, - "device id", device_id, "params", - params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( + "audio", "CreateInputStream", receivers_.current_context().id_for_trace(), + "device id", device_id, "params", params.AsHumanReadableString()); if (processing_config && processing_config->settings.requires_apm() && params.GetBufferDuration() != base::TimeDelta::FromMilliseconds(10)) { @@ -115,9 +116,10 @@ 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", this, - "device id", output_device_id, "params", - params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( + "audio", "CreateOutputStream", + receivers_.current_context().id_for_trace(), "device id", + output_device_id, "params", params.AsHumanReadableString()); // Unretained is safe since |this| indirectly owns the OutputStream. auto deleter_callback = base::BindOnce(&StreamFactory::DestroyOutputStream, @@ -151,8 +153,9 @@ 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", this, "group id", - group_id.GetLowForSerialization()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT1( + "audio", "BindMuter", receivers_.current_context().id_for_trace(), + "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(), @@ -186,10 +189,11 @@ 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", this, - "group id", - group_id.GetLowForSerialization(), - "params", params.AsHumanReadableString()); + TRACE_EVENT_NESTABLE_ASYNC_INSTANT2( + "audio", "CreateLoopbackStream", + receivers_.current_context().id_for_trace(), "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 f2ca6ee2695f04..2480950e769c14 100644 --- a/services/audio/stream_factory.h +++ b/services/audio/stream_factory.h @@ -22,6 +22,7 @@ #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; @@ -48,7 +49,8 @@ class StreamFactory final : public mojom::StreamFactory { explicit StreamFactory(media::AudioManager* audio_manager); ~StreamFactory() final; - void Bind(mojo::PendingReceiver receiver); + void Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref); // StreamFactory implementation. void CreateInputStream( @@ -107,7 +109,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 new file mode 100644 index 00000000000000..06dff3e0408344 --- /dev/null +++ b/services/audio/stream_factory_unittest.cc @@ -0,0 +1,42 @@ +// 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 04ce14562283ff..4f6c624885cfcc 100644 --- a/services/audio/system_info.cc +++ b/services/audio/system_info.cc @@ -7,6 +7,7 @@ #include #include "base/trace_event/trace_event.h" +#include "services/service_manager/public/cpp/service_context_ref.h" namespace audio { @@ -19,9 +20,10 @@ SystemInfo::~SystemInfo() { DCHECK_CALLED_ON_VALID_SEQUENCE(binding_sequence_checker_); } -void SystemInfo::Bind(mojo::PendingReceiver receiver) { +void SystemInfo::Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref) { DCHECK_CALLED_ON_VALID_SEQUENCE(binding_sequence_checker_); - receivers_.Add(this, std::move(receiver)); + receivers_.Add(this, std::move(receiver), std::move(context_ref)); } void SystemInfo::GetInputStreamParameters( diff --git a/services/audio/system_info.h b/services/audio/system_info.h index a99714346cec32..504881bcbdd3d9 100644 --- a/services/audio/system_info.h +++ b/services/audio/system_info.h @@ -14,6 +14,7 @@ #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; @@ -26,7 +27,8 @@ class SystemInfo : public mojom::SystemInfo { explicit SystemInfo(media::AudioManager* audio_manager); ~SystemInfo() override; - void Bind(mojo::PendingReceiver receiver); + void Bind(mojo::PendingReceiver receiver, + TracedServiceRef context_ref); private: // audio::mojom::SystemInfo implementation. @@ -50,7 +52,9 @@ class SystemInfo : public mojom::SystemInfo { media::AudioSystemHelper helper_; - mojo::ReceiverSet receivers_; + // Each receiver increases ref count of the service context, so that the + // service knows when it is in use. + 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 e157694a42b8de..8ec249d9dad484 100644 --- a/services/audio/test/audio_system_to_service_adapter_test.cc +++ b/services/audio/test/audio_system_to_service_adapter_test.cc @@ -12,7 +12,10 @@ #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" @@ -38,10 +41,18 @@ class AudioSystemToServiceAdapterTestBase : public testing::Test { std::make_unique(audio_manager_.get()); system_info_receiver_ = std::make_unique>( system_info_impl_.get()); - audio_system_ = - std::make_unique(base::BindRepeating( + + 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( &AudioSystemToServiceAdapterTestBase::BindSystemInfoReceiver, base::Unretained(this))); + + audio_system_ = + std::make_unique(std::move(connector)); } void TearDown() override { @@ -66,14 +77,14 @@ class AudioSystemToServiceAdapterTestBase : public testing::Test { std::unique_ptr audio_system_; private: - void BindSystemInfoReceiver( - mojo::PendingReceiver receiver) { + void BindSystemInfoReceiver(mojo::ScopedMessagePipeHandle handle) { EXPECT_TRUE(system_info_receiver_) << "AudioSystemToServiceAdapter should " "not request AudioSysteInfo during " "construction"; - ASSERT_FALSE(system_info_receiver_->is_bound()); + EXPECT_FALSE(system_info_receiver_->is_bound()); system_info_bind_requested_.Call(); - system_info_receiver_->Bind(std::move(receiver)); + system_info_receiver_->Bind( + mojo::PendingReceiver(std::move(handle))); } DISALLOW_COPY_AND_ASSIGN(AudioSystemToServiceAdapterTestBase); @@ -394,16 +405,22 @@ class AudioSystemToServiceAdapterDisconnectTest : public testing::Test { } }; // class MockSystemInfo - AudioSystemToServiceAdapter::SystemInfoBinder GetSystemInfoBinder() { - return base::BindRepeating( - &AudioSystemToServiceAdapterDisconnectTest::BindSystemInfoReceiver, - base::Unretained(this)); + 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; } - void BindSystemInfoReceiver( - mojo::PendingReceiver receiver) { + void BindSystemInfoReceiver(mojo::ScopedMessagePipeHandle handle) { ClientConnected(); - system_info_receiver_.Bind(std::move(receiver)); + system_info_receiver_.Bind( + mojo::PendingReceiver(std::move(handle))); system_info_receiver_.set_disconnect_handler(base::BindOnce( &AudioSystemToServiceAdapterDisconnectTest::OnConnectionError, base::Unretained(this))); @@ -430,8 +447,7 @@ class AudioSystemToServiceAdapterDisconnectTest : public testing::Test { TEST_F(AudioSystemToServiceAdapterDisconnectTest, ResponseDelayIsShorterThanDisconnectTimeout) { const base::TimeDelta kDisconnectTimeout = kResponseDelay * 2; - AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), - kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -447,8 +463,7 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, ResponseDelayIsLongerThanDisconnectTimeout) { const base::TimeDelta kDisconnectTimeout = kResponseDelay / 2; - AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), - kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -464,8 +479,7 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, DisconnectTimeoutIsResetOnSecondRequest) { const base::TimeDelta kDisconnectTimeout = kResponseDelay * 1.5; - AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), - kDisconnectTimeout); + AudioSystemToServiceAdapter audio_system(GetConnector(), kDisconnectTimeout); { EXPECT_CALL(*this, ClientConnected()); EXPECT_CALL(*this, ClientDisconnected()).Times(0); @@ -488,8 +502,7 @@ TEST_F(AudioSystemToServiceAdapterDisconnectTest, TEST_F(AudioSystemToServiceAdapterDisconnectTest, DoesNotDisconnectIfNoTimeout) { - AudioSystemToServiceAdapter audio_system(GetSystemInfoBinder(), - base::TimeDelta()); + AudioSystemToServiceAdapter audio_system(GetConnector(), 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 237d2e63f6707c..8886144dd320a7 100644 --- a/services/audio/test/debug_recording_session_unittest.cc +++ b/services/audio/test/debug_recording_session_unittest.cc @@ -19,8 +19,10 @@ #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" @@ -91,7 +93,7 @@ class DebugRecordingSessionTest : public media::AudioDebugRecordingTest { service_ = CreateEmbeddedService( static_cast(mock_audio_manager_.get()), - service_remote_.BindNewPipeAndPassReceiver()); + connector_factory_.RegisterInstance(audio::mojom::kServiceName)); task_environment_.RunUntilIdle(); } @@ -100,12 +102,10 @@ 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), - std::move(debug_recording))); + std::make_unique( + base::FilePath(kBaseFileName), + connector_factory_.CreateConnector())); task_environment_.RunUntilIdle(); return session; } @@ -117,7 +117,7 @@ class DebugRecordingSessionTest : public media::AudioDebugRecordingTest { } private: - mojo::Remote service_remote_; + service_manager::TestConnectorFactory connector_factory_; 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 0eba84a326e152..0735fcd9978985 100644 --- a/services/audio/test/in_process_service_test.cc +++ b/services/audio/test/in_process_service_test.cc @@ -10,8 +10,15 @@ #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/mojom/audio_service.mojom.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/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; @@ -19,27 +26,32 @@ using testing::Invoke; namespace audio { -class ServiceTestHelper { +class ServiceTestHelper : public service_manager::Service { public: class AudioThreadContext : public base::RefCountedThreadSafe { public: - explicit AudioThreadContext(media::AudioManager* audio_manager) - : audio_manager_(audio_manager) {} + AudioThreadContext(media::AudioManager* audio_manager, + base::TimeDelta service_quit_timeout) + : audio_manager_(audio_manager), + service_quit_timeout_(service_quit_timeout) {} void CreateServiceOnAudioThread( - mojo::PendingReceiver receiver) { + service_manager::mojom::ServiceRequest request) { if (!audio_manager_->GetTaskRunner()->BelongsToCurrentThread()) { audio_manager_->GetTaskRunner()->PostTask( FROM_HERE, base::BindOnce(&AudioThreadContext::CreateServiceOnAudioThread, - this, std::move(receiver))); + this, std::move(request))); return; } DCHECK(!service_); service_ = std::make_unique( std::make_unique(audio_manager_), - false /* device_notifications_enabled */, std::move(receiver)); + service_quit_timeout_, false /* device_notifications_enabled */, + std::make_unique(), std::move(request)); + service_->set_termination_closure(base::BindOnce( + &AudioThreadContext::QuitOnAudioThread, base::Unretained(this))); } void QuitOnAudioThread() { @@ -52,19 +64,21 @@ class ServiceTestHelper { virtual ~AudioThreadContext() = default; media::AudioManager* const audio_manager_; + const base::TimeDelta service_quit_timeout_; std::unique_ptr service_; DISALLOW_COPY_AND_ASSIGN(AudioThreadContext); }; - 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(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)) {} - ~ServiceTestHelper() { + ~ServiceTestHelper() override { // Ensure that the AudioThreadContext is destroyed on the correct thread by // passing our only reference into a task posted there. audio_manager_->GetTaskRunner()->PostTask( @@ -72,16 +86,34 @@ class ServiceTestHelper { std::move(audio_thread_context_))); } - mojom::AudioService& service() { return *service_remote_.get(); } + 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); + } + } 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 @@ -91,16 +123,37 @@ class ServiceTestHelper { template class InProcessServiceTest : public testing::Test { public: - InProcessServiceTest() - : audio_manager_( + 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_( std::make_unique(use_audio_thread)), - helper_(std::make_unique(&audio_manager_)), + helper_(std::make_unique( + &audio_manager_, + service_quit_timeout, + test_service_manager_.RegisterTestInstance(kTestServiceName))), audio_system_(std::make_unique( - base::BindRepeating(&InProcessServiceTest::BindSystemInfo, - base::Unretained(this)))) {} - ~InProcessServiceTest() override = default; + connector()->Clone())) {} + + InProcessServiceTest() + : InProcessServiceTest(base::TimeDelta() /* not timeout */) {} + + ~InProcessServiceTest() override {} protected: + service_manager::Connector* connector() { + DCHECK(helper_); + return helper_->connector(); + } + void TearDown() override { audio_system_.reset(); @@ -118,11 +171,8 @@ 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_; @@ -134,8 +184,8 @@ class InProcessServiceTest : public testing::Test { class FakeSystemInfoTest : public InProcessServiceTest, public FakeSystemInfo { public: - FakeSystemInfoTest() = default; - ~FakeSystemInfoTest() override = default; + FakeSystemInfoTest() {} + ~FakeSystemInfoTest() override {} protected: MOCK_METHOD0(MethodCalled, void()); @@ -159,6 +209,23 @@ 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 new file mode 100644 index 00000000000000..57005f9fc0e7bd --- /dev/null +++ b/services/audio/test/service_lifetime_connector_test.cc @@ -0,0 +1,277 @@ +// 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 new file mode 100644 index 00000000000000..43d989853b5c4c --- /dev/null +++ b/services/audio/test/service_lifetime_test_template.h @@ -0,0 +1,152 @@ +// 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 new file mode 100644 index 00000000000000..7098f5c592d5fa --- /dev/null +++ b/services/audio/test/service_observer_mock.cc @@ -0,0 +1,35 @@ +// 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 new file mode 100644 index 00000000000000..a0f8657fdf1342 --- /dev/null +++ b/services/audio/test/service_observer_mock.h @@ -0,0 +1,51 @@ +// 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 new file mode 100644 index 00000000000000..43d70b8b042805 --- /dev/null +++ b/services/audio/test/standalone_service_test.cc @@ -0,0 +1,64 @@ +// 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/traced_service_ref.cc b/services/audio/traced_service_ref.cc new file mode 100644 index 00000000000000..09a0df3b826f9e --- /dev/null +++ b/services/audio/traced_service_ref.cc @@ -0,0 +1,41 @@ +// 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 new file mode 100644 index 00000000000000..98b691ed7d2300 --- /dev/null +++ b/services/audio/traced_service_ref.h @@ -0,0 +1,47 @@ +// 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_