Commit 5dad049a authored by RJ Ascani's avatar RJ Ascani Committed by Commit Bot

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

This reverts commit cca7402b.

Reason for revert: This made the FuchsiaCdmManager tests flaky. See crbug.com/1143067

Original change's description:
> [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.
>
> Bug: 1140655
> Change-Id: I060edc909976e7b5a493afc3bc9a2a26e26994be
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500532
> Reviewed-by: Wez <wez@chromium.org>
> Commit-Queue: RJ Ascani <rjascani@google.com>
> Cr-Commit-Position: refs/heads/master@{#821353}

TBR=wez@chromium.org,rjascani@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1140655, 1143067
Change-Id: I06955764f82f32f3aa96443f91ffd566aede3b91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505977Reviewed-by: default avatarWez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Auto-Submit: RJ Ascani <rjascani@google.com>
Cr-Commit-Position: refs/heads/master@{#822164}
parent 2928283f
...@@ -17,10 +17,7 @@ ...@@ -17,10 +17,7 @@
#include "base/fuchsia/fuchsia_logging.h" #include "base/fuchsia/fuchsia_logging.h"
#include "base/hash/hash.h" #include "base/hash/hash.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/optional.h"
#include "base/strings/string_number_conversions.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 "media/fuchsia/cdm/service/provisioning_fetcher_impl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -33,16 +30,6 @@ std::string HexEncodeHash(const std::string& name) { ...@@ -33,16 +30,6 @@ std::string HexEncodeHash(const std::string& name) {
return base::HexEncode(&hash, sizeof(uint32_t)); 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 } // namespace
// Manages individual KeySystem connections. Provides data stores and // Manages individual KeySystem connections. Provides data stores and
...@@ -187,14 +174,24 @@ void FuchsiaCdmManager::CreateAndProvision( ...@@ -187,14 +174,24 @@ void FuchsiaCdmManager::CreateAndProvision(
request) { request) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); 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::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;
}
base::ThreadPool::PostTaskAndReplyWithResult( key_system_client->CreateCdm(std::move(storage_path),
FROM_HERE, {base::MayBlock()}, std::move(create_fetcher_cb),
base::BindOnce(&CreateStorageDirectory, storage_path), std::move(request));
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( void FuchsiaCdmManager::set_on_key_system_disconnect_for_test_callback(
...@@ -244,35 +241,6 @@ base::FilePath FuchsiaCdmManager::GetStoragePath(const std::string& key_system, ...@@ -244,35 +241,6 @@ base::FilePath FuchsiaCdmManager::GetStoragePath(const std::string& key_system,
.Append(HexEncodeHash(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( void FuchsiaCdmManager::OnKeySystemClientError(
const std::string& key_system_name) { const std::string& key_system_name) {
if (on_key_system_disconnect_for_test_callback_) { if (on_key_system_disconnect_for_test_callback_) {
......
...@@ -10,11 +10,8 @@ ...@@ -10,11 +10,8 @@
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
#include "base/files/file.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "media/base/provision_fetcher.h" #include "media/base/provision_fetcher.h"
...@@ -66,13 +63,6 @@ class FuchsiaCdmManager { ...@@ -66,13 +63,6 @@ class FuchsiaCdmManager {
KeySystemClient* CreateKeySystemClient(const std::string& key_system_name); KeySystemClient* CreateKeySystemClient(const std::string& key_system_name);
base::FilePath GetStoragePath(const std::string& key_system_name, base::FilePath GetStoragePath(const std::string& key_system_name,
const url::Origin& origin); 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); void OnKeySystemClientError(const std::string& key_system_name);
// A map of callbacks to create KeySystem channels indexed by their EME name. // A map of callbacks to create KeySystem channels indexed by their EME name.
...@@ -89,7 +79,6 @@ class FuchsiaCdmManager { ...@@ -89,7 +79,6 @@ class FuchsiaCdmManager {
on_key_system_disconnect_for_test_callback_; on_key_system_disconnect_for_test_callback_;
THREAD_CHECKER(thread_checker_); THREAD_CHECKER(thread_checker_);
base::WeakPtrFactory<FuchsiaCdmManager> weak_factory_{this};
}; };
} // namespace media } // namespace media
......
...@@ -96,8 +96,8 @@ class FuchsiaCdmManagerTest : public ::testing::Test { ...@@ -96,8 +96,8 @@ class FuchsiaCdmManagerTest : public ::testing::Test {
return mock_key_systems_[key_system_name]; return mock_key_systems_[key_system_name];
} }
base::test::TaskEnvironment task_environment_{ base::test::SingleThreadTaskEnvironment task_environment_{
base::test::TaskEnvironment::MainThreadType::IO}; base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
MockKeySystemMap mock_key_systems_; MockKeySystemMap mock_key_systems_;
base::ScopedTempDir temp_dir_; base::ScopedTempDir temp_dir_;
...@@ -109,7 +109,7 @@ TEST_F(FuchsiaCdmManagerTest, NoKeySystems) { ...@@ -109,7 +109,7 @@ TEST_F(FuchsiaCdmManagerTest, NoKeySystems) {
base::RunLoop run_loop; base::RunLoop run_loop;
drm::ContentDecryptionModulePtr cdm_ptr; drm::ContentDecryptionModulePtr cdm_ptr;
cdm_ptr.set_error_handler([&](zx_status_t status) { cdm_ptr.set_error_handler([&](zx_status_t status) {
EXPECT_EQ(status, ZX_ERR_NOT_FOUND); EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
run_loop.Quit(); run_loop.Quit();
}); });
......
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