Commit 4fb387e0 authored by Xiaohan Wang's avatar Xiaohan Wang Committed by Commit Bot

media: Refactor WebContentDecryptionModule creation

Current WebContentDecryptionModuleImpl and CdmSessionAdapter take a
blink::WebContentDecryptionModuleResult during CDM creation. There are a
few issues with this:
1. Bad encapsulation, e.g. CompleteWithSession() is exposed to these
   two classes.
2. Hard to test: There's no easy way to create one |result| for testing.
   See Bug.
3. Readability: Working with the result directly ends up with more code,
   e.g. kWebContentDecryptionModuleExceptionNotSupportedError.

Bug: 1018832
Test: No functionality change
Change-Id: I9b1ec2662873d9da6797133355b1a3261145feac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884277Reviewed-by: default avatarJohn Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710617}
parent 481fb0c1
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include "media/base/cdm_key_information.h" #include "media/base/cdm_key_information.h"
#include "media/base/cdm_promise.h" #include "media/base/cdm_promise.h"
#include "media/base/key_systems.h" #include "media/base/key_systems.h"
#include "media/blink/webcontentdecryptionmodule_impl.h"
#include "media/blink/webcontentdecryptionmodulesession_impl.h" #include "media/blink/webcontentdecryptionmodulesession_impl.h"
#include "media/cdm/cdm_context_ref_impl.h" #include "media/cdm/cdm_context_ref_impl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -36,12 +35,11 @@ CdmSessionAdapter::CdmSessionAdapter() : trace_id_(0) {} ...@@ -36,12 +35,11 @@ CdmSessionAdapter::CdmSessionAdapter() : trace_id_(0) {}
CdmSessionAdapter::~CdmSessionAdapter() = default; CdmSessionAdapter::~CdmSessionAdapter() = default;
void CdmSessionAdapter::CreateCdm( void CdmSessionAdapter::CreateCdm(CdmFactory* cdm_factory,
CdmFactory* cdm_factory, const std::string& key_system,
const std::string& key_system, const url::Origin& security_origin,
const url::Origin& security_origin, const CdmConfig& cdm_config,
const CdmConfig& cdm_config, WebCdmCreatedCB web_cdm_created_cb) {
std::unique_ptr<blink::WebContentDecryptionModuleResult> result) {
TRACE_EVENT_ASYNC_BEGIN0("media", "CdmSessionAdapter::CreateCdm", TRACE_EVENT_ASYNC_BEGIN0("media", "CdmSessionAdapter::CreateCdm",
++trace_id_); ++trace_id_);
...@@ -52,8 +50,8 @@ void CdmSessionAdapter::CreateCdm( ...@@ -52,8 +50,8 @@ void CdmSessionAdapter::CreateCdm(
// |this| instead of |weak_this| to prevent |this| from being destructed. // |this| instead of |weak_this| to prevent |this| from being destructed.
base::WeakPtr<CdmSessionAdapter> weak_this = weak_ptr_factory_.GetWeakPtr(); base::WeakPtr<CdmSessionAdapter> weak_this = weak_ptr_factory_.GetWeakPtr();
DCHECK(!cdm_created_result_); DCHECK(!web_cdm_created_cb_);
cdm_created_result_ = std::move(result); web_cdm_created_cb_ = std::move(web_cdm_created_cb);
cdm_factory->Create( cdm_factory->Create(
key_system, security_origin, cdm_config, key_system, security_origin, cdm_config,
...@@ -182,10 +180,7 @@ void CdmSessionAdapter::OnCdmCreated( ...@@ -182,10 +180,7 @@ void CdmSessionAdapter::OnCdmCreated(
cdm ? true : false); cdm ? true : false);
if (!cdm) { if (!cdm) {
cdm_created_result_->CompleteWithError( std::move(web_cdm_created_cb_).Run(nullptr, error_message);
blink::kWebContentDecryptionModuleExceptionNotSupportedError, 0,
blink::WebString::FromUTF8(error_message));
cdm_created_result_.reset();
return; return;
} }
...@@ -200,9 +195,8 @@ void CdmSessionAdapter::OnCdmCreated( ...@@ -200,9 +195,8 @@ void CdmSessionAdapter::OnCdmCreated(
cdm_ = cdm; cdm_ = cdm;
cdm_created_result_->CompleteWithContentDecryptionModule( std::move(web_cdm_created_cb_)
new WebContentDecryptionModuleImpl(this)); .Run(new WebContentDecryptionModuleImpl(this), "");
cdm_created_result_.reset();
} }
void CdmSessionAdapter::OnSessionMessage(const std::string& session_id, void CdmSessionAdapter::OnSessionMessage(const std::string& session_id,
......
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "media/base/cdm_config.h" #include "media/base/cdm_config.h"
#include "media/base/content_decryption_module.h" #include "media/base/content_decryption_module.h"
#include "third_party/blink/public/platform/web_content_decryption_module_result.h" #include "media/blink/webcontentdecryptionmodule_impl.h"
#include "third_party/blink/public/platform/web_content_decryption_module_session.h" #include "third_party/blink/public/platform/web_content_decryption_module_session.h"
namespace url { namespace url {
...@@ -42,12 +42,11 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> { ...@@ -42,12 +42,11 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> {
// Creates the CDM for |key_system| using |cdm_factory| and returns the result // Creates the CDM for |key_system| using |cdm_factory| and returns the result
// via |result|. // via |result|.
void CreateCdm( void CreateCdm(CdmFactory* cdm_factory,
CdmFactory* cdm_factory, const std::string& key_system,
const std::string& key_system, const url::Origin& security_origin,
const url::Origin& security_origin, const CdmConfig& cdm_config,
const CdmConfig& cdm_config, WebCdmCreatedCB web_cdm_created_cb);
std::unique_ptr<blink::WebContentDecryptionModuleResult> result);
// Provides a server certificate to be used to encrypt messages to the // Provides a server certificate to be used to encrypt messages to the
// license server. // license server.
...@@ -159,7 +158,7 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> { ...@@ -159,7 +158,7 @@ class CdmSessionAdapter : public base::RefCounted<CdmSessionAdapter> {
// OnCdmCreated() call. // OnCdmCreated() call.
uint32_t trace_id_; uint32_t trace_id_;
std::unique_ptr<blink::WebContentDecryptionModuleResult> cdm_created_result_; WebCdmCreatedCB web_cdm_created_cb_;
// NOTE: Weak pointers must be invalidated before all other member variables. // NOTE: Weak pointers must be invalidated before all other member variables.
base::WeakPtrFactory<CdmSessionAdapter> weak_ptr_factory_{this}; base::WeakPtrFactory<CdmSessionAdapter> weak_ptr_factory_{this};
......
...@@ -72,7 +72,7 @@ void WebContentDecryptionModuleImpl::Create( ...@@ -72,7 +72,7 @@ void WebContentDecryptionModuleImpl::Create(
const base::string16& key_system, const base::string16& key_system,
const blink::WebSecurityOrigin& security_origin, const blink::WebSecurityOrigin& security_origin,
const CdmConfig& cdm_config, const CdmConfig& cdm_config,
std::unique_ptr<blink::WebContentDecryptionModuleResult> result) { WebCdmCreatedCB web_cdm_created_cb) {
DCHECK(!security_origin.IsNull()); DCHECK(!security_origin.IsNull());
DCHECK(!key_system.empty()); DCHECK(!key_system.empty());
...@@ -80,9 +80,7 @@ void WebContentDecryptionModuleImpl::Create( ...@@ -80,9 +80,7 @@ void WebContentDecryptionModuleImpl::Create(
// Chromium only supports ASCII key systems. // Chromium only supports ASCII key systems.
if (!base::IsStringASCII(key_system)) { if (!base::IsStringASCII(key_system)) {
NOTREACHED(); NOTREACHED();
result->CompleteWithError( std::move(web_cdm_created_cb).Run(nullptr, "Invalid keysystem.");
blink::kWebContentDecryptionModuleExceptionNotSupportedError, 0,
"Invalid keysystem.");
return; return;
} }
...@@ -92,27 +90,24 @@ void WebContentDecryptionModuleImpl::Create( ...@@ -92,27 +90,24 @@ void WebContentDecryptionModuleImpl::Create(
key_system_ascii)) { key_system_ascii)) {
std::string message = std::string message =
"Keysystem '" + key_system_ascii + "' is not supported."; "Keysystem '" + key_system_ascii + "' is not supported.";
result->CompleteWithError( std::move(web_cdm_created_cb).Run(nullptr, message);
blink::kWebContentDecryptionModuleExceptionNotSupportedError, 0,
blink::WebString::FromUTF8(message));
return; return;
} }
// If unique security origin, don't try to create the CDM. // If unique security origin, don't try to create the CDM.
if (security_origin.IsUnique() || security_origin.ToString() == "null") { if (security_origin.IsUnique() || security_origin.ToString() == "null") {
result->CompleteWithError( std::move(web_cdm_created_cb)
blink::kWebContentDecryptionModuleExceptionNotSupportedError, 0, .Run(nullptr, "EME use is not allowed on unique origins.");
"EME use is not allowed on unique origins.");
return; return;
} }
// CdmSessionAdapter::CreateCdm() will keep a reference to |adapter|. Then // CdmSessionAdapter::CreateCdm() will keep a reference to |adapter|. Then
// if WebContentDecryptionModuleImpl is successfully created (returned in // if WebContentDecryptionModuleImpl is successfully created (returned in
// |result|), it will keep a reference to |adapter|. Otherwise, |adapter| will // |web_cdm_created_cb|), it will keep a reference to |adapter|. Otherwise,
// be destructed. // |adapter| will be destructed.
scoped_refptr<CdmSessionAdapter> adapter(new CdmSessionAdapter()); scoped_refptr<CdmSessionAdapter> adapter(new CdmSessionAdapter());
adapter->CreateCdm(cdm_factory, key_system_ascii, security_origin, cdm_config, adapter->CreateCdm(cdm_factory, key_system_ascii, security_origin, cdm_config,
std::move(result)); std::move(web_cdm_created_cb));
} }
WebContentDecryptionModuleImpl::WebContentDecryptionModuleImpl( WebContentDecryptionModuleImpl::WebContentDecryptionModuleImpl(
......
...@@ -11,13 +11,13 @@ ...@@ -11,13 +11,13 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "media/base/cdm_config.h" #include "media/base/cdm_config.h"
#include "media/blink/media_blink_export.h" #include "media/blink/media_blink_export.h"
#include "third_party/blink/public/platform/web_content_decryption_module.h" #include "third_party/blink/public/platform/web_content_decryption_module.h"
#include "third_party/blink/public/platform/web_content_decryption_module_result.h"
namespace blink { namespace blink {
class WebSecurityOrigin; class WebSecurityOrigin;
...@@ -30,15 +30,18 @@ class CdmContextRef; ...@@ -30,15 +30,18 @@ class CdmContextRef;
class CdmFactory; class CdmFactory;
class CdmSessionAdapter; class CdmSessionAdapter;
using WebCdmCreatedCB =
base::OnceCallback<void(blink::WebContentDecryptionModule* cdm,
const std::string& error_message)>;
class MEDIA_BLINK_EXPORT WebContentDecryptionModuleImpl class MEDIA_BLINK_EXPORT WebContentDecryptionModuleImpl
: public blink::WebContentDecryptionModule { : public blink::WebContentDecryptionModule {
public: public:
static void Create( static void Create(CdmFactory* cdm_factory,
CdmFactory* cdm_factory, const base::string16& key_system,
const base::string16& key_system, const blink::WebSecurityOrigin& security_origin,
const blink::WebSecurityOrigin& security_origin, const CdmConfig& cdm_config,
const CdmConfig& cdm_config, WebCdmCreatedCB web_cdm_created_cb);
std::unique_ptr<blink::WebContentDecryptionModuleResult> result);
~WebContentDecryptionModuleImpl() override; ~WebContentDecryptionModuleImpl() override;
......
...@@ -32,6 +32,24 @@ namespace { ...@@ -32,6 +32,24 @@ namespace {
const char kKeySystemSupportUMAPrefix[] = const char kKeySystemSupportUMAPrefix[] =
"Media.EME.RequestMediaKeySystemAccess."; "Media.EME.RequestMediaKeySystemAccess.";
// A helper function to complete blink::WebContentDecryptionModuleResult. Used
// to convert blink::WebContentDecryptionModuleResult to a callback.
void CompleteWebContentDecryptionModuleResult(
std::unique_ptr<blink::WebContentDecryptionModuleResult> result,
blink::WebContentDecryptionModule* cdm,
const std::string& error_message) {
DCHECK(result);
if (!cdm) {
result->CompleteWithError(
blink::kWebContentDecryptionModuleExceptionNotSupportedError, 0,
blink::WebString::FromUTF8(error_message));
return;
}
result->CompleteWithContentDecryptionModule(cdm);
}
} // namespace } // namespace
// Report usage of key system to UMA. There are 2 different counts logged: // Report usage of key system to UMA. There are 2 different counts logged:
...@@ -107,9 +125,10 @@ void WebEncryptedMediaClientImpl::CreateCdm( ...@@ -107,9 +125,10 @@ void WebEncryptedMediaClientImpl::CreateCdm(
const blink::WebSecurityOrigin& security_origin, const blink::WebSecurityOrigin& security_origin,
const CdmConfig& cdm_config, const CdmConfig& cdm_config,
std::unique_ptr<blink::WebContentDecryptionModuleResult> result) { std::unique_ptr<blink::WebContentDecryptionModuleResult> result) {
WebContentDecryptionModuleImpl::Create(cdm_factory_, key_system.Utf16(), WebContentDecryptionModuleImpl::Create(
security_origin, cdm_config, cdm_factory_, key_system.Utf16(), security_origin, cdm_config,
std::move(result)); base::BindOnce(&CompleteWebContentDecryptionModuleResult,
std::move(result)));
} }
void WebEncryptedMediaClientImpl::OnRequestSucceeded( void WebEncryptedMediaClientImpl::OnRequestSucceeded(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment