[Fuchsia] Move CDM data file i/o off main thread

The FuchsiaCdmManager needs to create directories for each origin to
pass to the platform CDM services to fulfill EME origin isolation
requirements. The FuchsiaCdmManager runs on the main thread and the
calls to CreateDirectoryAndGetError are potentially blocking. This CL
moves the directory creation step to a background task.

This is a reland of crrev.com/c/2500532. Since directory creation was
pushed to the threadpool, the order in which CDM requests are processed
is no longer certain. A couple of the tests were using the error handler
on a particular ContentDecryptionModulePtr to determine when to
terminate the RunLoop. The quit condition now requires all
ContentDecryptionModulePtrs to have their error handlers fired.

Bug: 1140655
Change-Id: Ie28885c1736861a1bba66acd3e9aa6f26f1208bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508031
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822958}

media/fuchsia/cdm/service/fuchsia_cdm_manager.cc

@@ -17,7 +17,10 @@
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/hash/hash.h"
 #include "base/logging.h"
+#include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.h"
 #include "media/fuchsia/cdm/service/provisioning_fetcher_impl.h"
 #include "url/origin.h"
 
@@ -30,6 +33,16 @@ std::string HexEncodeHash(const std::string& name) {
   return base::HexEncode(&hash, sizeof(uint32_t));
 }
 
+// Returns a nullopt if storage was created successfully.
+base::Optional<base::File::Error> CreateStorageDirectory(base::FilePath path) {
+  base::File::Error error;
+  bool success = base::CreateDirectoryAndGetError(path, &error);
+  if (!success) {
+    return error;
+  }
+  return {};
+}
+
 }  // namespace
 
 // Manages individual KeySystem connections. Provides data stores and
@@ -174,24 +187,14 @@ void FuchsiaCdmManager::CreateAndProvision(
         request) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  KeySystemClient* key_system_client = GetOrCreateKeySystemClient(key_system);
-  if (!key_system_client) {
-    // GetOrCreateKeySystemClient will log the reason for failure.
-    return;
-  }
-
   base::FilePath storage_path = GetStoragePath(key_system, origin);
-  base::File::Error error;
-  bool success = base::CreateDirectoryAndGetError(storage_path, &error);
-  if (!success) {
-    DLOG(ERROR) << "Failed to create directory: " << storage_path
-                << ", error: " << error;
-    return;
-  }
 
-  key_system_client->CreateCdm(std::move(storage_path),
-                               std::move(create_fetcher_cb),
-                               std::move(request));
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&CreateStorageDirectory, storage_path),
+      base::BindOnce(&FuchsiaCdmManager::CreateCdm, weak_factory_.GetWeakPtr(),
+                     key_system, std::move(create_fetcher_cb),
+                     std::move(request), storage_path));
 }
 
 void FuchsiaCdmManager::set_on_key_system_disconnect_for_test_callback(
@@ -241,6 +244,35 @@ base::FilePath FuchsiaCdmManager::GetStoragePath(const std::string& key_system,
       .Append(HexEncodeHash(key_system));
 }
 
+void FuchsiaCdmManager::CreateCdm(
+    const std::string& key_system_name,
+    CreateFetcherCB create_fetcher_cb,
+    fidl::InterfaceRequest<fuchsia::media::drm::ContentDecryptionModule>
+        request,
+    base::FilePath storage_path,
+    base::Optional<base::File::Error> storage_creation_error) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  if (storage_creation_error) {
+    DLOG(ERROR) << "Failed to create directory: " << storage_path
+                << ", error: " << *storage_creation_error;
+    request.Close(ZX_ERR_NO_RESOURCES);
+    return;
+  }
+
+  KeySystemClient* key_system_client =
+      GetOrCreateKeySystemClient(key_system_name);
+  if (!key_system_client) {
+    // GetOrCreateKeySystemClient will log the reason for failure.
+    request.Close(ZX_ERR_NOT_FOUND);
+    return;
+  }
+
+  key_system_client->CreateCdm(std::move(storage_path),
+                               std::move(create_fetcher_cb),
+                               std::move(request));
+}
+
 void FuchsiaCdmManager::OnKeySystemClientError(
     const std::string& key_system_name) {
   if (on_key_system_disconnect_for_test_callback_) {

media/fuchsia/cdm/service/fuchsia_cdm_manager.h

@@ -10,8 +10,11 @@
 
 #include "base/callback_forward.h"
 #include "base/containers/flat_map.h"
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/threading/thread_checker.h"
 #include "media/base/provision_fetcher.h"
 
@@ -63,6 +66,13 @@ class FuchsiaCdmManager {
   KeySystemClient* CreateKeySystemClient(const std::string& key_system_name);
   base::FilePath GetStoragePath(const std::string& key_system_name,
                                 const url::Origin& origin);
+  void CreateCdm(
+      const std::string& key_system_name,
+      CreateFetcherCB create_fetcher_cb,
+      fidl::InterfaceRequest<fuchsia::media::drm::ContentDecryptionModule>
+          request,
+      base::FilePath storage_path,
+      base::Optional<base::File::Error> storage_creation_error);
   void OnKeySystemClientError(const std::string& key_system_name);
 
   // A map of callbacks to create KeySystem channels indexed by their EME name.
@@ -79,6 +89,7 @@ class FuchsiaCdmManager {
       on_key_system_disconnect_for_test_callback_;
 
   THREAD_CHECKER(thread_checker_);
+  base::WeakPtrFactory<FuchsiaCdmManager> weak_factory_{this};
 };
 
 }  // namespace media

media/fuchsia/cdm/service/fuchsia_cdm_manager_test.cc

@@ -96,8 +96,8 @@ class FuchsiaCdmManagerTest : public ::testing::Test {
     return mock_key_systems_[key_system_name];
   }
 
-  base::test::SingleThreadTaskEnvironment task_environment_{
-      base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::MainThreadType::IO};
 
   MockKeySystemMap mock_key_systems_;
   base::ScopedTempDir temp_dir_;
@@ -109,7 +109,7 @@ TEST_F(FuchsiaCdmManagerTest, NoKeySystems) {
   base::RunLoop run_loop;
   drm::ContentDecryptionModulePtr cdm_ptr;
   cdm_ptr.set_error_handler([&](zx_status_t status) {
-    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    EXPECT_EQ(status, ZX_ERR_NOT_FOUND);
     run_loop.Quit();
   });
 
@@ -211,7 +211,14 @@ TEST_F(FuchsiaCdmManagerTest, SameOriginShareDataStore) {
 
   base::RunLoop run_loop;
   drm::ContentDecryptionModulePtr cdm1, cdm2;
-  cdm2.set_error_handler([&](zx_status_t) { run_loop.Quit(); });
+  auto error_handler = [&](zx_status_t status) {
+    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    if (!cdm1.is_bound() && !cdm2.is_bound()) {
+      run_loop.Quit();
+    }
+  };
+  cdm1.set_error_handler(error_handler);
+  cdm2.set_error_handler(error_handler);
 
   EXPECT_CALL(mock_key_system(kKeySystem), AddDataStore(Eq(1u), _, _))
       .WillOnce(
@@ -240,7 +247,15 @@ TEST_F(FuchsiaCdmManagerTest, DifferentOriginDoNotShareDataStore) {
 
   base::RunLoop run_loop;
   drm::ContentDecryptionModulePtr cdm1, cdm2;
-  cdm2.set_error_handler([&](zx_status_t) { run_loop.Quit(); });
+  auto error_handler = [&](zx_status_t status) {
+    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    if (!cdm1.is_bound() && !cdm2.is_bound()) {
+      run_loop.Quit();
+    }
+  };
+  cdm1.set_error_handler(error_handler);
+  cdm2.set_error_handler(error_handler);
+
   EXPECT_CALL(mock_key_system(kKeySystem), AddDataStore(Eq(1u), _, _))
       .WillOnce(
           WithArgs<2>(Invoke([](drm::KeySystem::AddDataStoreCallback callback) {