0

Cleanup media::KeySystemSupportObserver

- Rename KeySystemSupportObserver to KeySystemSupportRegistration
- Remove unnecessary virtual function
- Add clarifying comments.

TESTED:
* Add logs to see the size observer remote set in browser process.
  (after this line
  https://source.chromium.org/chromium/chromium/src/+/main:content/browser/media/key_system_support_impl.cc;drc=eb3c1d87f1c717de44fdaadb8b83041584f6a6b3;l=59)

The following is observed only when moving key systems to a per-frame
model. See
https://chromium-review.googlesource.com/c/chromium/src/+/5168850

* Visit integration.widevine.com/player
* Click play observer logs (observer_remotes_.size() = 1)
* Visit another site (e.g. shaka-player-demo.appspot.com) and play
  encrypted media
* Check logs find (observer_remotes_.size() = 2)
* Close integration.widevine.com/player tab and ensure render process
  for that tab is destroyed (e.g. in Task Manager)
* Visit integration.staging.widevine.com/player and click play
* Observer logs and find (observer_remotes_.size() = 2) whereas before
  https://chromium-review.googlesource.com/c/chromium/src/+/5168850 the
  logs would say (observer_remotes_.size() = 3)

Bug: b/321307544
Change-Id: I78133a594c0fcf96326ce25aa42fbe754491bc60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5381159
Commit-Queue: Feras Aldahlawi <feras@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Zijie He <zijiehe@google.com>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1275988}
This commit is contained in:
Feras Aldahlawi
2024-03-21 01:46:11 +00:00
committed by Chromium LUCI CQ
parent a98c8de532
commit 64a9bed52f
30 changed files with 102 additions and 96 deletions

@ -235,7 +235,7 @@ void AwContentRendererClient::RunScriptsAtDocumentStart(
communication->RunScriptsAtDocumentStart();
}
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
AwContentRendererClient::GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) {
// WebView always allows persisting data.

@ -55,7 +55,7 @@ class AwContentRendererClient : public content::ContentRendererClient,
uint64_t VisitedLinkHash(std::string_view canonical_url) override;
bool IsLinkVisited(uint64_t link_hash) override;
void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) override;
std::unique_ptr<blink::WebSocketHandshakeThrottleProvider>
CreateWebSocketHandshakeThrottleProvider() override;

@ -1591,7 +1591,7 @@ ChromeContentRendererClient::CreateWebSocketHandshakeThrottleProvider() {
browser_interface_broker_.get());
}
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
ChromeContentRendererClient::GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) {
#if BUILDFLAG(ENABLE_LIBRARY_CDMS) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)

@ -27,7 +27,7 @@
#include "content/public/renderer/render_thread.h"
#include "extensions/buildflags/buildflags.h"
#include "ipc/ipc_channel_proxy.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/media_buildflags.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/generic_pending_receiver.h"
@ -171,7 +171,7 @@ class ChromeContentRendererClient
std::unique_ptr<media::SpeechRecognitionClient> CreateSpeechRecognitionClient(
content::RenderFrame* render_frame) override;
#endif // BUILDFLAG(ENABLE_SPEECH_SERVICE)
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) override;
bool IsPluginAllowedToUseCameraDeviceAPI(const GURL& url) override;
void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;

@ -6,9 +6,9 @@
#include "chrome/renderer/chrome_render_thread_observer.h"
#include "components/cdm/renderer/key_system_support_update.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
std::unique_ptr<media::KeySystemSupportObserver> GetChromeKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetChromeKeySystems(
media::GetSupportedKeySystemsCB cb) {
return cdm::GetSupportedKeySystemsUpdates(
!ChromeRenderThreadObserver::is_incognito_process(), std::move(cb));

@ -6,10 +6,10 @@
#define CHROME_RENDERER_MEDIA_CHROME_KEY_SYSTEMS_H_
#include "media/base/key_system_info.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
// Register the key systems supported by the chrome/ layer.
std::unique_ptr<media::KeySystemSupportObserver> GetChromeKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetChromeKeySystems(
media::GetSupportedKeySystemsCB cb);
#endif // CHROME_RENDERER_MEDIA_CHROME_KEY_SYSTEMS_H_

@ -170,7 +170,7 @@ void CastContentRendererClient::RunScriptsAtDocumentStart(
void CastContentRendererClient::RunScriptsAtDocumentEnd(
content::RenderFrame* render_frame) {}
std::unique_ptr<::media::KeySystemSupportObserver>
std::unique_ptr<::media::KeySystemSupportRegistration>
CastContentRendererClient::GetSupportedKeySystems(
::media::GetSupportedKeySystemsCB cb) {
#if BUILDFLAG(IS_ANDROID)

@ -17,7 +17,7 @@
#include "content/public/renderer/content_renderer_client.h"
#include "media/base/audio_codecs.h"
#include "media/base/audio_parameters.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "mojo/public/cpp/bindings/receiver.h"
namespace cast_receiver {
@ -56,7 +56,7 @@ class CastContentRendererClient
void RenderFrameCreated(content::RenderFrame* render_frame) override;
void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
std::unique_ptr<::media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<::media::KeySystemSupportRegistration> GetSupportedKeySystems(
::media::GetSupportedKeySystemsCB cb) override;
bool IsSupportedAudioType(const ::media::AudioType& type) override;
bool IsSupportedVideoType(const ::media::VideoType& type) override;

@ -589,9 +589,9 @@ void OnKeySystemSupportUpdated(
} // namespace
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystemsUpdates(
bool can_persist_data,
media::GetSupportedKeySystemsCB cb) {
std::unique_ptr<media::KeySystemSupportRegistration>
GetSupportedKeySystemsUpdates(bool can_persist_data,
media::GetSupportedKeySystemsCB cb) {
return content::ObserveKeySystemSupportUpdate(base::BindRepeating(
&OnKeySystemSupportUpdated, can_persist_data, std::move(cb)));
}

@ -6,7 +6,7 @@
#define COMPONENTS_CDM_RENDERER_KEY_SYSTEM_SUPPORT_UPDATE_H_
#include "media/base/key_system_info.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
namespace cdm {
@ -15,9 +15,9 @@ namespace cdm {
// incognito mode). `cb` is called with the list of available key systems, and
// may be called multiple times if the list changes (e.g. a new key system
// becomes available).
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystemsUpdates(
bool can_persist_data,
media::GetSupportedKeySystemsCB cb);
std::unique_ptr<media::KeySystemSupportRegistration>
GetSupportedKeySystemsUpdates(bool can_persist_data,
media::GetSupportedKeySystemsCB cb);
} // namespace cdm

@ -167,7 +167,7 @@ bool ContentRendererClient::IsOriginIsolatedPepperPlugin(
return true;
}
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
ContentRendererClient::GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) {
std::move(cb).Run({});

@ -26,7 +26,7 @@
#include "content/public/common/content_client.h"
#include "media/base/audio_parameters.h"
#include "media/base/key_system_info.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/base/supported_types.h"
#include "third_party/blink/public/platform/url_loader_throttle_provider.h"
#include "third_party/blink/public/platform/web_content_settings_client.h"
@ -282,7 +282,7 @@ class CONTENT_EXPORT ContentRendererClient {
virtual bool IsOriginIsolatedPepperPlugin(const base::FilePath& plugin_path);
// Allows embedder to register the key system(s) it supports.
virtual std::unique_ptr<media::KeySystemSupportObserver>
virtual std::unique_ptr<media::KeySystemSupportRegistration>
GetSupportedKeySystems(media::GetSupportedKeySystemsCB cb);
// Allows embedder to describe customized audio capabilities.

@ -6,14 +6,16 @@
#include "base/logging.h"
#include "content/public/renderer/render_thread.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
namespace content {
namespace {
class KeySystemSupportObserverImpl
: public media::KeySystemSupportObserver,
: public media::KeySystemSupportRegistration,
public media::mojom::KeySystemSupportObserver {
public:
KeySystemSupportObserverImpl(
@ -38,24 +40,26 @@ class KeySystemSupportObserverImpl
mojo::Receiver<media::mojom::KeySystemSupportObserver> receiver_;
};
std::unique_ptr<media::KeySystemSupportObserver> ObserveKeySystemSupportUpdate(
media::KeySystemSupportCB cb) {
} // namespace
std::unique_ptr<media::KeySystemSupportRegistration>
ObserveKeySystemSupportUpdate(media::KeySystemSupportCB cb) {
DVLOG(1) << __func__;
// `key_system_support` will be destructed after this function returns. This
// is fine since the observer will stay registered in KeySystemSupportImpl,
// which is a singleton in the browser process.
// `key_system_support` will stay alive as long as the returned value of this
// function is not destructed by the caller.
mojo::Remote<media::mojom::KeySystemSupport> key_system_support;
content::RenderThread::Get()->BindHostReceiver(
key_system_support.BindNewPipeAndPassReceiver());
mojo::PendingRemote<media::mojom::KeySystemSupportObserver> observer_remote;
auto key_system_support_update =
std::make_unique<KeySystemSupportObserverImpl>(
std::move(cb), observer_remote.InitWithNewPipeAndPassReceiver());
std::unique_ptr<media::KeySystemSupportRegistration>
key_system_support_registration =
std::make_unique<KeySystemSupportObserverImpl>(
std::move(cb), observer_remote.InitWithNewPipeAndPassReceiver());
key_system_support->AddObserver(std::move(observer_remote));
return std::move(key_system_support_update);
return key_system_support_registration;
}
} // namespace content

@ -11,7 +11,7 @@
#include "base/functional/callback.h"
#include "content/common/content_export.h"
#include "media/base/key_system_capability.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/mojo/mojom/key_system_support.mojom.h"
namespace content {
@ -23,7 +23,7 @@ using KeySystemSupportCB = base::RepeatingCallback<void(KeySystemCapabilities)>;
// Observes key system support updates. The callback `cb` will be called with
// the current key system support, then called every time the key system support
// changes.
CONTENT_EXPORT std::unique_ptr<media::KeySystemSupportObserver>
CONTENT_EXPORT std::unique_ptr<media::KeySystemSupportRegistration>
ObserveKeySystemSupportUpdate(media::KeySystemSupportCB cb);
} // namespace content

@ -76,7 +76,7 @@ RenderMediaClient::RenderMediaClient()
RenderMediaClient::~RenderMediaClient() = default;
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
RenderMediaClient::GetSupportedKeySystems(media::GetSupportedKeySystemsCB cb) {
return GetContentClient()->renderer()->GetSupportedKeySystems(std::move(cb));
}

@ -8,7 +8,7 @@
#include "base/synchronization/waitable_event.h"
#include "media/base/audio_parameters.h"
#include "media/base/decoder.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/base/media_client.h"
#include "media/base/supported_video_decoder_config.h"
#include "media/mojo/mojom/interface_factory.mojom.h"
@ -30,7 +30,7 @@ class RenderMediaClient : public media::MediaClient {
static void Initialize();
// MediaClient implementation.
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) final;
bool IsSupportedAudioType(const media::AudioType& type) final;
bool IsSupportedVideoType(const media::VideoType& type) final;

@ -297,7 +297,7 @@ ShellContentRendererClient::CreateURLLoaderThrottleProvider(
}
#if BUILDFLAG(ENABLE_MOJO_CDM)
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
ShellContentRendererClient::GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) {
media::KeySystemInfos key_systems;

@ -56,7 +56,7 @@ class ShellContentRendererClient : public ContentRendererClient {
blink::URLLoaderThrottleProviderType provider_type) override;
#if BUILDFLAG(ENABLE_MOJO_CDM)
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) override;
#endif

@ -200,7 +200,7 @@ WebEngineContentRendererClient::CreateURLLoaderThrottleProvider(
return std::make_unique<WebEngineURLLoaderThrottleProvider>(this);
}
std::unique_ptr<media::KeySystemSupportObserver>
std::unique_ptr<media::KeySystemSupportRegistration>
WebEngineContentRendererClient::GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) {
media::KeySystemInfos key_systems;

@ -51,7 +51,7 @@ class WebEngineContentRendererClient : public content::ContentRendererClient {
// content::ContentRendererClient overrides.
void RenderThreadStarted() override;
void RenderFrameCreated(content::RenderFrame* render_frame) override;
std::unique_ptr<media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<media::KeySystemSupportRegistration> GetSupportedKeySystems(
media::GetSupportedKeySystemsCB cb) override;
bool IsSupportedVideoType(const media::VideoType& type) override;
std::unique_ptr<blink::URLLoaderThrottleProvider>

@ -198,8 +198,8 @@ source_set("base") {
"key_systems.h",
"key_systems_impl.cc",
"key_systems_impl.h",
"key_systems_support_observer.cc",
"key_systems_support_observer.h",
"key_systems_support_registration.cc",
"key_systems_support_registration.h",
"localized_strings.cc",
"localized_strings.h",
"logging_override_if_enabled.h",

@ -309,7 +309,7 @@ void KeySystemsImpl::UpdateSupportedKeySystems() {
OnSupportedKeySystemsUpdated({});
return;
}
key_system_support_observer_ = GetMediaClient()->GetSupportedKeySystems(
key_system_support_registration_ = GetMediaClient()->GetSupportedKeySystems(
base::BindRepeating(&KeySystemsImpl::OnSupportedKeySystemsUpdated,
weak_factory_.GetWeakPtr()));
}

@ -20,7 +20,7 @@
#include "media/base/eme_constants.h"
#include "media/base/key_system_info.h"
#include "media/base/key_systems.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/base/media_export.h"
namespace media {
@ -128,7 +128,8 @@ class KeySystemsImpl : public KeySystems {
// Makes sure all methods are called from the same thread.
base::ThreadChecker thread_checker_;
std::unique_ptr<KeySystemSupportObserver> key_system_support_observer_;
std::unique_ptr<KeySystemSupportRegistration>
key_system_support_registration_;
base::WeakPtrFactory<KeySystemsImpl> weak_factory_{this};
};

@ -1,12 +0,0 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "media/base/key_systems_support_observer.h"
namespace media {
KeySystemSupportObserver::KeySystemSupportObserver() = default;
KeySystemSupportObserver::~KeySystemSupportObserver() = default;
} // namespace media

@ -1,32 +0,0 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef MEDIA_BASE_KEY_SYSTEMS_SUPPORT_OBSERVER_H_
#define MEDIA_BASE_KEY_SYSTEMS_SUPPORT_OBSERVER_H_
#include "base/containers/flat_map.h"
#include "base/functional/callback.h"
#include "media/base/key_system_capability.h"
#include "media/base/media_export.h"
namespace media {
using KeySystemCapabilities =
base::flat_map<std::string, media::KeySystemCapability>;
using KeySystemSupportCB = base::RepeatingCallback<void(KeySystemCapabilities)>;
class MEDIA_EXPORT KeySystemSupportObserver {
public:
KeySystemSupportObserver();
KeySystemSupportObserver(const KeySystemSupportObserver&) = delete;
KeySystemSupportObserver& operator=(const KeySystemSupportObserver&) = delete;
virtual ~KeySystemSupportObserver();
virtual void OnKeySystemSupportUpdated(
const KeySystemCapabilities& key_system_capabilities) = 0;
};
} // namespace media
#endif // MEDIA_BASE_KEY_SYSTEMS_SUPPORT_OBSERVER_H_

@ -0,0 +1,12 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "media/base/key_systems_support_registration.h"
namespace media {
KeySystemSupportRegistration::KeySystemSupportRegistration() = default;
KeySystemSupportRegistration::~KeySystemSupportRegistration() = default;
} // namespace media

@ -0,0 +1,33 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef MEDIA_BASE_KEY_SYSTEMS_SUPPORT_REGISTRATION_H_
#define MEDIA_BASE_KEY_SYSTEMS_SUPPORT_REGISTRATION_H_
#include "base/containers/flat_map.h"
#include "base/functional/callback.h"
#include "media/base/key_system_capability.h"
#include "media/base/media_export.h"
namespace media {
using KeySystemCapabilities =
base::flat_map<std::string, media::KeySystemCapability>;
using KeySystemSupportCB = base::RepeatingCallback<void(KeySystemCapabilities)>;
// A class that is used to keep the KeySystemSupport and
// KeySystemSupportObserver mojo channel registered between renderer and
// browser alive and destructed properly.
class MEDIA_EXPORT KeySystemSupportRegistration {
public:
KeySystemSupportRegistration();
KeySystemSupportRegistration(const KeySystemSupportRegistration&) = delete;
KeySystemSupportRegistration& operator=(const KeySystemSupportRegistration&) =
delete;
virtual ~KeySystemSupportRegistration();
};
} // namespace media
#endif // MEDIA_BASE_KEY_SYSTEMS_SUPPORT_REGISTRATION_H_

@ -234,7 +234,7 @@ class TestMediaClient : public MediaClient {
~TestMediaClient() override;
// MediaClient implementation.
std::unique_ptr<::media::KeySystemSupportObserver> GetSupportedKeySystems(
std::unique_ptr<::media::KeySystemSupportRegistration> GetSupportedKeySystems(
GetSupportedKeySystemsCB cb) final;
bool IsSupportedAudioType(const AudioType& type) final;
bool IsSupportedVideoType(const VideoType& type) final;
@ -257,7 +257,7 @@ class TestMediaClient : public MediaClient {
TestMediaClient::TestMediaClient() = default;
TestMediaClient::~TestMediaClient() = default;
std::unique_ptr<::media::KeySystemSupportObserver>
std::unique_ptr<::media::KeySystemSupportRegistration>
TestMediaClient::GetSupportedKeySystems(GetSupportedKeySystemsCB cb) {
// Save the callback for future updates.
get_supported_key_systems_cb_ = cb;

@ -12,7 +12,7 @@
#include "media/base/audio_codecs.h"
#include "media/base/audio_parameters.h"
#include "media/base/key_system_info.h"
#include "media/base/key_systems_support_observer.h"
#include "media/base/key_systems_support_registration.h"
#include "media/base/media_export.h"
#include "media/base/media_types.h"
#include "media/base/video_codecs.h"
@ -42,7 +42,7 @@ class MEDIA_EXPORT MediaClient {
virtual ~MediaClient();
// Adds properties for supported key systems.
virtual std::unique_ptr<media::KeySystemSupportObserver>
virtual std::unique_ptr<media::KeySystemSupportRegistration>
GetSupportedKeySystems(GetSupportedKeySystemsCB cb) = 0;
// Returns true if the given audio config is supported.

@ -872,7 +872,7 @@ class MockMediaClient : public media::MediaClient {
// MediaClient implementation.
MOCK_METHOD1(GetSupportedKeySystems,
std::unique_ptr<::media::KeySystemSupportObserver>(
std::unique_ptr<::media::KeySystemSupportRegistration>(
GetSupportedKeySystemsCB cb));
MOCK_METHOD1(IsSupportedAudioType, bool(const media::AudioType& type));
MOCK_METHOD1(IsSupportedVideoType, bool(const media::VideoType& type));