diff options
author | xhwang <xhwang@chromium.org> | 2015-11-02 18:09:27 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-03 02:10:09 +0000 |
commit | b24facba10f213d4448bf108ba0d01821b2ba05d (patch) | |
tree | dc37dde6542fb240c51058ddde8b01e091a8aa66 | |
parent | 098cffd5ec2661f0e283584729f325feb95ecc73 (diff) | |
download | chromium_src-b24facba10f213d4448bf108ba0d01821b2ba05d.zip chromium_src-b24facba10f213d4448bf108ba0d01821b2ba05d.tar.gz chromium_src-b24facba10f213d4448bf108ba0d01821b2ba05d.tar.bz2 |
media: Make MediaKeys ref-counted.
MediaKeys (CDM) is created and owned by the EME stack (e.g. CdmSessionAdapter)
but is dereferenced by the media pipeline (e.g. DecryptingVideoDecoder). During
teardown, we need to make sure the CDM is not dereferenced by the media pipeline
after it's destructed.
In the Render process, this is guaranteed by the order of destruction of
MediaKeys and Media elements in the Blink layer.
However, when the CDM and the media pipeline are running out-of-process, because
the order of IPC messages on different IPC channels is nondeterministic, we need
some extra mechanism to ensure that the media pipeline doesn't dereference a
destructed CDM. Currently on Android, this is achieved by using an extra
PlayerTracker interface. However, in some situations, e.g. when the media
pipeline and CDM are running on different threads, the destruction handling code
could be pretty complicated.
When a MojoMediaApplication is hosted in a non-Render process, we face the same
issue.
Starting with this CL, we'll make MediaKeys ref-counted, so that the media
pipeline that uses it will also hold a ref-count to it. This will greatly
simplify the lifetime management between the CDM and the media pipeline.
BUG=511040
TEST=All existing use cases still work.
Review URL: https://codereview.chromium.org/1407933010
Cr-Commit-Position: refs/heads/master@{#357514}
59 files changed, 463 insertions, 519 deletions
diff --git a/chromecast/browser/media/cast_browser_cdm_factory.cc b/chromecast/browser/media/cast_browser_cdm_factory.cc index 62a1df2..e93927b 100644 --- a/chromecast/browser/media/cast_browser_cdm_factory.cc +++ b/chromecast/browser/media/cast_browser_cdm_factory.cc @@ -15,7 +15,7 @@ namespace chromecast { namespace media { -::media::ScopedBrowserCdmPtr CastBrowserCdmFactory::CreateBrowserCdm( +scoped_refptr<::media::MediaKeys> CastBrowserCdmFactory::CreateBrowserCdm( const std::string& key_system_name, bool use_hw_secure_codecs, const ::media::SessionMessageCB& session_message_cb, @@ -28,7 +28,7 @@ namespace media { CastKeySystem key_system(GetKeySystemByName(key_system_name)); - scoped_ptr<chromecast::media::BrowserCdmCast> browser_cdm; + scoped_refptr<chromecast::media::BrowserCdmCast> browser_cdm; if (key_system == chromecast::media::KEY_SYSTEM_CLEAR_KEY) { // TODO(gunsch): handle ClearKey decryption. See crbug.com/441957 } else { @@ -45,17 +45,17 @@ namespace media { ::media::BindToCurrentLoop(legacy_session_error_cb), ::media::BindToCurrentLoop(session_keys_change_cb), ::media::BindToCurrentLoop(session_expiration_update_cb))); - return ::media::ScopedBrowserCdmPtr(new BrowserCdmCastUi( - browser_cdm.Pass(), MediaMessageLoop::GetTaskRunner())); + return scoped_refptr<::media::MediaKeys>( + new BrowserCdmCastUi(browser_cdm, MediaMessageLoop::GetTaskRunner())); } LOG(INFO) << "No matching key system found."; - return ::media::ScopedBrowserCdmPtr(); + return nullptr; } -scoped_ptr<BrowserCdmCast> CastBrowserCdmFactory::CreatePlatformBrowserCdm( +scoped_refptr<BrowserCdmCast> CastBrowserCdmFactory::CreatePlatformBrowserCdm( const CastKeySystem& key_system) { - return scoped_ptr<BrowserCdmCast>(); + return nullptr; } } // namespace media diff --git a/chromecast/browser/media/cast_browser_cdm_factory.h b/chromecast/browser/media/cast_browser_cdm_factory.h index e10858d..c55b8fa 100644 --- a/chromecast/browser/media/cast_browser_cdm_factory.h +++ b/chromecast/browser/media/cast_browser_cdm_factory.h @@ -6,8 +6,8 @@ #define CHROMECAST_BROWSER_MEDIA_CAST_BROWSER_CDM_FACTORY_H_ #include "chromecast/media/base/key_systems_common.h" -#include "media/base/browser_cdm.h" #include "media/base/browser_cdm_factory.h" +#include "media/base/media_keys.h" namespace chromecast { namespace media { @@ -20,7 +20,7 @@ class CastBrowserCdmFactory : public ::media::BrowserCdmFactory { ~CastBrowserCdmFactory() override {}; // ::media::BrowserCdmFactory implementation: - ::media::ScopedBrowserCdmPtr CreateBrowserCdm( + scoped_refptr<::media::MediaKeys> CreateBrowserCdm( const std::string& key_system, bool use_hw_secure_codecs, const ::media::SessionMessageCB& session_message_cb, @@ -31,7 +31,7 @@ class CastBrowserCdmFactory : public ::media::BrowserCdmFactory { override; // Provides a platform-specific BrowserCdm instance. - virtual scoped_ptr<BrowserCdmCast> CreatePlatformBrowserCdm( + virtual scoped_refptr<BrowserCdmCast> CreatePlatformBrowserCdm( const CastKeySystem& key_system); private: diff --git a/chromecast/browser/media/cma_message_filter_host.cc b/chromecast/browser/media/cma_message_filter_host.cc index 0a531e7..6de55ad 100644 --- a/chromecast/browser/media/cma_message_filter_host.cc +++ b/chromecast/browser/media/cma_message_filter_host.cc @@ -108,7 +108,7 @@ void SetCdmOnUiThread( return; } - ::media::BrowserCdm* cdm = host->GetBrowserCdm(render_frame_id, cdm_id); + scoped_refptr<::media::MediaKeys> cdm = host->GetCdm(render_frame_id, cdm_id); if (!cdm) { LOG(WARNING) << "Could not find BrowserCdm (" << render_frame_id << "," << cdm_id << ")"; @@ -116,13 +116,10 @@ void SetCdmOnUiThread( } BrowserCdmCast* browser_cdm_cast = - static_cast<BrowserCdmCastUi*>(cdm)->browser_cdm_cast(); + static_cast<BrowserCdmCastUi*>(cdm.get())->browser_cdm_cast(); MediaMessageLoop::GetTaskRunner()->PostTask( - FROM_HERE, - base::Bind(&SetCdmOnCmaThread, - render_process_id, - media_id, - browser_cdm_cast)); + FROM_HERE, base::Bind(&SetCdmOnCmaThread, render_process_id, media_id, + base::Unretained(browser_cdm_cast))); } } // namespace diff --git a/chromecast/media/cdm/browser_cdm_cast.cc b/chromecast/media/cdm/browser_cdm_cast.cc index 92a0022..8d8ed64 100644 --- a/chromecast/media/cdm/browser_cdm_cast.cc +++ b/chromecast/media/cdm/browser_cdm_cast.cc @@ -89,7 +89,7 @@ void BrowserCdmCast::Initialize( const ::media::SessionExpirationUpdateCB& session_expiration_update_cb) { DCHECK(thread_checker_.CalledOnValidThread()); - player_tracker_impl_.reset(new ::media::PlayerTrackerImpl); + player_tracker_impl_.reset(new ::media::PlayerTrackerImpl()); session_message_cb_ = session_message_cb; session_closed_cb_ = session_closed_cb; @@ -152,24 +152,16 @@ void BrowserCdmCast::OnSessionKeysChange( base::Unretained(browser_cdm_cast_.get()), ##__VA_ARGS__)) BrowserCdmCastUi::BrowserCdmCastUi( - scoped_ptr<BrowserCdmCast> browser_cdm_cast, + const scoped_refptr<BrowserCdmCast>& browser_cdm_cast, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) - : browser_cdm_cast_(browser_cdm_cast.Pass()), task_runner_(task_runner) { -} + : browser_cdm_cast_(browser_cdm_cast), task_runner_(task_runner) {} BrowserCdmCastUi::~BrowserCdmCastUi() { DCHECK(thread_checker_.CalledOnValidThread()); - task_runner_->DeleteSoon(FROM_HERE, browser_cdm_cast_.release()); -} - -int BrowserCdmCastUi::RegisterPlayer(const base::Closure& new_key_cb, - const base::Closure& cdm_unset_cb) { - NOTREACHED() << "RegisterPlayer should be called on BrowserCdmCast"; - return -1; -} - -void BrowserCdmCastUi::UnregisterPlayer(int registration_id) { - NOTREACHED() << "UnregisterPlayer should be called on BrowserCdmCast"; + browser_cdm_cast_->AddRef(); + BrowserCdmCast* raw_cdm = browser_cdm_cast_.get(); + browser_cdm_cast_ = nullptr; + task_runner_->ReleaseSoon(FROM_HERE, raw_cdm); } BrowserCdmCast* BrowserCdmCastUi::browser_cdm_cast() const { diff --git a/chromecast/media/cdm/browser_cdm_cast.h b/chromecast/media/cdm/browser_cdm_cast.h index a4c80fb..b738b00 100644 --- a/chromecast/media/cdm/browser_cdm_cast.h +++ b/chromecast/media/cdm/browser_cdm_cast.h @@ -14,8 +14,10 @@ #include "base/callback.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/sequenced_task_runner_helpers.h" #include "base/threading/thread_checker.h" -#include "media/base/browser_cdm.h" +#include "media/base/media_keys.h" +#include "media/base/player_tracker.h" #include "media/cdm/json_web_key.h" namespace base { @@ -30,17 +32,17 @@ namespace chromecast { namespace media { class DecryptContextImpl; -// BrowserCdmCast is an extension of BrowserCdm that provides common +// BrowserCdmCast is an extension of MediaKeys that provides common // functionality across CDM implementations. // All these additional functions are synchronous so: // - either both the CDM and the media pipeline must be running on the same // thread, // - or BrowserCdmCast implementations must use some locks. // -class BrowserCdmCast : public ::media::BrowserCdm { +class BrowserCdmCast : public ::media::MediaKeys, + public ::media::PlayerTracker { public: BrowserCdmCast(); - ~BrowserCdmCast() override; void Initialize( const ::media::SessionMessageCB& session_message_cb, @@ -49,12 +51,12 @@ class BrowserCdmCast : public ::media::BrowserCdm { const ::media::SessionKeysChangeCB& session_keys_change_cb, const ::media::SessionExpirationUpdateCB& session_expiration_update_cb); - // PlayerTracker implementation. + // ::media::PlayerTracker implementation. int RegisterPlayer(const base::Closure& new_key_cb, const base::Closure& cdm_unset_cb) override; void UnregisterPlayer(int registration_id) override; - // ::media::BrowserCdm implementation: + // ::media::MediaKeys implementation: ::media::CdmContext* GetCdmContext() override; // Returns the decryption context needed to decrypt frames encrypted with @@ -75,6 +77,8 @@ class BrowserCdmCast : public ::media::BrowserCdm { private: friend class BrowserCdmCastUi; + ~BrowserCdmCast() override; + // Allow subclasses to override to provide key sysytem specific // initialization. virtual void InitializeInternal(); @@ -92,24 +96,20 @@ class BrowserCdmCast : public ::media::BrowserCdm { DISALLOW_COPY_AND_ASSIGN(BrowserCdmCast); }; -// BrowserCdm implementation that lives on the UI thread and forwards all calls +// MediaKeys implementation that lives on the UI thread and forwards all calls // to a BrowserCdmCast instance on the CMA thread. This is used to simplify the // UI-CMA threading interaction. -class BrowserCdmCastUi : public ::media::BrowserCdm { +class BrowserCdmCastUi : public ::media::MediaKeys { public: BrowserCdmCastUi( - scoped_ptr<BrowserCdmCast> browser_cdm_cast, + const scoped_refptr<BrowserCdmCast>& browser_cdm_cast, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); - ~BrowserCdmCastUi() override; - - // PlayerTracker implementation: - int RegisterPlayer(const base::Closure& new_key_cb, - const base::Closure& cdm_unset_cb) override; - void UnregisterPlayer(int registration_id) override; BrowserCdmCast* browser_cdm_cast() const; private: + ~BrowserCdmCastUi() override; + // ::media::MediaKeys implementation: void SetServerCertificate( const std::vector<uint8_t>& certificate, @@ -131,7 +131,7 @@ class BrowserCdmCastUi : public ::media::BrowserCdm { scoped_ptr<::media::SimpleCdmPromise> promise) override; ::media::CdmContext* GetCdmContext() override; - scoped_ptr<BrowserCdmCast> browser_cdm_cast_; + scoped_refptr<BrowserCdmCast> browser_cdm_cast_; scoped_refptr<base::SingleThreadTaskRunner> task_runner_; base::ThreadChecker thread_checker_; diff --git a/content/browser/media/android/media_drm_credential_manager.cc b/content/browser/media/android/media_drm_credential_manager.cc index 8e62770..11b7463 100644 --- a/content/browser/media/android/media_drm_credential_manager.cc +++ b/content/browser/media/android/media_drm_credential_manager.cc @@ -85,7 +85,7 @@ void MediaDrmCredentialManager::OnResetCredentialsCompleted( } base::ResetAndReturn(&reset_credentials_cb_).Run(success); - media_drm_bridge_.reset(); + media_drm_bridge_ = nullptr; } // TODO(ddorwin): The key system should be passed in. http://crbug.com/459400 diff --git a/content/browser/media/android/media_drm_credential_manager.h b/content/browser/media/android/media_drm_credential_manager.h index 97bfe80..d5cbb0c 100644 --- a/content/browser/media/android/media_drm_credential_manager.h +++ b/content/browser/media/android/media_drm_credential_manager.h @@ -52,7 +52,7 @@ class MediaDrmCredentialManager { bool ResetCredentialsInternal(SecurityLevel security_level); // The MediaDrmBridge object used to perform the credential reset. - media::ScopedMediaDrmBridgePtr media_drm_bridge_; + scoped_refptr<media::MediaDrmBridge> media_drm_bridge_; // The callback provided by the caller. ResetCredentialsCB reset_credentials_cb_; diff --git a/content/browser/media/cdm/browser_cdm_manager.cc b/content/browser/media/cdm/browser_cdm_manager.cc index 7294ef4..6ba327b 100644 --- a/content/browser/media/cdm/browser_cdm_manager.cc +++ b/content/browser/media/cdm/browser_cdm_manager.cc @@ -19,7 +19,6 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host_observer.h" #include "content/public/browser/web_contents.h" -#include "media/base/browser_cdm.h" #include "media/base/browser_cdm_factory.h" #include "media/base/cdm_promise.h" #include "media/base/limits.h" @@ -30,9 +29,7 @@ namespace content { -using media::BrowserCdm; using media::MediaKeys; -using media::ScopedBrowserCdmPtr; namespace { @@ -222,10 +219,11 @@ bool BrowserCdmManager::OnMessageReceived(const IPC::Message& msg) { return handled; } -media::BrowserCdm* BrowserCdmManager::GetCdm(int render_frame_id, - int cdm_id) const { +scoped_refptr<MediaKeys> BrowserCdmManager::GetCdm(int render_frame_id, + int cdm_id) const { DCHECK(task_runner_->RunsTasksOnCurrentThread()); - return cdm_map_.get(GetId(render_frame_id, cdm_id)); + const auto& iter = cdm_map_.find(GetId(render_frame_id, cdm_id)); + return iter == cdm_map_.end() ? nullptr : iter->second; } void BrowserCdmManager::RenderFrameDeleted(int render_frame_id) { @@ -264,20 +262,19 @@ void BrowserCdmManager::ResolvePromiseWithSession( void BrowserCdmManager::RejectPromise(int render_frame_id, int cdm_id, uint32_t promise_id, - media::MediaKeys::Exception exception, + MediaKeys::Exception exception, uint32_t system_code, const std::string& error_message) { Send(new CdmMsg_RejectPromise(render_frame_id, cdm_id, promise_id, exception, system_code, error_message)); } -void BrowserCdmManager::OnSessionMessage( - int render_frame_id, - int cdm_id, - const std::string& session_id, - media::MediaKeys::MessageType message_type, - const std::vector<uint8>& message, - const GURL& legacy_destination_url) { +void BrowserCdmManager::OnSessionMessage(int render_frame_id, + int cdm_id, + const std::string& session_id, + MediaKeys::MessageType message_type, + const std::vector<uint8>& message, + const GURL& legacy_destination_url) { GURL verified_gurl = legacy_destination_url; if (!verified_gurl.is_valid() && !verified_gurl.is_empty()) { DLOG(WARNING) << "SessionMessage legacy_destination_url is invalid : " @@ -356,7 +353,7 @@ void BrowserCdmManager::OnSetServerCertificate( scoped_ptr<SimplePromise> promise(new SimplePromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; @@ -414,7 +411,7 @@ void BrowserCdmManager::OnCreateSessionAndGenerateRequest( return; } - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { DLOG(WARNING) << "No CDM found for: " << render_frame_id << ", " << cdm_id; promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); @@ -439,7 +436,7 @@ void BrowserCdmManager::OnLoadSession( scoped_ptr<NewSessionPromise> promise(new NewSessionPromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { DLOG(WARNING) << "No CDM found for: " << render_frame_id << ", " << cdm_id; promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); @@ -463,7 +460,7 @@ void BrowserCdmManager::OnUpdateSession(int render_frame_id, scoped_ptr<SimplePromise> promise(new SimplePromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; @@ -493,7 +490,7 @@ void BrowserCdmManager::OnCloseSession(int render_frame_id, scoped_ptr<SimplePromise> promise(new SimplePromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; @@ -511,7 +508,7 @@ void BrowserCdmManager::OnRemoveSession(int render_frame_id, scoped_ptr<SimplePromise> promise(new SimplePromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; @@ -542,7 +539,7 @@ void BrowserCdmManager::AddCdm(int render_frame_id, scoped_ptr<SimplePromise> promise(new SimplePromise( weak_ptr_factory_.GetWeakPtr(), render_frame_id, cdm_id, promise_id)); - ScopedBrowserCdmPtr cdm(media::CreateBrowserCdm( + scoped_refptr<MediaKeys> cdm(media::CreateBrowserCdm( key_system, use_hw_secure_codecs, BROWSER_CDM_MANAGER_CB(OnSessionMessage), BROWSER_CDM_MANAGER_CB(OnSessionClosed), @@ -557,7 +554,7 @@ void BrowserCdmManager::AddCdm(int render_frame_id, } uint64 id = GetId(render_frame_id, cdm_id); - cdm_map_.add(id, cdm.Pass()); + cdm_map_[id] = cdm; cdm_security_origin_map_[id] = security_origin; promise->resolve(); } @@ -566,13 +563,13 @@ void BrowserCdmManager::RemoveAllCdmForFrame(int render_frame_id) { DCHECK(task_runner_->RunsTasksOnCurrentThread()); std::vector<uint64> ids_to_remove; - for (CdmMap::iterator it = cdm_map_.begin(); it != cdm_map_.end(); ++it) { - if (IdBelongsToFrame(it->first, render_frame_id)) - ids_to_remove.push_back(it->first); + for (const auto& entry : cdm_map_) { + if (IdBelongsToFrame(entry.first, render_frame_id)) + ids_to_remove.push_back(entry.first); } - for (size_t i = 0; i < ids_to_remove.size(); ++i) - RemoveCdm(ids_to_remove[i]); + for (const auto& id_to_remove : ids_to_remove) + RemoveCdm(id_to_remove); } void BrowserCdmManager::RemoveCdm(uint64 id) { @@ -653,7 +650,7 @@ void BrowserCdmManager::CreateSessionAndGenerateRequestIfPermitted( return; } - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; @@ -678,7 +675,7 @@ void BrowserCdmManager::LoadSessionIfPermitted( return; } - BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id); + scoped_refptr<MediaKeys> cdm = GetCdm(render_frame_id, cdm_id); if (!cdm) { promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found."); return; diff --git a/content/browser/media/cdm/browser_cdm_manager.h b/content/browser/media/cdm/browser_cdm_manager.h index 857cdd1..fd2168d 100644 --- a/content/browser/media/cdm/browser_cdm_manager.h +++ b/content/browser/media/cdm/browser_cdm_manager.h @@ -11,7 +11,6 @@ #include "base/basictypes.h" #include "base/callback.h" -#include "base/containers/scoped_ptr_hash_map.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "content/common/content_export.h" @@ -20,7 +19,6 @@ #include "content/public/browser/browser_message_filter.h" #include "content/public/common/permission_status.mojom.h" #include "ipc/ipc_message.h" -#include "media/base/browser_cdm.h" #include "media/base/cdm_promise.h" #include "media/base/eme_constants.h" #include "media/base/media_keys.h" @@ -52,7 +50,9 @@ class CONTENT_EXPORT BrowserCdmManager : public BrowserMessageFilter { const IPC::Message& message) override; bool OnMessageReceived(const IPC::Message& message) override; - media::BrowserCdm* GetCdm(int render_frame_id, int cdm_id) const; + // Returns the CDM associated with |render_frame_id| and |cdm_id|. Returns + // null if no such CDM exists. + scoped_refptr<media::MediaKeys> GetCdm(int render_frame_id, int cdm_id) const; // Notifies that the render frame has been deleted so that all CDMs belongs // to this render frame needs to be destroyed as well. This is needed because @@ -196,8 +196,8 @@ class CONTENT_EXPORT BrowserCdmManager : public BrowserMessageFilter { // The key in the following maps is a combination of |render_frame_id| and // |cdm_id|. - // Map of managed BrowserCdms. - typedef base::ScopedPtrHashMap<uint64, media::ScopedBrowserCdmPtr> CdmMap; + // Map of managed CDMs. + typedef std::map<uint64, scoped_refptr<media::MediaKeys>> CdmMap; CdmMap cdm_map_; // Map of CDM's security origin. diff --git a/content/browser/media/media_web_contents_observer.cc b/content/browser/media/media_web_contents_observer.cc index 8e48e68..c17a9ff 100644 --- a/content/browser/media/media_web_contents_observer.cc +++ b/content/browser/media/media_web_contents_observer.cc @@ -141,7 +141,7 @@ void MediaWebContentsObserver::OnSetCdm(RenderFrameHost* render_frame_host, return; } - media::BrowserCdm* cdm = + scoped_refptr<media::MediaKeys> cdm = browser_cdm_manager->GetCdm(render_frame_host->GetRoutingID(), cdm_id); if (!cdm) { NOTREACHED() << "OnSetCdm: CDM not found for " << cdm_id; diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index e4b1109..dee22d3 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc @@ -1087,8 +1087,9 @@ void RenderProcessHostImpl::SendUpdateValueState(unsigned int target, } #if defined(ENABLE_BROWSER_CDMS) -media::BrowserCdm* RenderProcessHostImpl::GetBrowserCdm(int render_frame_id, - int cdm_id) const { +scoped_refptr<media::MediaKeys> RenderProcessHostImpl::GetCdm( + int render_frame_id, + int cdm_id) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); BrowserCdmManager* manager = BrowserCdmManager::FromProcess(GetID()); if (!manager) diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h index 026809a..c9bf7f6 100644 --- a/content/browser/renderer_host/render_process_host_impl.h +++ b/content/browser/renderer_host/render_process_host_impl.h @@ -159,8 +159,8 @@ class CONTENT_EXPORT RenderProcessHostImpl void SendUpdateValueState( unsigned int target, const gpu::ValueState& state) override; #if defined(ENABLE_BROWSER_CDMS) - media::BrowserCdm* GetBrowserCdm(int render_frame_id, - int cdm_id) const override; + scoped_refptr<media::MediaKeys> GetCdm(int render_frame_id, + int cdm_id) const override; #endif // IPC::Sender via RenderProcessHost. diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h index 9ae9b16..46e147a 100644 --- a/content/public/browser/render_process_host.h +++ b/content/public/browser/render_process_host.h @@ -29,7 +29,7 @@ union ValueState; namespace media { class AudioOutputController; -class BrowserCdm; +class MediaKeys; } namespace content { @@ -287,10 +287,10 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender, const GetAudioOutputControllersCallback& callback) const = 0; #if defined(ENABLE_BROWSER_CDMS) - // Returns the ::media::BrowserCdm instance associated with |render_frame_id| - // and |cdm_id|, or nullptr if not found. - virtual media::BrowserCdm* GetBrowserCdm(int render_frame_id, - int cdm_id) const = 0; + // Returns the CDM instance associated with |render_frame_id| and |cdm_id|, + // or nullptr if not found. + virtual scoped_refptr<media::MediaKeys> GetCdm(int render_frame_id, + int cdm_id) const = 0; #endif // Returns the current number of active views in this process. Excludes diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc index e0f9d01..aef164f 100644 --- a/content/public/test/mock_render_process_host.cc +++ b/content/public/test/mock_render_process_host.cc @@ -27,6 +27,10 @@ #include "content/public/browser/render_widget_host_iterator.h" #include "content/public/browser/storage_partition.h" +#if defined(ENABLE_BROWSER_CDMS) +#include "media/base/media_keys.h" +#endif + namespace content { MockRenderProcessHost::MockRenderProcessHost(BrowserContext* browser_context) @@ -282,8 +286,9 @@ void MockRenderProcessHost::SendUpdateValueState( } #if defined(ENABLE_BROWSER_CDMS) -media::BrowserCdm* MockRenderProcessHost::GetBrowserCdm(int render_frame_id, - int cdm_id) const { +scoped_refptr<media::MediaKeys> MockRenderProcessHost::GetCdm( + int render_frame_id, + int cdm_id) const { return nullptr; } #endif diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h index fef3de8..2ff295c 100644 --- a/content/public/test/mock_render_process_host.h +++ b/content/public/test/mock_render_process_host.h @@ -93,8 +93,8 @@ class MockRenderProcessHost : public RenderProcessHost { void SendUpdateValueState( unsigned int target, const gpu::ValueState& state) override; #if defined(ENABLE_BROWSER_CDMS) - media::BrowserCdm* GetBrowserCdm(int render_frame_id, - int cdm_id) const override; + scoped_refptr<media::MediaKeys> GetCdm(int render_frame_id, + int cdm_id) const override; #endif // IPC::Sender via RenderProcessHost. diff --git a/content/renderer/media/crypto/ppapi_decryptor.cc b/content/renderer/media/crypto/ppapi_decryptor.cc index 4b35c99..3e13269 100644 --- a/content/renderer/media/crypto/ppapi_decryptor.cc +++ b/content/renderer/media/crypto/ppapi_decryptor.cc @@ -55,18 +55,17 @@ void PpapiDecryptor::Create( return; } - scoped_ptr<PpapiDecryptor> ppapi_decryptor( + scoped_refptr<PpapiDecryptor> ppapi_decryptor( new PpapiDecryptor(pepper_cdm_wrapper.Pass(), session_message_cb, session_closed_cb, legacy_session_error_cb, session_keys_change_cb, session_expiration_update_cb)); - // PpapiDecryptor ownership passed to the promise, but keep a copy in order - // to call InitializeCdm(). - PpapiDecryptor* ppapi_decryptor_copy = ppapi_decryptor.get(); + // |ppapi_decryptor| ownership is passed to the promise. scoped_ptr<media::CdmInitializedPromise> promise( - new media::CdmInitializedPromise(cdm_created_cb, ppapi_decryptor.Pass())); - ppapi_decryptor_copy->InitializeCdm(key_system, allow_distinctive_identifier, - allow_persistent_state, promise.Pass()); + new media::CdmInitializedPromise(cdm_created_cb, ppapi_decryptor)); + + ppapi_decryptor->InitializeCdm(key_system, allow_distinctive_identifier, + allow_persistent_state, promise.Pass()); } PpapiDecryptor::PpapiDecryptor( diff --git a/content/renderer/media/crypto/ppapi_decryptor.h b/content/renderer/media/crypto/ppapi_decryptor.h index 6660a7b..d21d9c8 100644 --- a/content/renderer/media/crypto/ppapi_decryptor.h +++ b/content/renderer/media/crypto/ppapi_decryptor.h @@ -48,8 +48,6 @@ class PpapiDecryptor : public media::MediaKeys, const media::SessionExpirationUpdateCB& session_expiration_update_cb, const media::CdmCreatedCB& cdm_created_cb); - ~PpapiDecryptor() override; - // media::MediaKeys implementation. void SetServerCertificate( const std::vector<uint8_t>& certificate, @@ -104,6 +102,8 @@ class PpapiDecryptor : public media::MediaKeys, const media::SessionKeysChangeCB& session_keys_change_cb, const media::SessionExpirationUpdateCB& session_expiration_update_cb); + ~PpapiDecryptor() override; + void InitializeCdm(const std::string& key_system, bool allow_distinctive_identifier, bool allow_persistent_state, diff --git a/content/renderer/media/crypto/proxy_media_keys.cc b/content/renderer/media/crypto/proxy_media_keys.cc index f28be12..f64c326 100644 --- a/content/renderer/media/crypto/proxy_media_keys.cc +++ b/content/renderer/media/crypto/proxy_media_keys.cc @@ -29,24 +29,16 @@ void ProxyMediaKeys::Create( const media::SessionExpirationUpdateCB& session_expiration_update_cb, const media::CdmCreatedCB& cdm_created_cb) { DCHECK(manager); - scoped_ptr<ProxyMediaKeys> proxy_media_keys(new ProxyMediaKeys( + scoped_refptr<ProxyMediaKeys> proxy_media_keys(new ProxyMediaKeys( manager, session_message_cb, session_closed_cb, legacy_session_error_cb, session_keys_change_cb, session_expiration_update_cb)); - // ProxyMediaKeys ownership passed to the promise, but keep a copy in order - // to call InitializeCdm(). - ProxyMediaKeys* proxy_media_keys_copy = proxy_media_keys.get(); + // ProxyMediaKeys ownership passed to the promise. scoped_ptr<media::CdmInitializedPromise> promise( - new media::CdmInitializedPromise(cdm_created_cb, - proxy_media_keys.Pass())); - proxy_media_keys_copy->InitializeCdm(key_system, security_origin, - use_hw_secure_codecs, promise.Pass()); -} + new media::CdmInitializedPromise(cdm_created_cb, proxy_media_keys)); -ProxyMediaKeys::~ProxyMediaKeys() { - manager_->DestroyCdm(cdm_id_); - manager_->UnregisterMediaKeys(cdm_id_); - cdm_promise_adapter_.Clear(); + proxy_media_keys->InitializeCdm(key_system, security_origin, + use_hw_secure_codecs, promise.Pass()); } void ProxyMediaKeys::SetServerCertificate( @@ -193,6 +185,12 @@ ProxyMediaKeys::ProxyMediaKeys( cdm_id_ = manager->RegisterMediaKeys(this); } +ProxyMediaKeys::~ProxyMediaKeys() { + manager_->DestroyCdm(cdm_id_); + manager_->UnregisterMediaKeys(cdm_id_); + cdm_promise_adapter_.Clear(); +} + void ProxyMediaKeys::InitializeCdm( const std::string& key_system, const GURL& security_origin, diff --git a/content/renderer/media/crypto/proxy_media_keys.h b/content/renderer/media/crypto/proxy_media_keys.h index b256dfa..66291cd 100644 --- a/content/renderer/media/crypto/proxy_media_keys.h +++ b/content/renderer/media/crypto/proxy_media_keys.h @@ -39,8 +39,6 @@ class ProxyMediaKeys : public media::MediaKeys, public media::CdmContext { const media::SessionExpirationUpdateCB& session_expiration_update_cb, const media::CdmCreatedCB& cdm_created_cb); - ~ProxyMediaKeys() override; - // MediaKeys implementation. void SetServerCertificate( const std::vector<uint8_t>& certificate, @@ -99,6 +97,8 @@ class ProxyMediaKeys : public media::MediaKeys, public media::CdmContext { const media::SessionKeysChangeCB& session_keys_change_cb, const media::SessionExpirationUpdateCB& session_expiration_update_cb); + ~ProxyMediaKeys() override; + void InitializeCdm(const std::string& key_system, const GURL& security_origin, bool use_hw_secure_codecs, diff --git a/content/renderer/media/crypto/render_cdm_factory.cc b/content/renderer/media/crypto/render_cdm_factory.cc index 013bae6..b3bfecf 100644 --- a/content/renderer/media/crypto/render_cdm_factory.cc +++ b/content/renderer/media/crypto/render_cdm_factory.cc @@ -65,11 +65,11 @@ void RenderCdmFactory::Create( // identifiers and persistent state. Once that changes we can sanity check // here that neither is allowed for AesDecryptor, since it does not support // them and should never be configured that way. http://crbug.com/455271 - scoped_ptr<media::MediaKeys> cdm( + scoped_refptr<media::MediaKeys> cdm( new media::AesDecryptor(security_origin, session_message_cb, session_closed_cb, session_keys_change_cb)); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(cdm_created_cb, base::Passed(&cdm), "")); + FROM_HERE, base::Bind(cdm_created_cb, cdm, "")); return; } diff --git a/media/base/BUILD.gn b/media/base/BUILD.gn index d3a3a69..c6705a0 100644 --- a/media/base/BUILD.gn +++ b/media/base/BUILD.gn @@ -240,8 +240,6 @@ source_set("base") { if (enable_browser_cdms) { sources += [ - "browser_cdm.cc", - "browser_cdm.h", "browser_cdm_factory.cc", "browser_cdm_factory.h", ] diff --git a/media/base/android/browser_cdm_factory_android.cc b/media/base/android/browser_cdm_factory_android.cc index 66912bc..9b43379 100644 --- a/media/base/android/browser_cdm_factory_android.cc +++ b/media/base/android/browser_cdm_factory_android.cc @@ -13,7 +13,7 @@ namespace media { -ScopedBrowserCdmPtr BrowserCdmFactoryAndroid::CreateBrowserCdm( +scoped_refptr<MediaKeys> BrowserCdmFactoryAndroid::CreateBrowserCdm( const std::string& key_system, bool use_hw_secure_codecs, const SessionMessageCB& session_message_cb, @@ -23,16 +23,16 @@ ScopedBrowserCdmPtr BrowserCdmFactoryAndroid::CreateBrowserCdm( const SessionExpirationUpdateCB& session_expiration_update_cb) { if (!MediaDrmBridge::IsKeySystemSupported(key_system)) { NOTREACHED() << "Key system not supported unexpectedly: " << key_system; - return ScopedBrowserCdmPtr(); + return nullptr; } - ScopedMediaDrmBridgePtr cdm( + scoped_refptr<MediaDrmBridge> cdm( MediaDrmBridge::Create(key_system, session_message_cb, session_closed_cb, legacy_session_error_cb, session_keys_change_cb, session_expiration_update_cb)); if (!cdm) { NOTREACHED() << "MediaDrmBridge cannot be created for " << key_system; - return ScopedBrowserCdmPtr(); + return nullptr; } if (key_system == kWidevineKeySystem) { @@ -41,7 +41,7 @@ ScopedBrowserCdmPtr BrowserCdmFactoryAndroid::CreateBrowserCdm( : MediaDrmBridge::SECURITY_LEVEL_3; if (!cdm->SetSecurityLevel(security_level)) { DVLOG(1) << "failed to set security level " << security_level; - return ScopedBrowserCdmPtr(); + return nullptr; } } else { // Assume other key systems require hardware-secure codecs and thus do not @@ -50,11 +50,11 @@ ScopedBrowserCdmPtr BrowserCdmFactoryAndroid::CreateBrowserCdm( NOTREACHED() << key_system << " may require use_video_overlay_for_embedded_encrypted_video"; - return ScopedBrowserCdmPtr(); + return nullptr; } } - return cdm.Pass(); + return cdm; } } // namespace media diff --git a/media/base/android/browser_cdm_factory_android.h b/media/base/android/browser_cdm_factory_android.h index df6ca19..babae75 100644 --- a/media/base/android/browser_cdm_factory_android.h +++ b/media/base/android/browser_cdm_factory_android.h @@ -16,7 +16,7 @@ class MEDIA_EXPORT BrowserCdmFactoryAndroid : public BrowserCdmFactory { BrowserCdmFactoryAndroid() {} ~BrowserCdmFactoryAndroid() final {}; - ScopedBrowserCdmPtr CreateBrowserCdm( + scoped_refptr<MediaKeys> CreateBrowserCdm( const std::string& key_system, bool use_hw_secure_codecs, const SessionMessageCB& session_message_cb, diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc index b444b11..a58ac27b 100644 --- a/media/base/android/media_codec_player.cc +++ b/media/base/android/media_codec_player.cc @@ -6,6 +6,7 @@ #include "base/barrier_closure.h" #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/lazy_instance.h" #include "base/logging.h" #include "base/thread_task_runner_handle.h" @@ -49,7 +50,6 @@ MediaCodecPlayer::MediaCodecPlayer( interpolator_(&default_tick_clock_), pending_start_(false), pending_seek_(kNoTimestamp()), - drm_bridge_(nullptr), cdm_registration_id_(0), key_is_required_(false), key_is_added_(false), @@ -99,9 +99,10 @@ MediaCodecPlayer::~MediaCodecPlayer() media_stat_->StopAndReport(GetInterpolatedTime()); - if (drm_bridge_) { + if (cdm_) { DCHECK(cdm_registration_id_); - drm_bridge_->UnregisterPlayer(cdm_registration_id_); + static_cast<MediaDrmBridge*>(cdm_.get()) + ->UnregisterPlayer(cdm_registration_id_); } } @@ -410,7 +411,8 @@ bool MediaCodecPlayer::IsPlayerReady() { return true; } -void MediaCodecPlayer::SetCdm(BrowserCdm* cdm) { +void MediaCodecPlayer::SetCdm(const scoped_refptr<MediaKeys>& cdm) { + DCHECK(cdm); RUN_ON_MEDIA_THREAD(SetCdm, cdm); DVLOG(1) << __FUNCTION__; @@ -423,27 +425,31 @@ void MediaCodecPlayer::SetCdm(BrowserCdm* cdm) { return; } - if (drm_bridge_) { + if (cdm_) { NOTREACHED() << "Currently we do not support resetting CDM."; return; } - DCHECK(cdm); - drm_bridge_ = static_cast<MediaDrmBridge*>(cdm); + cdm_ = cdm; - DCHECK(drm_bridge_); + // Only MediaDrmBridge will be set on MediaCodecPlayer. + MediaDrmBridge* drm_bridge = static_cast<MediaDrmBridge*>(cdm_.get()); - cdm_registration_id_ = drm_bridge_->RegisterPlayer( - base::Bind(&MediaCodecPlayer::OnKeyAdded, media_weak_this_), - base::Bind(&MediaCodecPlayer::OnCdmUnset, media_weak_this_)); + // Register CDM callbacks. The callbacks registered will be posted back to the + // media thread via BindToCurrentLoop. - MediaDrmBridge::MediaCryptoReadyCB cb = BindToCurrentLoop( - base::Bind(&MediaCodecPlayer::OnMediaCryptoReady, media_weak_this_)); + // Since |this| holds a reference to the |cdm_|, by the time the CDM is + // destructed, UnregisterPlayer() must have been called and |this| has been + // destructed as well. So the |cdm_unset_cb| will never have a chance to be + // called. + // TODO(xhwang): Remove |cdm_unset_cb| after it's not used on all platforms. + cdm_registration_id_ = drm_bridge->RegisterPlayer( + BindToCurrentLoop( + base::Bind(&MediaCodecPlayer::OnKeyAdded, media_weak_this_)), + base::Bind(&base::DoNothing)); - // Post back to MediaDrmBridge's default thread. - ui_task_runner_->PostTask(FROM_HERE, - base::Bind(&MediaDrmBridge::SetMediaCryptoReadyCB, - drm_bridge_->WeakPtr(), cb)); + drm_bridge->SetMediaCryptoReadyCB(BindToCurrentLoop( + base::Bind(&MediaCodecPlayer::OnMediaCryptoReady, media_weak_this_))); } // Callbacks from Demuxer. @@ -965,31 +971,6 @@ void MediaCodecPlayer::OnKeyAdded() { } } -void MediaCodecPlayer::OnCdmUnset() { - DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread()); - DVLOG(1) << __FUNCTION__; - - // This comment is copied from MediaSourcePlayer::OnCdmUnset(). - // TODO(xhwang): Currently this is only called during teardown. Support full - // detachment of CDM during playback. This will be needed when we start to - // support setMediaKeys(0) (see http://crbug.com/330324), or when we release - // MediaDrm when the video is paused, or when the device goes to sleep (see - // http://crbug.com/272421). - - if (audio_decoder_) { - audio_decoder_->SetNeedsReconfigure(); - } - - if (video_decoder_) { - video_decoder_->SetProtectedSurfaceRequired(false); - video_decoder_->SetNeedsReconfigure(); - } - - cdm_registration_id_ = 0; - drm_bridge_ = nullptr; - media_crypto_.reset(); -} - // State machine operations, called on Media thread void MediaCodecPlayer::SetState(PlayerState new_state) { diff --git a/media/base/android/media_codec_player.h b/media/base/android/media_codec_player.h index 376c2e4..fff4129 100644 --- a/media/base/android/media_codec_player.h +++ b/media/base/android/media_codec_player.h @@ -157,7 +157,6 @@ namespace media { -class BrowserCdm; class MediaCodecAudioDecoder; class MediaCodecVideoDecoder; @@ -214,7 +213,7 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid, bool CanSeekForward() override; bool CanSeekBackward() override; bool IsPlayerReady() override; - void SetCdm(BrowserCdm* cdm) override; + void SetCdm(const scoped_refptr<MediaKeys>& cdm) override; // DemuxerAndroidClient implementation. void OnDemuxerConfigsAvailable(const DemuxerConfigs& params) override; @@ -300,11 +299,10 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid, // Callbacks from video decoder void OnVideoResolutionChanged(const gfx::Size& size); - // Callbacks from CDM + // Callbacks from MediaDrmBridge. void OnMediaCryptoReady(MediaDrmBridge::JavaObjectPtr media_crypto, bool needs_protected_surface); void OnKeyAdded(); - void OnCdmUnset(); // Operations called from the state machine. void SetState(PlayerState new_state); @@ -395,10 +393,11 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid, // For testing only. DecodersTimeCallback decoders_time_cb_; - // DRM + // Holds a ref-count to the CDM to keep |media_crypto_| valid. + scoped_refptr<MediaKeys> cdm_; + MediaDrmBridge::JavaObjectPtr media_crypto_; - MediaDrmBridge* drm_bridge_; int cdm_registration_id_; // The flag is set when the player receives the error from decoder that the diff --git a/media/base/android/media_drm_bridge.cc b/media/base/android/media_drm_bridge.cc index 2de3f21..47822bf 100644 --- a/media/base/android/media_drm_bridge.cc +++ b/media/base/android/media_drm_bridge.cc @@ -25,7 +25,6 @@ #include "jni/MediaDrmBridge_jni.h" #include "media/base/android/media_client_android.h" #include "media/base/android/media_drm_bridge_delegate.h" -#include "media/base/android/media_task_runner.h" #include "media/base/cdm_key_information.h" #include "widevine_cdm_version.h" // In SHARED_INTERMEDIATE_DIR. @@ -214,37 +213,6 @@ std::string GetSecurityLevelString( } // namespace -MediaDrmBridge::~MediaDrmBridge() { - DVLOG(1) << __FUNCTION__; - - DCHECK(!use_media_thread_ || GetMediaTaskRunner()->BelongsToCurrentThread()); - - player_tracker_.NotifyCdmUnset(); -} - -void MediaDrmBridge::DeleteOnCorrectThread() { - DCHECK(task_runner_->BelongsToCurrentThread()); - DVLOG(1) << __FUNCTION__; - - JNIEnv* env = AttachCurrentThread(); - if (!j_media_drm_.is_null()) - Java_MediaDrmBridge_destroy(env, j_media_drm_.obj()); - - // After the call to Java_MediaDrmBridge_destroy() Java won't call native - // methods anymore, this is ensured by MediaDrmBridge.java. - - // CdmPromiseAdapter must be destroyed on the UI thread. - cdm_promise_adapter_.reset(); - - // Post deletion onto Media thread if we use it. - if (use_media_thread_) { - weak_factory_.InvalidateWeakPtrs(); - GetMediaTaskRunner()->DeleteSoon(FROM_HERE, this); - } else { - delete this; - } -} - // static bool MediaDrmBridge::IsAvailable() { if (base::android::BuildInfo::GetInstance()->sdk_int() < 19) @@ -286,7 +254,7 @@ std::vector<std::string> MediaDrmBridge::GetPlatformKeySystemNames() { } // static -ScopedMediaDrmBridgePtr MediaDrmBridge::Create( +scoped_refptr<MediaDrmBridge> MediaDrmBridge::Create( const std::string& key_system, const SessionMessageCB& session_message_cb, const SessionClosedCB& session_closed_cb, @@ -295,37 +263,32 @@ ScopedMediaDrmBridgePtr MediaDrmBridge::Create( const SessionExpirationUpdateCB& session_expiration_update_cb) { DVLOG(1) << __FUNCTION__; - scoped_ptr<MediaDrmBridge, BrowserCdmDeleter> media_drm_bridge; if (!IsAvailable()) - return media_drm_bridge.Pass(); + return nullptr; UUID scheme_uuid = g_key_system_manager.Get().GetUUID(key_system); if (scheme_uuid.empty()) - return media_drm_bridge.Pass(); + return nullptr; - media_drm_bridge.reset( + scoped_refptr<MediaDrmBridge> media_drm_bridge( new MediaDrmBridge(scheme_uuid, session_message_cb, session_closed_cb, legacy_session_error_cb, session_keys_change_cb, session_expiration_update_cb)); if (media_drm_bridge->j_media_drm_.is_null()) - media_drm_bridge.reset(); + media_drm_bridge = nullptr; - return media_drm_bridge.Pass(); + return media_drm_bridge; } // static -ScopedMediaDrmBridgePtr MediaDrmBridge::CreateWithoutSessionSupport( +scoped_refptr<MediaDrmBridge> MediaDrmBridge::CreateWithoutSessionSupport( const std::string& key_system) { return MediaDrmBridge::Create( key_system, SessionMessageCB(), SessionClosedCB(), LegacySessionErrorCB(), SessionKeysChangeCB(), SessionExpirationUpdateCB()); } -base::WeakPtr<MediaDrmBridge> MediaDrmBridge::WeakPtr() { - return weak_factory_.GetWeakPtr(); -} - void MediaDrmBridge::SetServerCertificate( const std::vector<uint8_t>& certificate, scoped_ptr<media::SimpleCdmPromise> promise) { @@ -452,14 +415,25 @@ CdmContext* MediaDrmBridge::GetCdmContext() { return nullptr; } +void MediaDrmBridge::DeleteOnCorrectThread() const { + DVLOG(1) << __FUNCTION__; + + if (!task_runner_->BelongsToCurrentThread()) { + // When DeleteSoon returns false, |this| will be leaked, which is okay. + task_runner_->DeleteSoon(FROM_HERE, this); + } else { + delete this; + } +} + int MediaDrmBridge::RegisterPlayer(const base::Closure& new_key_cb, const base::Closure& cdm_unset_cb) { - DCHECK(!use_media_thread_ || GetMediaTaskRunner()->BelongsToCurrentThread()); + // |player_tracker_| can be accessed from any thread. return player_tracker_.RegisterPlayer(new_key_cb, cdm_unset_cb); } void MediaDrmBridge::UnregisterPlayer(int registration_id) { - DCHECK(!use_media_thread_ || GetMediaTaskRunner()->BelongsToCurrentThread()); + // |player_tracker_| can be accessed from any thread. player_tracker_.UnregisterPlayer(registration_id); } @@ -527,7 +501,14 @@ ScopedJavaLocalRef<jobject> MediaDrmBridge::GetMediaCrypto() { void MediaDrmBridge::SetMediaCryptoReadyCB( const MediaCryptoReadyCB& media_crypto_ready_cb) { - DCHECK(task_runner_->BelongsToCurrentThread()); + if (!task_runner_->BelongsToCurrentThread()) { + task_runner_->PostTask( + FROM_HERE, + base::Bind(&MediaDrmBridge::SetMediaCryptoReadyCB, + weak_factory_.GetWeakPtr(), media_crypto_ready_cb)); + return; + } + DVLOG(1) << __FUNCTION__; if (media_crypto_ready_cb.is_null()) { @@ -558,25 +539,27 @@ void MediaDrmBridge::OnMediaCryptoReady(JNIEnv* env, jobject j_media_drm) { return; task_runner_->PostTask( - FROM_HERE, base::Bind(&MediaDrmBridge::NotifyMediaCryptoReady, WeakPtr(), + FROM_HERE, base::Bind(&MediaDrmBridge::NotifyMediaCryptoReady, + weak_factory_.GetWeakPtr(), base::ResetAndReturn(&media_crypto_ready_cb_))); } void MediaDrmBridge::OnPromiseResolved(JNIEnv* env, jobject j_media_drm, jint j_promise_id) { - task_runner_->PostTask(FROM_HERE, base::Bind(&MediaDrmBridge::ResolvePromise, - WeakPtr(), j_promise_id)); + task_runner_->PostTask(FROM_HERE, + base::Bind(&MediaDrmBridge::ResolvePromise, + weak_factory_.GetWeakPtr(), j_promise_id)); } void MediaDrmBridge::OnPromiseResolvedWithSession(JNIEnv* env, jobject j_media_drm, jint j_promise_id, jbyteArray j_session_id) { - task_runner_->PostTask( - FROM_HERE, - base::Bind(&MediaDrmBridge::ResolvePromiseWithSession, WeakPtr(), - j_promise_id, GetSessionId(env, j_session_id))); + task_runner_->PostTask(FROM_HERE, + base::Bind(&MediaDrmBridge::ResolvePromiseWithSession, + weak_factory_.GetWeakPtr(), j_promise_id, + GetSessionId(env, j_session_id))); } void MediaDrmBridge::OnPromiseRejected(JNIEnv* env, @@ -585,8 +568,8 @@ void MediaDrmBridge::OnPromiseRejected(JNIEnv* env, jstring j_error_message) { task_runner_->PostTask( FROM_HERE, - base::Bind(&MediaDrmBridge::RejectPromise, WeakPtr(), j_promise_id, - ConvertJavaStringToUTF8(env, j_error_message))); + base::Bind(&MediaDrmBridge::RejectPromise, weak_factory_.GetWeakPtr(), + j_promise_id, ConvertJavaStringToUTF8(env, j_error_message))); } void MediaDrmBridge::OnSessionMessage(JNIEnv* env, @@ -626,7 +609,7 @@ void MediaDrmBridge::OnSessionKeysChange(JNIEnv* env, DVLOG(2) << __FUNCTION__; if (has_additional_usable_key) - NotifyNewKeyOnCorrectThread(); + player_tracker_.NotifyNewKey(); CdmKeysInfo cdm_keys_info; @@ -722,8 +705,6 @@ MediaDrmBridge::MediaDrmBridge( session_expiration_update_cb_(session_expiration_update_cb), cdm_promise_adapter_(new CdmPromiseAdapter()), task_runner_(base::ThreadTaskRunnerHandle::Get()), - use_media_thread_(UseMediaThreadForMediaPlayback()), - media_weak_factory_(this), weak_factory_(this) { DVLOG(1) << __FUNCTION__; @@ -736,6 +717,20 @@ MediaDrmBridge::MediaDrmBridge( env, j_scheme_uuid.obj(), reinterpret_cast<intptr_t>(this))); } +MediaDrmBridge::~MediaDrmBridge() { + DCHECK(task_runner_->BelongsToCurrentThread()); + DVLOG(1) << __FUNCTION__; + + JNIEnv* env = AttachCurrentThread(); + + // After the call to Java_MediaDrmBridge_destroy() Java won't call native + // methods anymore, this is ensured by MediaDrmBridge.java. + if (!j_media_drm_.is_null()) + Java_MediaDrmBridge_destroy(env, j_media_drm_.obj()); + + player_tracker_.NotifyCdmUnset(); +} + // TODO(ddorwin): This is specific to Widevine. http://crbug.com/459400 // static bool MediaDrmBridge::IsSecureDecoderRequired(SecurityLevel security_level) { @@ -752,21 +747,6 @@ MediaDrmBridge::SecurityLevel MediaDrmBridge::GetSecurityLevel() { return GetSecurityLevelFromString(security_level_str); } -void MediaDrmBridge::NotifyNewKeyOnCorrectThread() { - // Repost this method onto the Media thread if |use_media_thread_| is true. - if (use_media_thread_ && !GetMediaTaskRunner()->BelongsToCurrentThread()) { - GetMediaTaskRunner()->PostTask( - FROM_HERE, base::Bind(&MediaDrmBridge::NotifyNewKeyOnCorrectThread, - media_weak_factory_.GetWeakPtr())); - return; - } - - DCHECK(!use_media_thread_ || GetMediaTaskRunner()->BelongsToCurrentThread()); - DVLOG(1) << __FUNCTION__; - - player_tracker_.NotifyNewKey(); -} - void MediaDrmBridge::NotifyMediaCryptoReady(const MediaCryptoReadyCB& cb) { DCHECK(task_runner_->BelongsToCurrentThread()); diff --git a/media/base/android/media_drm_bridge.h b/media/base/android/media_drm_bridge.h index a525f83..90aec56 100644 --- a/media/base/android/media_drm_bridge.h +++ b/media/base/android/media_drm_bridge.h @@ -13,9 +13,10 @@ #include "base/callback.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" -#include "media/base/browser_cdm.h" #include "media/base/cdm_promise_adapter.h" #include "media/base/media_export.h" +#include "media/base/media_keys.h" +#include "media/base/player_tracker.h" #include "media/cdm/player_tracker_impl.h" #include "url/gurl.h" @@ -23,12 +24,15 @@ class GURL; namespace media { -class MediaDrmBridge; +// Implements a CDM using Android MediaDrm API. +// +// Thread Safety: +// +// This class lives on the thread where it is created. All methods must be +// called on the |task_runner_| except for the PlayerTracker methods and +// SetMediaCryptoReadyCB(), which can be called on any thread. -using ScopedMediaDrmBridgePtr = scoped_ptr<MediaDrmBridge, BrowserCdmDeleter>; - -// This class provides DRM services for android EME implementation. -class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { +class MEDIA_EXPORT MediaDrmBridge : public MediaKeys, public PlayerTracker { public: // TODO(ddorwin): These are specific to Widevine. http://crbug.com/459400 enum SecurityLevel { @@ -48,10 +52,6 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { using MediaCryptoReadyCB = base::Callback<void(JavaObjectPtr media_crypto, bool needs_protected_surface)>; - ~MediaDrmBridge() override; - - void DeleteOnCorrectThread() override; - // Checks whether MediaDRM is available. // All other static methods check IsAvailable() internally. There's no need // to check IsAvailable() explicitly before calling them. @@ -75,7 +75,7 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { // Returns a MediaDrmBridge instance if |key_system| is supported, or a NULL // pointer otherwise. // TODO(xhwang): Is it okay not to update session expiration info? - static ScopedMediaDrmBridgePtr Create( + static scoped_refptr<MediaDrmBridge> Create( const std::string& key_system, const SessionMessageCB& session_message_cb, const SessionClosedCB& session_closed_cb, @@ -86,13 +86,10 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { // Returns a MediaDrmBridge instance if |key_system| is supported, or a NULL // otherwise. No session callbacks are provided. This is used when we need to // use MediaDrmBridge without creating any sessions. - static ScopedMediaDrmBridgePtr CreateWithoutSessionSupport( + static scoped_refptr<MediaDrmBridge> CreateWithoutSessionSupport( const std::string& key_system); - // Returns a WeakPtr to be used on the |task_runner_|. - base::WeakPtr<MediaDrmBridge> WeakPtr(); - - // MediaKeys (via BrowserCdm) implementation. + // MediaKeys implementation. void SetServerCertificate( const std::vector<uint8_t>& certificate, scoped_ptr<media::SimpleCdmPromise> promise) override; @@ -112,8 +109,14 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { void RemoveSession(const std::string& session_id, scoped_ptr<media::SimpleCdmPromise> promise) override; CdmContext* GetCdmContext() override; - - // PlayerTracker (via BrowserCdm) implementation. + void DeleteOnCorrectThread() const override; + + // PlayerTracker implementation. Can be called on any thread. + // The registered callbacks will be fired on |task_runner_|. The caller + // should make sure that the callbacks are posted to the correct thread. + // + // Note: RegisterPlayer() should be called before SetMediaCryptoReadyCB() to + // avoid missing any new key notifications. int RegisterPlayer(const base::Closure& new_key_cb, const base::Closure& cdm_unset_cb) override; void UnregisterPlayer(int registration_id) override; @@ -144,8 +147,11 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { // otherwise. base::android::ScopedJavaLocalRef<jobject> GetMediaCrypto(); - // Sets callback which will be called when MediaCrypto is ready. If - // |media_crypto_ready_cb| is null, previously set callback will be cleared. + // Registers a callback which will be called when MediaCrypto is ready. + // Can be called on any thread. Only one callback should be registered. + // The registered callbacks will be fired on |task_runner_|. The caller + // should make sure that the callbacks are posted to the correct thread. + // TODO(xhwang): Move this up to be close to RegisterPlayer(). void SetMediaCryptoReadyCB(const MediaCryptoReadyCB& media_crypto_ready_cb); // All the OnXxx functions below are called from Java. The implementation must @@ -212,6 +218,9 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { void OnResetDeviceCredentialsCompleted(JNIEnv* env, jobject, bool success); private: + // For DeleteSoon() in DeleteOnCorrectThread(). + friend class base::DeleteHelper<MediaDrmBridge>; + MediaDrmBridge(const std::vector<uint8>& scheme_uuid, const SessionMessageCB& session_message_cb, const SessionClosedCB& session_closed_cb, @@ -219,14 +228,13 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { const SessionKeysChangeCB& session_keys_change_cb, const SessionExpirationUpdateCB& session_expiration_update_cb); + ~MediaDrmBridge() override; + static bool IsSecureDecoderRequired(SecurityLevel security_level); // Get the security level of the media. SecurityLevel GetSecurityLevel(); - // A helper method that calls a |player_tracker_| method on correct thread. - void NotifyNewKeyOnCorrectThread(); - // A helper method that calculates the |media_crypto_ready_cb_| arguments and // run this callback. void NotifyMediaCryptoReady(const MediaCryptoReadyCB& cb); @@ -248,24 +256,15 @@ class MEDIA_EXPORT MediaDrmBridge : public BrowserCdm { ResetCredentialsCB reset_credentials_cb_; - // The |player_tracker_| must be accessed by one thread only. It is accessed - // by the Media thread when |use_media_thread_| is true. PlayerTrackerImpl player_tracker_; + // TODO(xhwang): Host a CdmPromiseAdapter directly. No need to use scoped_ptr. scoped_ptr<CdmPromiseAdapter> cdm_promise_adapter_; // Default task runner. scoped_refptr<base::SingleThreadTaskRunner> task_runner_; - // This flag is set when we use media thread for certain callbacks. - const bool use_media_thread_; - // NOTE: Weak pointers must be invalidated before all other member variables. - - // WeakPtrFactory to generate weak pointers to be used on the media thread. - base::WeakPtrFactory<MediaDrmBridge> media_weak_factory_; - - // Default WeakPtrFactory. base::WeakPtrFactory<MediaDrmBridge> weak_factory_; DISALLOW_COPY_AND_ASSIGN(MediaDrmBridge); diff --git a/media/base/android/media_drm_bridge_unittest.cc b/media/base/android/media_drm_bridge_unittest.cc index f215a42..233dc75 100644 --- a/media/base/android/media_drm_bridge_unittest.cc +++ b/media/base/android/media_drm_bridge_unittest.cc @@ -90,7 +90,7 @@ TEST(MediaDrmBridgeTest, CreateWithoutSessionSupport_InvalidKeySystem) { TEST(MediaDrmBridgeTest, SetSecurityLevel_Widevine) { base::MessageLoop message_loop_; - scoped_ptr<MediaDrmBridge, BrowserCdmDeleter> media_drm_bridge = + scoped_refptr<MediaDrmBridge> media_drm_bridge = MediaDrmBridge::CreateWithoutSessionSupport(kWidevineKeySystem); EXPECT_TRUE_IF_WIDEVINE_AVAILABLE(media_drm_bridge); if (!media_drm_bridge) diff --git a/media/base/android/media_player_android.cc b/media/base/android/media_player_android.cc index c08c259..9bde3de 100644 --- a/media/base/android/media_player_android.cc +++ b/media/base/android/media_player_android.cc @@ -42,7 +42,7 @@ GURL MediaPlayerAndroid::GetFirstPartyForCookies() { return GURL(); } -void MediaPlayerAndroid::SetCdm(BrowserCdm* /* cdm */) { +void MediaPlayerAndroid::SetCdm(const scoped_refptr<MediaKeys>& /* cdm */) { // Players that support EME should override this. LOG(ERROR) << "EME not supported on base MediaPlayerAndroid class."; return; diff --git a/media/base/android/media_player_android.h b/media/base/android/media_player_android.h index e24ad6c..24fb599 100644 --- a/media/base/android/media_player_android.h +++ b/media/base/android/media_player_android.h @@ -19,7 +19,7 @@ namespace media { -class BrowserCdm; +class MediaKeys; class MediaPlayerManager; // This class serves as the base class for different media player @@ -78,7 +78,7 @@ class MEDIA_EXPORT MediaPlayerAndroid { virtual GURL GetFirstPartyForCookies(); // Associates the |cdm| with this player. - virtual void SetCdm(BrowserCdm* cdm); + virtual void SetCdm(const scoped_refptr<MediaKeys>& cdm); // Requests playback permission from MediaPlayerManager. // Overridden in MediaCodecPlayer to pass data between threads. diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc index d4408b3..fd52bba 100644 --- a/media/base/android/media_source_player.cc +++ b/media/base/android/media_source_player.cc @@ -11,6 +11,7 @@ #include "base/barrier_closure.h" #include "base/basictypes.h" #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/callback_helpers.h" #include "base/logging.h" #include "base/strings/string_number_conversions.h" @@ -39,7 +40,6 @@ MediaSourcePlayer::MediaSourcePlayer( interpolator_(&default_tick_clock_), doing_browser_seek_(false), pending_seek_(false), - drm_bridge_(NULL), cdm_registration_id_(0), is_waiting_for_key_(false), key_added_while_decode_pending_(false), @@ -69,9 +69,10 @@ MediaSourcePlayer::MediaSourcePlayer( MediaSourcePlayer::~MediaSourcePlayer() { Release(); - DCHECK_EQ(!drm_bridge_, !cdm_registration_id_); - if (drm_bridge_) { - drm_bridge_->UnregisterPlayer(cdm_registration_id_); + DCHECK_EQ(!cdm_, !cdm_registration_id_); + if (cdm_) { + static_cast<MediaDrmBridge*>(cdm_.get()) + ->UnregisterPlayer(cdm_registration_id_); cdm_registration_id_ = 0; } } @@ -267,13 +268,15 @@ void MediaSourcePlayer::OnMediaCryptoReady( bool /* needs_protected_surface */) { // Callback parameters are ignored in this player. They are intended for // MediaCodecPlayer which uses a different threading scheme. - DCHECK(!drm_bridge_->GetMediaCrypto().is_null()); + DCHECK(!static_cast<MediaDrmBridge*>(cdm_.get())->GetMediaCrypto().is_null()); // Retry decoder creation if the decoders are waiting for MediaCrypto. RetryDecoderCreation(true, true); } -void MediaSourcePlayer::SetCdm(BrowserCdm* cdm) { +void MediaSourcePlayer::SetCdm(const scoped_refptr<MediaKeys>& cdm) { + DCHECK(cdm); + // Currently we don't support DRM change during the middle of playback, even // if the player is paused. // TODO(qinmin): support DRM change after playback has started. @@ -283,25 +286,29 @@ void MediaSourcePlayer::SetCdm(BrowserCdm* cdm) { << "This is not well supported!"; } - if (drm_bridge_) { + if (cdm_) { NOTREACHED() << "Currently we do not support resetting CDM."; return; } + cdm_ = cdm; + // Only MediaDrmBridge will be set on MediaSourcePlayer. - drm_bridge_ = static_cast<MediaDrmBridge*>(cdm); + MediaDrmBridge* drm_bridge = static_cast<MediaDrmBridge*>(cdm_.get()); - cdm_registration_id_ = drm_bridge_->RegisterPlayer( + // No need to set |cdm_unset_cb| since |this| holds a reference to the |cdm_|. + cdm_registration_id_ = drm_bridge->RegisterPlayer( base::Bind(&MediaSourcePlayer::OnKeyAdded, weak_this_), - base::Bind(&MediaSourcePlayer::OnCdmUnset, weak_this_)); + base::Bind(&base::DoNothing)); - audio_decoder_job_->SetDrmBridge(drm_bridge_); - video_decoder_job_->SetDrmBridge(drm_bridge_); + audio_decoder_job_->SetDrmBridge(drm_bridge); + video_decoder_job_->SetDrmBridge(drm_bridge); - if (drm_bridge_->GetMediaCrypto().is_null()) { + if (drm_bridge->GetMediaCrypto().is_null()) { + // Use BindToCurrentLoop to avoid reentrancy. MediaDrmBridge::MediaCryptoReadyCB cb = BindToCurrentLoop( base::Bind(&MediaSourcePlayer::OnMediaCryptoReady, weak_this_)); - drm_bridge_->SetMediaCryptoReadyCB(cb); + drm_bridge->SetMediaCryptoReadyCB(cb); return; } @@ -835,18 +842,4 @@ void MediaSourcePlayer::ResumePlaybackAfterKeyAdded() { StartInternal(); } -void MediaSourcePlayer::OnCdmUnset() { - DVLOG(1) << __FUNCTION__; - DCHECK(drm_bridge_); - // TODO(xhwang): Currently this is only called during teardown. Support full - // detachment of CDM during playback. This will be needed when we start to - // support setMediaKeys(0) (see http://crbug.com/330324), or when we release - // MediaDrm when the video is paused, or when the device goes to sleep (see - // http://crbug.com/272421). - audio_decoder_job_->SetDrmBridge(NULL); - video_decoder_job_->SetDrmBridge(NULL); - cdm_registration_id_ = 0; - drm_bridge_ = NULL; -} - } // namespace media diff --git a/media/base/android/media_source_player.h b/media/base/android/media_source_player.h index 7e4d706..0cfcfd0 100644 --- a/media/base/android/media_source_player.h +++ b/media/base/android/media_source_player.h @@ -63,7 +63,7 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid, bool CanSeekForward() override; bool CanSeekBackward() override; bool IsPlayerReady() override; - void SetCdm(BrowserCdm* cdm) override; + void SetCdm(const scoped_refptr<MediaKeys>& cdm) override; // DemuxerAndroidClient implementation. void OnDemuxerConfigsAvailable(const DemuxerConfigs& params) override; @@ -165,9 +165,6 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid, // available. void ResumePlaybackAfterKeyAdded(); - // Called when the CDM is detached. - void OnCdmUnset(); - // Test-only method to setup hook for the completion of the next decode cycle. // This callback state is cleared when it is next run. // Prevent usage creep by only calling this from the @@ -245,7 +242,9 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid, // elapses. base::CancelableClosure decoder_starvation_callback_; - MediaDrmBridge* drm_bridge_; + // Holds a ref-count to the CDM. + scoped_refptr<MediaKeys> cdm_; + int cdm_registration_id_; // No decryption key available to decrypt the encrypted buffer. In this case, diff --git a/media/base/browser_cdm.cc b/media/base/browser_cdm.cc deleted file mode 100644 index 45a2ea2..0000000 --- a/media/base/browser_cdm.cc +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2014 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 "media/base/browser_cdm.h" - -namespace media { - -BrowserCdm::BrowserCdm() { -} - -BrowserCdm::~BrowserCdm() { -} - -// For most subclasses we can delete on the caller thread. -void BrowserCdm::DeleteOnCorrectThread() { - delete this; -} - -} // namespace media diff --git a/media/base/browser_cdm.h b/media/base/browser_cdm.h deleted file mode 100644 index 5e4cb28..0000000 --- a/media/base/browser_cdm.h +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2014 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 MEDIA_BASE_BROWSER_CDM_H_ -#define MEDIA_BASE_BROWSER_CDM_H_ - -#include "base/memory/scoped_ptr.h" -#include "media/base/media_export.h" -#include "media/base/media_keys.h" -#include "media/base/player_tracker.h" - -namespace media { - -// Interface for browser side CDMs. -class MEDIA_EXPORT BrowserCdm : public MediaKeys, public PlayerTracker { - public: - ~BrowserCdm() override; - - // Virtual destructor. For most subclasses we can delete on the caller thread. - virtual void DeleteOnCorrectThread(); - - protected: - BrowserCdm(); - - private: - DISALLOW_COPY_AND_ASSIGN(BrowserCdm); -}; - -struct MEDIA_EXPORT BrowserCdmDeleter { - inline void operator()(BrowserCdm* ptr) const { - ptr->DeleteOnCorrectThread(); - } -}; - -using ScopedBrowserCdmPtr = scoped_ptr<BrowserCdm, BrowserCdmDeleter>; - -} // namespace media - -#endif // MEDIA_BASE_BROWSER_CDM_H_ diff --git a/media/base/browser_cdm_factory.cc b/media/base/browser_cdm_factory.cc index 73fd4c7..69a6a02 100644 --- a/media/base/browser_cdm_factory.cc +++ b/media/base/browser_cdm_factory.cc @@ -21,7 +21,7 @@ void SetBrowserCdmFactory(BrowserCdmFactory* factory) { g_cdm_factory = factory; } -ScopedBrowserCdmPtr CreateBrowserCdm( +scoped_refptr<MediaKeys> CreateBrowserCdm( const std::string& key_system, bool use_hw_secure_codecs, const SessionMessageCB& session_message_cb, @@ -31,10 +31,10 @@ ScopedBrowserCdmPtr CreateBrowserCdm( const SessionExpirationUpdateCB& session_expiration_update_cb) { if (!g_cdm_factory) { #if defined(OS_ANDROID) - SetBrowserCdmFactory(new BrowserCdmFactoryAndroid); + SetBrowserCdmFactory(new BrowserCdmFactoryAndroid()); #else LOG(ERROR) << "Cannot create BrowserCdm: no BrowserCdmFactory available!"; - return ScopedBrowserCdmPtr(); + return nullptr; #endif } diff --git a/media/base/browser_cdm_factory.h b/media/base/browser_cdm_factory.h index 99525a7..8a05a67 100644 --- a/media/base/browser_cdm_factory.h +++ b/media/base/browser_cdm_factory.h @@ -8,18 +8,19 @@ #include <string> #include "base/macros.h" -#include "base/memory/scoped_ptr.h" -#include "media/base/browser_cdm.h" +#include "base/memory/ref_counted.h" #include "media/base/media_export.h" +#include "media/base/media_keys.h" namespace media { +// TODO(xhwang): Merge this with media::CdmFactory. class MEDIA_EXPORT BrowserCdmFactory { public: BrowserCdmFactory() {} virtual ~BrowserCdmFactory() {} - virtual ScopedBrowserCdmPtr CreateBrowserCdm( + virtual scoped_refptr<MediaKeys> CreateBrowserCdm( const std::string& key_system, bool use_hw_secure_codecs, const SessionMessageCB& session_message_cb, @@ -36,12 +37,12 @@ class MEDIA_EXPORT BrowserCdmFactory { // BrowserCdmFactory per process. void SetBrowserCdmFactory(BrowserCdmFactory* factory); -// Creates a BrowserCdm for |key_system|. Returns NULL if the CDM cannot be +// Creates a MediaKeys for |key_system|. Returns NULL if the CDM cannot be // created. // |use_hw_secure_codecs| indicates that the CDM should be configured to use // hardware-secure codecs (for platforms that support it). // TODO(xhwang): Add ifdef for IPC based CDM. -ScopedBrowserCdmPtr MEDIA_EXPORT +scoped_refptr<MediaKeys> MEDIA_EXPORT CreateBrowserCdm(const std::string& key_system, bool use_hw_secure_codecs, const SessionMessageCB& session_message_cb, diff --git a/media/base/cdm_factory.h b/media/base/cdm_factory.h index 4a5cfb7..a120707 100644 --- a/media/base/cdm_factory.h +++ b/media/base/cdm_factory.h @@ -17,7 +17,7 @@ namespace media { // Callback used when CDM is created. |error_message| only used if // MediaKeys is null (i.e. CDM can't be created). -using CdmCreatedCB = base::Callback<void(scoped_ptr<MediaKeys>, +using CdmCreatedCB = base::Callback<void(const scoped_refptr<MediaKeys>&, const std::string& error_message)>; struct CdmConfig; diff --git a/media/base/cdm_initialized_promise.cc b/media/base/cdm_initialized_promise.cc index dcc9c79..d15d514e 100644 --- a/media/base/cdm_initialized_promise.cc +++ b/media/base/cdm_initialized_promise.cc @@ -6,17 +6,17 @@ namespace media { -CdmInitializedPromise::CdmInitializedPromise(const CdmCreatedCB& cdm_created_cb, - scoped_ptr<MediaKeys> cdm) - : cdm_created_cb_(cdm_created_cb), cdm_(cdm.Pass()) { -} +CdmInitializedPromise::CdmInitializedPromise( + const CdmCreatedCB& cdm_created_cb, + const scoped_refptr<MediaKeys>& cdm) + : cdm_created_cb_(cdm_created_cb), cdm_(cdm) {} CdmInitializedPromise::~CdmInitializedPromise() { } void CdmInitializedPromise::resolve() { MarkPromiseSettled(); - cdm_created_cb_.Run(cdm_.Pass(), ""); + cdm_created_cb_.Run(cdm_, ""); } void CdmInitializedPromise::reject(MediaKeys::Exception exception_code, diff --git a/media/base/cdm_initialized_promise.h b/media/base/cdm_initialized_promise.h index bc4a569..f0ca012 100644 --- a/media/base/cdm_initialized_promise.h +++ b/media/base/cdm_initialized_promise.h @@ -5,7 +5,7 @@ #ifndef MEDIA_BASE_CDM_INITIALIZED_PROMISE_H_ #define MEDIA_BASE_CDM_INITIALIZED_PROMISE_H_ -#include "base/memory/scoped_ptr.h" +#include "base/memory/ref_counted.h" #include "media/base/cdm_factory.h" #include "media/base/cdm_promise.h" #include "media/base/media_export.h" @@ -19,7 +19,7 @@ namespace media { class MEDIA_EXPORT CdmInitializedPromise : public SimpleCdmPromise { public: CdmInitializedPromise(const CdmCreatedCB& cdm_created_cb, - scoped_ptr<MediaKeys> cdm); + const scoped_refptr<MediaKeys>& cdm); ~CdmInitializedPromise() override; // SimpleCdmPromise implementation. @@ -30,7 +30,9 @@ class MEDIA_EXPORT CdmInitializedPromise : public SimpleCdmPromise { private: CdmCreatedCB cdm_created_cb_; - scoped_ptr<MediaKeys> cdm_; + + // Holds a ref-count of the CDM. + scoped_refptr<MediaKeys> cdm_; }; } // namespace media diff --git a/media/base/media_keys.cc b/media/base/media_keys.cc index cf5256b..03e7acf 100644 --- a/media/base/media_keys.cc +++ b/media/base/media_keys.cc @@ -10,4 +10,13 @@ MediaKeys::MediaKeys() {} MediaKeys::~MediaKeys() {} +void MediaKeys::DeleteOnCorrectThread() const { + delete this; +} + +// static +void MediaKeysTraits::Destruct(const MediaKeys* media_keys) { + media_keys->DeleteOnCorrectThread(); +} + } // namespace media diff --git a/media/base/media_keys.h b/media/base/media_keys.h index dc4da6c..a84cdf7 100644 --- a/media/base/media_keys.h +++ b/media/base/media_keys.h @@ -10,6 +10,7 @@ #include "base/basictypes.h" #include "base/callback.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "media/base/eme_constants.h" @@ -24,6 +25,7 @@ namespace media { class CdmContext; struct CdmKeyInformation; +struct MediaKeysTraits; template <typename... T> class CdmPromiseTemplate; @@ -32,11 +34,35 @@ typedef CdmPromiseTemplate<std::string> NewSessionCdmPromise; typedef CdmPromiseTemplate<> SimpleCdmPromise; typedef ScopedVector<CdmKeyInformation> CdmKeysInfo; -// Performs media key operations. +// An interface that represents the Content Decryption Module (CDM) in the +// Encrypted Media Extensions (EME) spec in Chromium. +// See http://w3c.github.io/encrypted-media/#cdm // -// All key operations are called on the renderer thread. Therefore, these calls -// should be fast and nonblocking; key events should be fired asynchronously. -class MEDIA_EXPORT MediaKeys{ +// * Ownership +// +// This class is ref-counted. However, a ref-count should only be held by: +// - The owner of the CDM. This is usually some class in the EME stack, e.g. +// CdmSessionAdapter in the render process, or MojoCdmService in a non-render +// process. +// - The media player that uses the CDM, to prevent the CDM from being +// destructed while still being used by the media player. +// +// When binding class methods into callbacks, prefer WeakPtr to using |this| +// directly to avoid having a ref-count held by the callback. +// +// * Thread Safety +// +// Most CDM operations happen on one thread. However, it is not uncommon that +// the media player lives on a different thread and may call into the CDM from +// that thread. For example, if the CDM supports a Decryptor interface, the +// Decryptor methods could be called on a different thread. The CDM +// implementation should make sure it's thread safe for these situations. +// +// TODO(xhwang): Rename MediaKeys to ContentDecryptionModule. See +// http://crbug.com/309237 + +class MEDIA_EXPORT MediaKeys + : public base::RefCountedThreadSafe<MediaKeys, MediaKeysTraits> { public: // Reported to UMA, so never reuse a value! // Must be kept in sync with blink::WebMediaPlayerClient::MediaKeyErrorCode @@ -86,8 +112,6 @@ class MEDIA_EXPORT MediaKeys{ MESSAGE_TYPE_MAX = LICENSE_RELEASE }; - virtual ~MediaKeys(); - // Provides a server certificate to be used to encrypt messages to the // license server. virtual void SetServerCertificate(const std::vector<uint8_t>& certificate, @@ -137,16 +161,30 @@ class MEDIA_EXPORT MediaKeys{ // not used after |this| is destructed. virtual CdmContext* GetCdmContext() = 0; + // Deletes |this| on the correct thread. By default |this| is deleted + // immediately. Override this method if |this| needs to be deleted on a + // specific thread. + virtual void DeleteOnCorrectThread() const; + protected: + friend class base::RefCountedThreadSafe<MediaKeys, MediaKeysTraits>; + MediaKeys(); + virtual ~MediaKeys(); private: DISALLOW_COPY_AND_ASSIGN(MediaKeys); }; -// Key event callbacks. See the spec for details: -// https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-media.html#event-summary +struct MEDIA_EXPORT MediaKeysTraits { + // Destroys |media_keys| on the correct thread. + static void Destruct(const MediaKeys* media_keys); +}; + +// CDM session event callbacks. +// Called when the CDM needs to queue a message event to the session object. +// See http://w3c.github.io/encrypted-media/#dom-evt-message typedef base::Callback<void(const std::string& session_id, MediaKeys::MessageType message_type, const std::vector<uint8_t>& message, @@ -156,19 +194,25 @@ typedef base::Callback<void(const std::string& session_id, // Called when the session specified by |session_id| is closed. Note that the // CDM may close a session at any point, such as in response to a CloseSession() // call, when the session is no longer needed, or when system resources are -// lost. See for details: http://w3c.github.io/encrypted-media/#session-close +// lost. See http://w3c.github.io/encrypted-media/#session-close typedef base::Callback<void(const std::string& session_id)> SessionClosedCB; +// TODO(xhwang): Remove after prefixed EME support is removed. See +// http://crbug.com/249976 typedef base::Callback<void(const std::string& session_id, MediaKeys::Exception exception, uint32_t system_code, const std::string& error_message)> LegacySessionErrorCB; +// Called when there has been a change in the keys in the session or their +// status. See http://w3c.github.io/encrypted-media/#dom-evt-keystatuseschange typedef base::Callback<void(const std::string& session_id, bool has_additional_usable_key, CdmKeysInfo keys_info)> SessionKeysChangeCB; +// Called when the CDM changes the expiration time of a session. +// See http://w3c.github.io/encrypted-media/#update-expiration typedef base::Callback<void(const std::string& session_id, const base::Time& new_expiry_time)> SessionExpirationUpdateCB; diff --git a/media/base/player_tracker.h b/media/base/player_tracker.h index 0ed1b3f..ff41c0a 100644 --- a/media/base/player_tracker.h +++ b/media/base/player_tracker.h @@ -19,7 +19,8 @@ class MEDIA_EXPORT PlayerTracker { // Registers player callbacks with the CDM. // - |new_key_cb| is fired when a new decryption key becomes available. - // - |cdm_unset_cb| is fired when the CDM is detached from the player. + // - |cdm_unset_cb| is fired when the CDM is detached from the player. The + // player should stop using the CDM and release any ref-count to the CDM. // Returns a registration ID which can be used to unregister a player. virtual int RegisterPlayer(const base::Closure& new_key_cb, const base::Closure& cdm_unset_cb) = 0; diff --git a/media/blink/cdm_session_adapter.cc b/media/blink/cdm_session_adapter.cc index 4dcfe96..e260340 100644 --- a/media/blink/cdm_session_adapter.cc +++ b/media/blink/cdm_session_adapter.cc @@ -128,11 +128,10 @@ const std::string& CdmSessionAdapter::GetKeySystemUMAPrefix() const { return key_system_uma_prefix_; } -void CdmSessionAdapter::OnCdmCreated( - const std::string& key_system, - base::TimeTicks start_time, - scoped_ptr<MediaKeys> cdm, - const std::string& error_message) { +void CdmSessionAdapter::OnCdmCreated(const std::string& key_system, + base::TimeTicks start_time, + const scoped_refptr<MediaKeys>& cdm, + const std::string& error_message) { DVLOG(2) << __FUNCTION__; DCHECK(!cdm_); @@ -155,7 +154,7 @@ void CdmSessionAdapter::OnCdmCreated( // Only report time for successful CDM creation. ReportTimeToCreateCdmUMA(base::TimeTicks::Now() - start_time); - cdm_ = cdm.Pass(); + cdm_ = cdm; cdm_created_result_->completeWithContentDecryptionModule( new WebContentDecryptionModuleImpl(this)); diff --git a/media/blink/cdm_session_adapter.h b/media/blink/cdm_session_adapter.h index ac4219e..a6fc2bc 100644 --- a/media/blink/cdm_session_adapter.h +++ b/media/blink/cdm_session_adapter.h @@ -111,7 +111,7 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> { // Callback for CreateCdm(). void OnCdmCreated(const std::string& key_system, base::TimeTicks start_time, - scoped_ptr<MediaKeys> cdm, + const scoped_refptr<MediaKeys>& cdm, const std::string& error_message); // Callbacks for firing session events. @@ -136,7 +136,7 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> { void ReportTimeToCreateCdmUMA(base::TimeDelta cdm_creation_time) const; - scoped_ptr<MediaKeys> cdm_; + scoped_refptr<MediaKeys> cdm_; SessionMap sessions_; diff --git a/media/cdm/aes_decryptor.h b/media/cdm/aes_decryptor.h index e254026..2b5a49e 100644 --- a/media/cdm/aes_decryptor.h +++ b/media/cdm/aes_decryptor.h @@ -37,7 +37,6 @@ class MEDIA_EXPORT AesDecryptor : public MediaKeys, const SessionMessageCB& session_message_cb, const SessionClosedCB& session_closed_cb, const SessionKeysChangeCB& session_keys_change_cb); - ~AesDecryptor() override; // MediaKeys implementation. void SetServerCertificate(const std::vector<uint8_t>& certificate, @@ -116,6 +115,8 @@ class MEDIA_EXPORT AesDecryptor : public MediaKeys, scoped_ptr<SessionIdDecryptionKeyMap>> KeyIdToSessionKeysMap; + ~AesDecryptor() override; + // Creates a DecryptionKey using |key_string| and associates it with |key_id|. // Returns true if successful. bool AddDecryptionKey(const std::string& session_id, diff --git a/media/cdm/aes_decryptor_unittest.cc b/media/cdm/aes_decryptor_unittest.cc index 9896575..2d8b1a6 100644 --- a/media/cdm/aes_decryptor_unittest.cc +++ b/media/cdm/aes_decryptor_unittest.cc @@ -214,13 +214,14 @@ enum PromiseResult { RESOLVED, REJECTED }; class AesDecryptorTest : public testing::Test { public: AesDecryptorTest() - : decryptor_(GURL::EmptyGURL(), - base::Bind(&AesDecryptorTest::OnSessionMessage, - base::Unretained(this)), - base::Bind(&AesDecryptorTest::OnSessionClosed, - base::Unretained(this)), - base::Bind(&AesDecryptorTest::OnSessionKeysChange, - base::Unretained(this))), + : decryptor_( + new AesDecryptor(GURL::EmptyGURL(), + base::Bind(&AesDecryptorTest::OnSessionMessage, + base::Unretained(this)), + base::Bind(&AesDecryptorTest::OnSessionClosed, + base::Unretained(this)), + base::Bind(&AesDecryptorTest::OnSessionKeysChange, + base::Unretained(this)))), decrypt_cb_(base::Bind(&AesDecryptorTest::BufferDecrypted, base::Unretained(this))), original_data_(kOriginalData, kOriginalData + kOriginalDataSize), @@ -233,8 +234,7 @@ class AesDecryptorTest : public testing::Test { iv_(kIv, kIv + arraysize(kIv)), normal_subsample_entries_( kSubsampleEntriesNormal, - kSubsampleEntriesNormal + arraysize(kSubsampleEntriesNormal)) { - } + kSubsampleEntriesNormal + arraysize(kSubsampleEntriesNormal)) {} protected: void OnResolveWithSession(PromiseResult expected_result, @@ -285,9 +285,9 @@ class AesDecryptorTest : public testing::Test { DCHECK(!key_id.empty()); EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsJSONDictionary(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest(MediaKeys::TEMPORARY_SESSION, - EmeInitDataType::WEBM, key_id, - CreateSessionPromise(RESOLVED)); + decryptor_->CreateSessionAndGenerateRequest(MediaKeys::TEMPORARY_SESSION, + EmeInitDataType::WEBM, key_id, + CreateSessionPromise(RESOLVED)); // This expects the promise to be called synchronously, which is the case // for AesDecryptor. return session_id_; @@ -296,7 +296,7 @@ class AesDecryptorTest : public testing::Test { // Closes the session specified by |session_id|. void CloseSession(const std::string& session_id) { EXPECT_CALL(*this, OnSessionClosed(session_id)); - decryptor_.CloseSession(session_id, CreatePromise(RESOLVED)); + decryptor_->CloseSession(session_id, CreatePromise(RESOLVED)); } // Removes the session specified by |session_id|. This should simply do a @@ -305,7 +305,7 @@ class AesDecryptorTest : public testing::Test { // http://crbug.com/249976. void RemoveSession(const std::string& session_id) { EXPECT_CALL(*this, OnSessionClosed(session_id)); - decryptor_.RemoveSession(session_id, CreatePromise(RESOLVED)); + decryptor_->RemoveSession(session_id, CreatePromise(RESOLVED)); } MOCK_METHOD2(OnSessionKeysChangeCalled, @@ -334,9 +334,9 @@ class AesDecryptorTest : public testing::Test { EXPECT_CALL(*this, OnSessionKeysChangeCalled(_, _)).Times(0); } - decryptor_.UpdateSession(session_id, - std::vector<uint8>(key.begin(), key.end()), - CreatePromise(expected_result)); + decryptor_->UpdateSession(session_id, + std::vector<uint8>(key.begin(), key.end()), + CreatePromise(expected_result)); } bool KeysInfoContains(std::vector<uint8> expected) { @@ -380,7 +380,7 @@ class AesDecryptorTest : public testing::Test { break; } - decryptor_.Decrypt(Decryptor::kVideo, encrypted, decrypt_cb_); + decryptor_->Decrypt(Decryptor::kVideo, encrypted, decrypt_cb_); std::vector<uint8> decrypted_text; if (decrypted.get() && decrypted->data_size()) { @@ -413,7 +413,7 @@ class AesDecryptorTest : public testing::Test { const GURL& legacy_destination_url)); MOCK_METHOD1(OnSessionClosed, void(const std::string& session_id)); - AesDecryptor decryptor_; + scoped_refptr<AesDecryptor> decryptor_; AesDecryptor::DecryptCB decrypt_cb_; std::string session_id_; CdmKeysInfo keys_info_; @@ -431,7 +431,7 @@ class AesDecryptorTest : public testing::Test { TEST_F(AesDecryptorTest, CreateSessionWithNullInitData) { EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM, std::vector<uint8>(), CreateSessionPromise(RESOLVED)); } @@ -439,19 +439,19 @@ TEST_F(AesDecryptorTest, CreateSessionWithNullInitData) { TEST_F(AesDecryptorTest, MultipleCreateSession) { EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM, std::vector<uint8>(), CreateSessionPromise(RESOLVED)); EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM, std::vector<uint8>(), CreateSessionPromise(RESOLVED)); EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM, std::vector<uint8>(), CreateSessionPromise(RESOLVED)); } @@ -474,12 +474,12 @@ TEST_F(AesDecryptorTest, CreateSessionWithCencInitData) { #if defined(USE_PROPRIETARY_CODECS) EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsJSONDictionary(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::CENC, std::vector<uint8>(init_data, init_data + arraysize(init_data)), CreateSessionPromise(RESOLVED)); #else - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::CENC, std::vector<uint8>(init_data, init_data + arraysize(init_data)), CreateSessionPromise(REJECTED)); @@ -492,7 +492,7 @@ TEST_F(AesDecryptorTest, CreateSessionWithKeyIdsInitData) { EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsJSONDictionary(), GURL::EmptyGURL())); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::KEYIDS, std::vector<uint8>(init_data, init_data + arraysize(init_data) - 1), CreateSessionPromise(RESOLVED)); @@ -525,7 +525,7 @@ TEST_F(AesDecryptorTest, NoKey) { scoped_refptr<DecoderBuffer> encrypted_buffer = CreateEncryptedBuffer( encrypted_data_, key_id_, iv_, no_subsample_entries_); EXPECT_CALL(*this, BufferDecrypted(AesDecryptor::kNoKey, IsNull())); - decryptor_.Decrypt(Decryptor::kVideo, encrypted_buffer, decrypt_cb_); + decryptor_->Decrypt(Decryptor::kVideo, encrypted_buffer, decrypt_cb_); } TEST_F(AesDecryptorTest, KeyReplacement) { diff --git a/media/cdm/default_cdm_factory.cc b/media/cdm/default_cdm_factory.cc index c025fe9..675cb05 100644 --- a/media/cdm/default_cdm_factory.cc +++ b/media/cdm/default_cdm_factory.cc @@ -43,11 +43,11 @@ void DefaultCdmFactory::Create( return; } - scoped_ptr<MediaKeys> cdm( + scoped_refptr<MediaKeys> cdm( new AesDecryptor(security_origin, session_message_cb, session_closed_cb, session_keys_change_cb)); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(cdm_created_cb, base::Passed(&cdm), "")); + FROM_HERE, base::Bind(cdm_created_cb, cdm, "")); } } // namespace media diff --git a/media/cdm/player_tracker_impl.cc b/media/cdm/player_tracker_impl.cc index e4abd61..98f2050 100644 --- a/media/cdm/player_tracker_impl.cc +++ b/media/cdm/player_tracker_impl.cc @@ -11,25 +11,21 @@ namespace media { PlayerTrackerImpl::PlayerCallbacks::PlayerCallbacks( - base::Closure new_key_cb, - base::Closure cdm_unset_cb) - : new_key_cb(new_key_cb), cdm_unset_cb(cdm_unset_cb) { -} + const base::Closure& new_key_cb, + const base::Closure& cdm_unset_cb) + : new_key_cb(new_key_cb), cdm_unset_cb(cdm_unset_cb) {} PlayerTrackerImpl::PlayerCallbacks::~PlayerCallbacks() { } PlayerTrackerImpl::PlayerTrackerImpl() : next_registration_id_(1) { - // Enable PlayerTrackerImpl to be created on another thread than it will be - // later exclusively used. - thread_checker_.DetachFromThread(); } PlayerTrackerImpl::~PlayerTrackerImpl() {} int PlayerTrackerImpl::RegisterPlayer(const base::Closure& new_key_cb, const base::Closure& cdm_unset_cb) { - DCHECK(thread_checker_.CalledOnValidThread()); + base::AutoLock lock(lock_); int registration_id = next_registration_id_++; DCHECK(!ContainsKey(player_callbacks_map_, registration_id)); player_callbacks_map_.insert(std::make_pair( @@ -38,24 +34,36 @@ int PlayerTrackerImpl::RegisterPlayer(const base::Closure& new_key_cb, } void PlayerTrackerImpl::UnregisterPlayer(int registration_id) { - DCHECK(thread_checker_.CalledOnValidThread()); + base::AutoLock lock(lock_); DCHECK(ContainsKey(player_callbacks_map_, registration_id)) << registration_id; player_callbacks_map_.erase(registration_id); } void PlayerTrackerImpl::NotifyNewKey() { - DCHECK(thread_checker_.CalledOnValidThread()); - std::map<int, PlayerCallbacks>::iterator iter = player_callbacks_map_.begin(); - for (; iter != player_callbacks_map_.end(); ++iter) - iter->second.new_key_cb.Run(); + std::vector<base::Closure> new_key_callbacks; + + { + base::AutoLock lock(lock_); + for (const auto& entry : player_callbacks_map_) + new_key_callbacks.push_back(entry.second.new_key_cb); + } + + for (const auto& cb : new_key_callbacks) + cb.Run(); } void PlayerTrackerImpl::NotifyCdmUnset() { - DCHECK(thread_checker_.CalledOnValidThread()); - std::map<int, PlayerCallbacks>::iterator iter = player_callbacks_map_.begin(); - for (; iter != player_callbacks_map_.end(); ++iter) - iter->second.cdm_unset_cb.Run(); + std::vector<base::Closure> cdm_unset_callbacks; + + { + base::AutoLock lock(lock_); + for (const auto& entry : player_callbacks_map_) + cdm_unset_callbacks.push_back(entry.second.cdm_unset_cb); + } + + for (const auto& cb : cdm_unset_callbacks) + cb.Run(); } } // namespace media diff --git a/media/cdm/player_tracker_impl.h b/media/cdm/player_tracker_impl.h index 6d4cee3..7da1a25 100644 --- a/media/cdm/player_tracker_impl.h +++ b/media/cdm/player_tracker_impl.h @@ -9,15 +9,14 @@ #include "base/basictypes.h" #include "base/callback.h" -#include "base/threading/thread_checker.h" +#include "base/synchronization/lock.h" #include "media/base/media_export.h" #include "media/base/player_tracker.h" namespace media { // A common implementation that can be shared by different PlayerTracker -// implementations. This class is not thread safe and should only be called -// on one thread. +// implementations. This class is thread safe and can be called on any thread. class MEDIA_EXPORT PlayerTrackerImpl : public PlayerTracker { public: PlayerTrackerImpl(); @@ -34,18 +33,20 @@ class MEDIA_EXPORT PlayerTrackerImpl : public PlayerTracker { private: struct PlayerCallbacks { - PlayerCallbacks(base::Closure new_key_cb, base::Closure cdm_unset_cb); + PlayerCallbacks(const base::Closure& new_key_cb, + const base::Closure& cdm_unset_cb); ~PlayerCallbacks(); base::Closure new_key_cb; base::Closure cdm_unset_cb; }; + // Lock used to serialize access to other data members. + base::Lock lock_; + int next_registration_id_; std::map<int, PlayerCallbacks> player_callbacks_map_; - base::ThreadChecker thread_checker_; - DISALLOW_COPY_AND_ASSIGN(PlayerTrackerImpl); }; diff --git a/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc b/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc index 4533f39..f6c7781 100644 --- a/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc +++ b/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc @@ -267,12 +267,12 @@ namespace media { ClearKeyCdm::ClearKeyCdm(ClearKeyCdmHost* host, const std::string& key_system, const GURL& origin) - : decryptor_( + : decryptor_(new AesDecryptor( origin, base::Bind(&ClearKeyCdm::OnSessionMessage, base::Unretained(this)), base::Bind(&ClearKeyCdm::OnSessionClosed, base::Unretained(this)), base::Bind(&ClearKeyCdm::OnSessionKeysChange, - base::Unretained(this))), + base::Unretained(this)))), host_(host), key_system_(key_system), has_received_keys_change_event_for_emulated_loadsession_(false), @@ -311,7 +311,7 @@ void ClearKeyCdm::CreateSessionAndGenerateRequest( base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id))); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( ConvertSessionType(session_type), ConvertInitDataType(init_data_type), std::vector<uint8_t>(init_data, init_data + init_data_size), promise.Pass()); @@ -347,7 +347,7 @@ void ClearKeyCdm::LoadSession(uint32 promise_id, base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id))); - decryptor_.CreateSessionAndGenerateRequest( + decryptor_->CreateSessionAndGenerateRequest( MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM, std::vector<uint8_t>(), promise.Pass()); } @@ -369,7 +369,7 @@ void ClearKeyCdm::UpdateSession(uint32 promise_id, promise_id), base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id))); - decryptor_.UpdateSession( + decryptor_->UpdateSession( web_session_str, std::vector<uint8_t>(response, response + response_size), promise.Pass()); @@ -394,7 +394,7 @@ void ClearKeyCdm::CloseSession(uint32 promise_id, &ClearKeyCdm::OnPromiseResolved, base::Unretained(this), promise_id), base::Bind( &ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id))); - decryptor_.CloseSession(web_session_str, promise.Pass()); + decryptor_->CloseSession(web_session_str, promise.Pass()); } void ClearKeyCdm::RemoveSession(uint32 promise_id, @@ -423,7 +423,7 @@ void ClearKeyCdm::RemoveSession(uint32 promise_id, promise_id), base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id))); - decryptor_.RemoveSession(web_session_str, promise.Pass()); + decryptor_->RemoveSession(web_session_str, promise.Pass()); } void ClearKeyCdm::SetServerCertificate(uint32 promise_id, @@ -671,9 +671,8 @@ cdm::Status ClearKeyCdm::DecryptToMediaDecoderBuffer( media::Decryptor::Status status = media::Decryptor::kError; // The AesDecryptor does not care what the stream type is. Pass kVideo // for both audio and video decryption. - decryptor_.Decrypt( - media::Decryptor::kVideo, - buffer, + decryptor_->Decrypt( + media::Decryptor::kVideo, buffer, base::Bind(&CopyDecryptResults, &status, decrypted_buffer)); if (status == media::Decryptor::kError) @@ -707,9 +706,9 @@ void ClearKeyCdm::LoadLoadableSession() { base::Bind(&ClearKeyCdm::OnLoadSessionUpdated, base::Unretained(this)), base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this), promise_id_for_emulated_loadsession_))); - decryptor_.UpdateSession(session_id_for_emulated_loadsession_, - std::vector<uint8_t>(jwk_set.begin(), jwk_set.end()), - promise.Pass()); + decryptor_->UpdateSession( + session_id_for_emulated_loadsession_, + std::vector<uint8_t>(jwk_set.begin(), jwk_set.end()), promise.Pass()); } void ClearKeyCdm::OnSessionMessage(const std::string& session_id, diff --git a/media/cdm/ppapi/external_clear_key/clear_key_cdm.h b/media/cdm/ppapi/external_clear_key/clear_key_cdm.h index 6418679..70d1584 100644 --- a/media/cdm/ppapi/external_clear_key/clear_key_cdm.h +++ b/media/cdm/ppapi/external_clear_key/clear_key_cdm.h @@ -145,7 +145,7 @@ class ClearKeyCdm : public ClearKeyCdmInterface { // Keep track of the last session created. void SetSessionId(const std::string& session_id); - AesDecryptor decryptor_; + scoped_refptr<AesDecryptor> decryptor_; ClearKeyCdmHost* host_; diff --git a/media/cdm/proxy_decryptor.cc b/media/cdm/proxy_decryptor.cc index 321a4a2c..b42666a 100644 --- a/media/cdm/proxy_decryptor.cc +++ b/media/cdm/proxy_decryptor.cc @@ -60,7 +60,7 @@ ProxyDecryptor::ProxyDecryptor(MediaPermission* media_permission, ProxyDecryptor::~ProxyDecryptor() { // Destroy the decryptor explicitly before destroying the plugin. - media_keys_.reset(); + media_keys_ = nullptr; } void ProxyDecryptor::CreateCdm(CdmFactory* cdm_factory, @@ -96,7 +96,7 @@ void ProxyDecryptor::CreateCdm(CdmFactory* cdm_factory, void ProxyDecryptor::OnCdmCreated(const std::string& key_system, const GURL& security_origin, const CdmContextReadyCB& cdm_context_ready_cb, - scoped_ptr<MediaKeys> cdm, + const scoped_refptr<MediaKeys>& cdm, const std::string& /* error_message */) { is_creating_cdm_ = false; @@ -106,7 +106,7 @@ void ProxyDecryptor::OnCdmCreated(const std::string& key_system, key_system_ = key_system; security_origin_ = security_origin; is_clear_key_ = IsClearKey(key_system) || IsExternalClearKey(key_system); - media_keys_ = cdm.Pass(); + media_keys_ = cdm; cdm_context_ready_cb.Run(media_keys_->GetCdmContext()); } diff --git a/media/cdm/proxy_decryptor.h b/media/cdm/proxy_decryptor.h index c7f60b9..bcf34c2 100644 --- a/media/cdm/proxy_decryptor.h +++ b/media/cdm/proxy_decryptor.h @@ -10,6 +10,7 @@ #include "base/basictypes.h" #include "base/containers/hash_tables.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" @@ -77,7 +78,7 @@ class MEDIA_EXPORT ProxyDecryptor { void OnCdmCreated(const std::string& key_system, const GURL& security_origin, const CdmContextReadyCB& cdm_context_ready_cb, - scoped_ptr<MediaKeys> cdm, + const scoped_refptr<MediaKeys>& cdm, const std::string& error_message); void GenerateKeyRequestInternal(EmeInitDataType init_data_type, @@ -129,7 +130,7 @@ class MEDIA_EXPORT ProxyDecryptor { bool is_creating_cdm_; // The real MediaKeys that manages key operations for the ProxyDecryptor. - scoped_ptr<MediaKeys> media_keys_; + scoped_refptr<MediaKeys> media_keys_; #if defined(OS_CHROMEOS) || defined(OS_ANDROID) MediaPermission* media_permission_; diff --git a/media/media.gyp b/media/media.gyp index 58d592b..9acf41e 100644 --- a/media/media.gyp +++ b/media/media.gyp @@ -715,8 +715,6 @@ }], ['enable_browser_cdms==1', { 'sources': [ - 'base/browser_cdm.cc', - 'base/browser_cdm.h', 'base/browser_cdm_factory.cc', 'base/browser_cdm_factory.h', ], diff --git a/media/mojo/services/mojo_cdm.cc b/media/mojo/services/mojo_cdm.cc index 218dda4..00eb28a 100644 --- a/media/mojo/services/mojo_cdm.cc +++ b/media/mojo/services/mojo_cdm.cc @@ -35,18 +35,17 @@ void MojoCdm::Create( const media::SessionKeysChangeCB& session_keys_change_cb, const media::SessionExpirationUpdateCB& session_expiration_update_cb, const media::CdmCreatedCB& cdm_created_cb) { - scoped_ptr<MojoCdm> mojo_cdm( + scoped_refptr<MojoCdm> mojo_cdm( new MojoCdm(remote_cdm.Pass(), session_message_cb, session_closed_cb, legacy_session_error_cb, session_keys_change_cb, session_expiration_update_cb)); - // |mojo_cdm|'s ownership will be passed to the promise. Get a raw pointer - // here in order to call Initialize(). - MojoCdm* mojo_cdm_ptr = mojo_cdm.get(); + // |mojo_cdm| ownership is passed to the promise. scoped_ptr<CdmInitializedPromise> promise( - new CdmInitializedPromise(cdm_created_cb, mojo_cdm.Pass())); - mojo_cdm_ptr->InitializeCdm(key_system, security_origin, cdm_config, - promise.Pass()); + new CdmInitializedPromise(cdm_created_cb, mojo_cdm)); + + mojo_cdm->InitializeCdm(key_system, security_origin, cdm_config, + promise.Pass()); } MojoCdm::MojoCdm(interfaces::ContentDecryptionModulePtr remote_cdm, diff --git a/media/mojo/services/mojo_cdm.h b/media/mojo/services/mojo_cdm.h index 0cb2ee9..57369ebe 100644 --- a/media/mojo/services/mojo_cdm.h +++ b/media/mojo/services/mojo_cdm.h @@ -41,8 +41,6 @@ class MojoCdm : public MediaKeys, const media::SessionExpirationUpdateCB& session_expiration_update_cb, const media::CdmCreatedCB& cdm_created_cb); - ~MojoCdm() final; - // MediaKeys implementation. void SetServerCertificate(const std::vector<uint8_t>& certificate, scoped_ptr<SimpleCdmPromise> promise) final; @@ -75,6 +73,8 @@ class MojoCdm : public MediaKeys, const SessionKeysChangeCB& session_keys_change_cb, const SessionExpirationUpdateCB& session_expiration_update_cb); + ~MojoCdm() final; + void InitializeCdm(const std::string& key_system, const GURL& security_origin, const media::CdmConfig& cdm_config, diff --git a/media/mojo/services/mojo_cdm_service.cc b/media/mojo/services/mojo_cdm_service.cc index c30a4f7..4a16387 100644 --- a/media/mojo/services/mojo_cdm_service.cc +++ b/media/mojo/services/mojo_cdm_service.cc @@ -140,7 +140,7 @@ CdmContext* MojoCdmService::GetCdmContext() { } void MojoCdmService::OnCdmCreated(scoped_ptr<CdmIdMojoCdmPromise> promise, - scoped_ptr<MediaKeys> cdm, + const scoped_refptr<MediaKeys>& cdm, const std::string& error_message) { // TODO(xhwang): This should not happen when KeySystemInfo is properly // populated. See http://crbug.com/469366 @@ -149,7 +149,7 @@ void MojoCdmService::OnCdmCreated(scoped_ptr<CdmIdMojoCdmPromise> promise, return; } - cdm_ = cdm.Pass(); + cdm_ = cdm; cdm_id_ = next_cdm_id_++; context_->RegisterCdm(cdm_id_, this); promise->resolve(cdm_id_); diff --git a/media/mojo/services/mojo_cdm_service.h b/media/mojo/services/mojo_cdm_service.h index 0f974da..c669a74 100644 --- a/media/mojo/services/mojo_cdm_service.h +++ b/media/mojo/services/mojo_cdm_service.h @@ -7,6 +7,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "media/base/media_keys.h" @@ -76,7 +77,7 @@ class MojoCdmService : public interfaces::ContentDecryptionModule { private: // Callback for CdmFactory::Create(). void OnCdmCreated(scoped_ptr<MojoCdmPromise<int>> promise, - scoped_ptr<MediaKeys> cdm, + const scoped_refptr<MediaKeys>& cdm, const std::string& error_message); // Callbacks for firing session events. @@ -105,7 +106,7 @@ class MojoCdmService : public interfaces::ContentDecryptionModule { mojo::ServiceProvider* service_provider_; CdmFactory* cdm_factory_; - scoped_ptr<MediaKeys> cdm_; + scoped_refptr<MediaKeys> cdm_; // Set to a valid CDM ID if the |cdm_| is successfully created. int cdm_id_; diff --git a/media/test/pipeline_integration_test.cc b/media/test/pipeline_integration_test.cc index 9f979ce..b62b5cb 100644 --- a/media/test/pipeline_integration_test.cc +++ b/media/test/pipeline_integration_test.cc @@ -4,6 +4,7 @@ #include "base/bind.h" #include "base/command_line.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/stl_util.h" #include "base/strings/string_split.h" @@ -202,14 +203,15 @@ class FakeEncryptedMedia { }; FakeEncryptedMedia(AppBase* app) - : decryptor_(GURL::EmptyGURL(), - base::Bind(&FakeEncryptedMedia::OnSessionMessage, - base::Unretained(this)), - base::Bind(&FakeEncryptedMedia::OnSessionClosed, - base::Unretained(this)), - base::Bind(&FakeEncryptedMedia::OnSessionKeysChange, - base::Unretained(this))), - cdm_context_(&decryptor_), + : decryptor_(new AesDecryptor( + GURL::EmptyGURL(), + base::Bind(&FakeEncryptedMedia::OnSessionMessage, + base::Unretained(this)), + base::Bind(&FakeEncryptedMedia::OnSessionClosed, + base::Unretained(this)), + base::Bind(&FakeEncryptedMedia::OnSessionKeysChange, + base::Unretained(this)))), + cdm_context_(decryptor_.get()), app_(app) {} CdmContext* GetCdmContext() { return &cdm_context_; } @@ -220,7 +222,7 @@ class FakeEncryptedMedia { const std::vector<uint8_t>& message, const GURL& legacy_destination_url) { app_->OnSessionMessage(session_id, message_type, message, - legacy_destination_url, &decryptor_); + legacy_destination_url, decryptor_.get()); } void OnSessionClosed(const std::string& session_id) { @@ -244,7 +246,7 @@ class FakeEncryptedMedia { void OnEncryptedMediaInitData(EmeInitDataType init_data_type, const std::vector<uint8_t>& init_data) { - app_->OnEncryptedMediaInitData(init_data_type, init_data, &decryptor_); + app_->OnEncryptedMediaInitData(init_data_type, init_data, decryptor_.get()); } private: @@ -259,7 +261,7 @@ class FakeEncryptedMedia { Decryptor* decryptor_; }; - AesDecryptor decryptor_; + scoped_refptr<AesDecryptor> decryptor_; TestCdmContext cdm_context_; scoped_ptr<AppBase> app_; }; |