Update MediaDrmStorage::Initialize() to indicate success properly

Rather than relying on an empty origin ID to indicate failure to
initialize, return a bool parameter that specifies it properly.
This will allow future changes that support pre-provisioned origin
IDs to handle the case where an empty origin ID is used temporarily.

BUG=917527
TEST=components_unittests pass

Change-Id: I1ee06f60af49ffa08dc16ed0d5def5fc0be7351e
Reviewed-on: https://chromium-review.googlesource.com/c/1453100
Commit-Queue: John Rummell <jrummell@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630088}

components/cdm/browser/media_drm_storage_impl.cc

@@ -656,7 +656,7 @@ MediaDrmStorageImpl::~MediaDrmStorageImpl() {
   DVLOG(1) << __func__;
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (init_cb_)
-    std::move(init_cb_).Run(base::UnguessableToken::Null());
+    std::move(init_cb_).Run(false, base::UnguessableToken());
 }
 
 void MediaDrmStorageImpl::Initialize(InitializeCallback callback) {
@@ -665,7 +665,7 @@ void MediaDrmStorageImpl::Initialize(InitializeCallback callback) {
   DCHECK(!init_cb_);
 
   if (is_initialized_) {
-    std::move(callback).Run(origin_id_);
+    std::move(callback).Run(true, origin_id_);
     return;
   }
   DCHECK(!origin_id_);
@@ -686,7 +686,7 @@ void MediaDrmStorageImpl::OnOriginIdObtained(
     const base::UnguessableToken& origin_id) {
   is_initialized_ = true;
   origin_id_ = origin_id;
-  std::move(init_cb_).Run(origin_id_);
+  std::move(init_cb_).Run(true, origin_id_);
 }
 
 void MediaDrmStorageImpl::OnProvisioned(OnProvisionedCallback callback) {

components/cdm/browser/media_drm_storage_impl_unittest.cc

@@ -30,8 +30,10 @@ const char kTestOrigin[] = "https://www.testorigin.com:80";
 const char kTestOrigin2[] = "https://www.testorigin2.com:80";
 
 void OnMediaDrmStorageInit(base::UnguessableToken* out_origin_id,
+                           bool success,
                            const base::UnguessableToken& origin_id) {
   DCHECK(out_origin_id);
+  DCHECK(success);
   DCHECK(origin_id);
   *out_origin_id = origin_id;
 }

media/base/android/media_drm_bridge_factory.cc

@@ -83,20 +83,18 @@ void MediaDrmBridgeFactory::Create(
                      weak_factory_.GetWeakPtr()));
 }
 
-void MediaDrmBridgeFactory::OnStorageInitialized() {
+void MediaDrmBridgeFactory::OnStorageInitialized(bool success) {
   DCHECK(storage_);
+  DVLOG(2) << __func__ << ": success = " << success
+           << ", origin_id = " << storage_->origin_id();
 
-  // MediaDrmStorageBridge should always return a valid origin ID after
-  // initialize. Otherwise the pipe is broken and we should not create
-  // MediaDrmBridge here.
-  auto origin_id = storage_->origin_id();
-  DVLOG(2) << __func__ << ": origin_id = " << origin_id;
-  if (origin_id.empty()) {
+  // MediaDrmStorageBridge should only be created on a successful Initialize().
+  if (!success) {
     std::move(cdm_created_cb_).Run(nullptr, "Cannot fetch origin ID");
     return;
   }
 
-  CreateMediaDrmBridge(origin_id);
+  CreateMediaDrmBridge(storage_->origin_id());
 }
 
 void MediaDrmBridgeFactory::CreateMediaDrmBridge(const std::string& origin_id) {

media/base/android/media_drm_bridge_factory.h

@@ -42,7 +42,7 @@ class MEDIA_EXPORT MediaDrmBridgeFactory : public CdmFactory {
 
  private:
   // Callback for Initialize() on |storage_|.
-  void OnStorageInitialized();
+  void OnStorageInitialized(bool success);
 
   // Creates |media_drm_bridge_|, and call SetMediaCryptoReadyCB() to wait for
   // MediaCrypto to be ready.

media/base/android/media_drm_storage.h

@@ -49,7 +49,8 @@ class MEDIA_EXPORT MediaDrmStorage
 
   // Callback for storage initialization.
   using InitCB =
-      base::OnceCallback<void(const base::UnguessableToken& origin_id)>;
+      base::OnceCallback<void(bool success,
+                              const base::UnguessableToken& origin_id)>;
 
   // Callback to return the result of LoadPersistentSession. |key_set_id| and
   // |mime_type| must be non-empty if |success| is true, and vice versa.

media/base/android/media_drm_storage_bridge.cc

@@ -38,7 +38,7 @@ MediaDrmStorageBridge::MediaDrmStorageBridge()
 MediaDrmStorageBridge::~MediaDrmStorageBridge() = default;
 
 void MediaDrmStorageBridge::Initialize(const CreateStorageCB& create_storage_cb,
-                                       base::OnceClosure init_cb) {
+                                       InitCB init_cb) {
   DCHECK(create_storage_cb);
   impl_ = create_storage_cb.Run();
 
@@ -147,14 +147,20 @@ void MediaDrmStorageBridge::RunAndroidBoolCallback(JavaObjectPtr j_callback,
 }
 
 void MediaDrmStorageBridge::OnInitialized(
-    base::OnceClosure init_cb,
+    InitCB init_cb,
+    bool success,
     const base::UnguessableToken& origin_id) {
-  DCHECK(origin_id_.empty());
-
-  if (origin_id)
+  // Note: It's possible that |success| is true but |origin_id| is empty,
+  // to indicate per-device provisioning. In this case, we cannot use
+  // ToString() which would be the string 0000000000000000. Instead, do
+  // not set |origin_id_| so that it remains empty.
+  if (success && origin_id) {
     origin_id_ = origin_id.ToString();
+  } else {
+    DCHECK(!origin_id);
+  }
 
-  std::move(init_cb).Run();
+  std::move(init_cb).Run(success);
 }
 
 void MediaDrmStorageBridge::OnSessionDataLoaded(

media/base/android/media_drm_storage_bridge.h

@@ -27,16 +27,17 @@ namespace media {
 // to talk to the concrete implementation for persistent data management.
 class MediaDrmStorageBridge {
  public:
+  using InitCB = base::OnceCallback<void(bool)>;
+
   MediaDrmStorageBridge();
   ~MediaDrmStorageBridge();
 
   // Once storage is initialized, |init_cb| will be called and it will have a
   // random generated origin id for later usage. If this function isn't called,
   // all the other functions will fail.
-  void Initialize(const CreateStorageCB& create_storage_cb,
-                  base::OnceClosure init_cb);
+  void Initialize(const CreateStorageCB& create_storage_cb, InitCB init_cb);
 
-  std::string origin_id() const { return origin_id_; }
+  const std::string& origin_id() const { return origin_id_; }
 
   // The following OnXXX functions are called by Java. The functions will post
   // task on message loop immediately to avoid reentrancy issues.
@@ -73,7 +74,8 @@ class MediaDrmStorageBridge {
 
  private:
   void RunAndroidBoolCallback(JavaObjectPtr j_callback, bool success);
-  void OnInitialized(base::OnceClosure init_cb,
+  void OnInitialized(InitCB init_cb,
+                     bool success,
                      const base::UnguessableToken& origin_id);
   void OnSessionDataLoaded(
       JavaObjectPtr j_callback,

media/mojo/interfaces/media_drm_storage.mojom

@@ -23,12 +23,15 @@ struct SessionData {
 // service is the true source for the persistent license and origin
 // provisioning.
 interface MediaDrmStorage {
-  // Initializes |this| and returns a random identifier than can be used to
-  // identify the current origin. The origin ID should be randomly generated if
-  // it doesn't exist. |origin_id| must be valid and unique among all origins.
-  // If origin information doesn't exist, the implementation will persist the
-  // the information (e.g. origin ID, provision time) in the storage.
-  Initialize() => (mojo_base.mojom.UnguessableToken origin_id);
+  // Initializes |this| and if successful (|success| = true) returns an origin
+  // ID that can be used to identify the current origin. If |origin_id| is
+  // not empty, then it must be valid and unique among all origins. The
+  // implementation will persist the information (e.g. origin ID, provision
+  // time) in the storage. If |origin_id| is empty, then device-wide
+  // provisioning is to be used. If Initialize() fails or returns an empty
+  // origin ID then the other methods below should not be called, and will
+  // fail if they are called.
+  Initialize() => (bool success, mojo_base.mojom.UnguessableToken origin_id);
 
   // Saves origin information (e.g. origin ID, provision time) in the storage
   // after MediaDrm is provisioned for current origin. It will clear all

media/mojo/services/mojo_media_drm_storage.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/unguessable_token.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
 
 namespace media {
@@ -28,7 +29,7 @@ MojoMediaDrmStorage::~MojoMediaDrmStorage() {}
 void MojoMediaDrmStorage::Initialize(InitCB init_cb) {
   DVLOG(1) << __func__;
   media_drm_storage_ptr_->Initialize(
-      mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(init_cb),
+      mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(init_cb), false,
                                                   base::UnguessableToken()));
 }