Commit f206ba57 authored by Dominik Laskowski's avatar Dominik Laskowski Committed by Commit Bot

display: Simplify content protection task deferral

DisplayConfigurator serializes asynchronous content protection requests
by storing pending query/apply tasks as closures in a queue. It also
maintains queues for the query/apply callbacks supplied by the client.

This CL consolidates the queues by relating QueryContentProtectionTask
and ApplyContentProtectionTask by a common base class. It also replaces
the Boolean for task success/failure with tri-state, whose extra bit is
used to distinguish killed tasks during DisplayConfigurator destruction
and (in a future CL) display reconfiguration.

Bug: 929449
Test: display_unittests
Change-Id: I3640b657ed5952c40e0999836ae92287984a3e9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1495201Reviewed-by: default avatarDaniel Nicoara <dnicoara@chromium.org>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Commit-Queue: Dominik Laskowski <domlaskowski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649234}
parent b7b92d6f
......@@ -44,18 +44,21 @@ ApplyContentProtectionTask::ApplyContentProtectionTask(
requests_(std::move(requests)),
callback_(std::move(callback)) {}
ApplyContentProtectionTask::~ApplyContentProtectionTask() {}
ApplyContentProtectionTask::~ApplyContentProtectionTask() {
if (callback_)
std::move(callback_).Run(Status::KILLED);
}
void ApplyContentProtectionTask::Run() {
std::vector<DisplaySnapshot*> hdcp_capable_displays;
if (!GetHDCPCapableDisplays(*layout_manager_, &hdcp_capable_displays)) {
std::move(callback_).Run(/*success=*/false);
std::move(callback_).Run(Status::FAILURE);
return;
}
pending_requests_ = hdcp_capable_displays.size();
if (pending_requests_ == 0) {
std::move(callback_).Run(/*success=*/true);
std::move(callback_).Run(Status::SUCCESS);
return;
}
......@@ -81,7 +84,7 @@ void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id,
return;
if (!success_) {
std::move(callback_).Run(/*success=*/false);
std::move(callback_).Run(Status::FAILURE);
return;
}
......@@ -91,7 +94,7 @@ void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id,
void ApplyContentProtectionTask::ApplyProtections() {
std::vector<DisplaySnapshot*> hdcp_capable_displays;
if (!GetHDCPCapableDisplays(*layout_manager_, &hdcp_capable_displays)) {
std::move(callback_).Run(/*success=*/false);
std::move(callback_).Run(Status::FAILURE);
return;
}
......@@ -103,7 +106,7 @@ void ApplyContentProtectionTask::ApplyProtections() {
auto it = hdcp_states_.find(display->display_id());
// If the display can't be found, the display configuration changed.
if (it == hdcp_states_.end()) {
std::move(callback_).Run(/*success=*/false);
std::move(callback_).Run(Status::FAILURE);
return;
}
......@@ -119,7 +122,7 @@ void ApplyContentProtectionTask::ApplyProtections() {
// All the requested changes are the same as the current HDCP state. Nothing
// to do anymore, just ack the content protection change.
if (pending_requests_ == 0) {
std::move(callback_).Run(/*success=*/true);
std::move(callback_).Run(Status::SUCCESS);
return;
}
......@@ -136,7 +139,7 @@ void ApplyContentProtectionTask::OnSetHDCPState(bool success) {
pending_requests_--;
if (pending_requests_ == 0)
std::move(callback_).Run(success_);
std::move(callback_).Run(success_ ? Status::SUCCESS : Status::FAILURE);
}
uint32_t ApplyContentProtectionTask::GetDesiredProtectionMask(
......
......@@ -31,17 +31,18 @@ class NativeDisplayDelegate;
//
// Note, in steps 1a and 2a, if no HDCP capable displays are found or if errors
// are reported, the task finishes early and skips to step 3.
class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask {
class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask
: public DisplayConfigurator::ContentProtectionTask {
public:
using ResponseCallback = base::OnceCallback<void(bool success)>;
using ResponseCallback = base::OnceCallback<void(Status)>;
ApplyContentProtectionTask(DisplayLayoutManager* layout_manager,
NativeDisplayDelegate* native_display_delegate,
DisplayConfigurator::ContentProtections requests,
ResponseCallback callback);
~ApplyContentProtectionTask();
~ApplyContentProtectionTask() override;
void Run();
void Run() override;
private:
void OnGetHDCPState(int64_t display_id, bool success, HDCPState state);
......
......@@ -37,21 +37,15 @@ std::unique_ptr<DisplaySnapshot> CreateDisplaySnapshot(
class ApplyContentProtectionTaskTest : public testing::Test {
public:
enum class Response {
ERROR,
SUCCESS,
NOT_CALLED,
};
using Response = ApplyContentProtectionTask::Status;
ApplyContentProtectionTaskTest() = default;
~ApplyContentProtectionTaskTest() override = default;
void ResponseCallback(bool success) {
response_ = success ? Response::SUCCESS : Response::ERROR;
}
void ResponseCallback(Response response) { response_ = response; }
protected:
Response response_ = Response::NOT_CALLED;
Response response_ = Response::KILLED;
ActionLogger log_;
TestNativeDisplayDelegate display_delegate_{&log_};
......@@ -115,7 +109,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToUnknownDisplay) {
base::Unretained(this)));
task.Run();
EXPECT_EQ(Response::ERROR, response_);
EXPECT_EQ(Response::FAILURE, response_);
EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
}
......@@ -134,7 +128,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToDisplayThatCannotGetHdcp) {
base::Unretained(this)));
task.Run();
EXPECT_EQ(Response::ERROR, response_);
EXPECT_EQ(Response::FAILURE, response_);
EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
}
......@@ -153,7 +147,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToDisplayThatCannotSetHdcp) {
base::Unretained(this)));
task.Run();
EXPECT_EQ(Response::ERROR, response_);
EXPECT_EQ(Response::FAILURE, response_);
EXPECT_EQ(
JoinActions(GetSetHDCPStateAction(*layout_manager.GetDisplayStates()[0],
HDCP_STATE_DESIRED)
......
......@@ -588,15 +588,8 @@ DisplayConfigurator::~DisplayConfigurator() {
CallAndClearInProgressCallbacks(false);
CallAndClearQueuedCallbacks(false);
while (!query_protection_callbacks_.empty()) {
std::move(query_protection_callbacks_.front()).Run(false, 0, 0);
query_protection_callbacks_.pop();
}
while (!apply_protection_callbacks_.empty()) {
std::move(apply_protection_callbacks_.front()).Run(false);
apply_protection_callbacks_.pop();
}
// Destroy to fire failure callbacks before weak pointers for |this| expire.
content_protection_tasks_ = {};
}
void DisplayConfigurator::SetDelegateForTesting(
......@@ -781,29 +774,17 @@ void DisplayConfigurator::UnregisterContentProtectionClient(
}
}
apply_protection_callbacks_.push(base::DoNothing());
ApplyContentProtectionTask* task = new ApplyContentProtectionTask(
QueueContentProtectionTask(new ApplyContentProtectionTask(
layout_manager_.get(), native_display_delegate_.get(), protections,
base::Bind(&DisplayConfigurator::OnContentProtectionClientUnregistered,
weak_ptr_factory_.GetWeakPtr()));
content_protection_tasks_.push(
base::Bind(&ApplyContentProtectionTask::Run, base::Owned(task)));
if (content_protection_tasks_.size() == 1)
content_protection_tasks_.front().Run();
base::BindOnce(
&DisplayConfigurator::OnContentProtectionClientUnregistered,
weak_ptr_factory_.GetWeakPtr())));
}
void DisplayConfigurator::OnContentProtectionClientUnregistered(bool success) {
DCHECK(!content_protection_tasks_.empty());
content_protection_tasks_.pop();
DCHECK(!apply_protection_callbacks_.empty());
ApplyContentProtectionCallback callback =
std::move(apply_protection_callbacks_.front());
apply_protection_callbacks_.pop();
if (!content_protection_tasks_.empty())
content_protection_tasks_.front().Run();
void DisplayConfigurator::OnContentProtectionClientUnregistered(
ContentProtectionTask::Status status) {
if (status != ContentProtectionTask::Status::KILLED)
DequeueContentProtectionTask();
}
void DisplayConfigurator::QueryContentProtection(
......@@ -827,22 +808,21 @@ void DisplayConfigurator::QueryContentProtection(
}
}
query_protection_callbacks_.push(std::move(callback));
QueryContentProtectionTask* task = new QueryContentProtectionTask(
QueueContentProtectionTask(new QueryContentProtectionTask(
layout_manager_.get(), native_display_delegate_.get(), display_id,
base::Bind(&DisplayConfigurator::OnContentProtectionQueried,
weak_ptr_factory_.GetWeakPtr(), *client_id, display_id));
content_protection_tasks_.push(
base::Bind(&QueryContentProtectionTask::Run, base::Owned(task)));
if (content_protection_tasks_.size() == 1)
content_protection_tasks_.front().Run();
base::BindOnce(&DisplayConfigurator::OnContentProtectionQueried,
weak_ptr_factory_.GetWeakPtr(), std::move(callback),
*client_id, display_id)));
}
void DisplayConfigurator::OnContentProtectionQueried(uint64_t client_id,
int64_t display_id,
bool success,
uint32_t connection_mask,
uint32_t protection_mask) {
void DisplayConfigurator::OnContentProtectionQueried(
QueryContentProtectionCallback callback,
uint64_t client_id,
int64_t display_id,
ContentProtectionTask::Status status,
uint32_t connection_mask,
uint32_t protection_mask) {
bool success = status == ContentProtectionTask::Status::SUCCESS;
uint32_t client_mask = CONTENT_PROTECTION_METHOD_NONE;
// Don't reveal protections requested by other clients.
......@@ -857,17 +837,10 @@ void DisplayConfigurator::OnContentProtectionQueried(uint64_t client_id,
protection_mask &= client_mask;
DCHECK(!content_protection_tasks_.empty());
content_protection_tasks_.pop();
DCHECK(!query_protection_callbacks_.empty());
QueryContentProtectionCallback callback =
std::move(query_protection_callbacks_.front());
query_protection_callbacks_.pop();
std::move(callback).Run(success, connection_mask, protection_mask);
if (!content_protection_tasks_.empty())
content_protection_tasks_.front().Run();
if (status != ContentProtectionTask::Status::KILLED)
DequeueContentProtectionTask();
}
void DisplayConfigurator::ApplyContentProtection(
......@@ -892,30 +865,36 @@ void DisplayConfigurator::ApplyContentProtection(
}
protections[display_id] |= protection_mask;
apply_protection_callbacks_.push(std::move(callback));
ApplyContentProtectionTask* task = new ApplyContentProtectionTask(
QueueContentProtectionTask(new ApplyContentProtectionTask(
layout_manager_.get(), native_display_delegate_.get(), protections,
base::Bind(&DisplayConfigurator::OnContentProtectionApplied,
weak_ptr_factory_.GetWeakPtr(), *client_id, display_id,
protection_mask));
content_protection_tasks_.push(
base::Bind(&ApplyContentProtectionTask::Run, base::Owned(task)));
base::BindOnce(&DisplayConfigurator::OnContentProtectionApplied,
weak_ptr_factory_.GetWeakPtr(), std::move(callback),
*client_id, display_id, protection_mask)));
}
void DisplayConfigurator::QueueContentProtectionTask(
ContentProtectionTask* task) {
content_protection_tasks_.emplace(task);
if (content_protection_tasks_.size() == 1)
content_protection_tasks_.front().Run();
content_protection_tasks_.front()->Run();
}
void DisplayConfigurator::OnContentProtectionApplied(uint64_t client_id,
int64_t display_id,
uint32_t protection_mask,
bool success) {
void DisplayConfigurator::DequeueContentProtectionTask() {
DCHECK(!content_protection_tasks_.empty());
content_protection_tasks_.pop();
DCHECK(!apply_protection_callbacks_.empty());
ApplyContentProtectionCallback callback =
std::move(apply_protection_callbacks_.front());
apply_protection_callbacks_.pop();
if (!content_protection_tasks_.empty())
content_protection_tasks_.front()->Run();
}
void DisplayConfigurator::OnContentProtectionApplied(
ApplyContentProtectionCallback callback,
uint64_t client_id,
int64_t display_id,
uint32_t protection_mask,
ContentProtectionTask::Status status) {
bool success = status == ContentProtectionTask::Status::SUCCESS;
if (success) {
if (protection_mask == CONTENT_PROTECTION_METHOD_NONE) {
auto it = content_protection_requests_.find(client_id);
......@@ -933,8 +912,8 @@ void DisplayConfigurator::OnContentProtectionApplied(uint64_t client_id,
std::move(callback).Run(success);
if (!content_protection_tasks_.empty())
content_protection_tasks_.front().Run();
if (status != ContentProtectionTask::Status::KILLED)
DequeueContentProtectionTask();
}
bool DisplayConfigurator::SetColorMatrix(
......
......@@ -62,6 +62,15 @@ class DISPLAY_MANAGER_EXPORT DisplayConfigurator
using ContentProtections =
base::flat_map<int64_t /* display_id */, uint32_t /* protection_mask */>;
// Though only run once, a task must outlive its asynchronous operations, so
// cannot be a OnceCallback.
struct ContentProtectionTask {
enum class Status { KILLED, FAILURE, SUCCESS };
virtual ~ContentProtectionTask() = default;
virtual void Run() = 0;
};
class Observer {
public:
virtual ~Observer() {}
......@@ -359,16 +368,22 @@ class DISPLAY_MANAGER_EXPORT DisplayConfigurator
// Content protection callbacks called by the tasks when they finish. These
// are responsible for destroying the task, replying to the caller that made
// the task and starting the a new content protection task if one is queued.
void OnContentProtectionQueried(uint64_t client_id,
void OnContentProtectionQueried(QueryContentProtectionCallback callback,
uint64_t client_id,
int64_t display_id,
bool success,
ContentProtectionTask::Status status,
uint32_t connection_mask,
uint32_t protection_mask);
void OnContentProtectionApplied(uint64_t client_id,
void OnContentProtectionApplied(ApplyContentProtectionCallback callback,
uint64_t client_id,
int64_t display_id,
uint32_t protection_mask,
bool success);
void OnContentProtectionClientUnregistered(bool success);
ContentProtectionTask::Status status);
void OnContentProtectionClientUnregistered(
ContentProtectionTask::Status status);
void QueueContentProtectionTask(ContentProtectionTask* task);
void DequeueContentProtectionTask();
// Callbacks used to signal when the native platform has released/taken
// display control.
......@@ -427,10 +442,6 @@ class DISPLAY_MANAGER_EXPORT DisplayConfigurator
// task.
std::vector<ConfigurationCallback> in_progress_configuration_callbacks_;
base::queue<base::Closure> content_protection_tasks_;
base::queue<QueryContentProtectionCallback> query_protection_callbacks_;
base::queue<ApplyContentProtectionCallback> apply_protection_callbacks_;
// True if the caller wants to force the display configuration process.
bool force_configure_;
......@@ -449,6 +460,9 @@ class DISPLAY_MANAGER_EXPORT DisplayConfigurator
// Content protections requested by each client.
base::flat_map<uint64_t, ContentProtections> content_protection_requests_;
// Pending tasks to query or apply content protection.
base::queue<std::unique_ptr<ContentProtectionTask>> content_protection_tasks_;
// Display controlled by an external entity.
bool display_externally_controlled_;
......
......@@ -24,7 +24,12 @@ QueryContentProtectionTask::QueryContentProtectionTask(
display_id_(display_id),
callback_(std::move(callback)) {}
QueryContentProtectionTask::~QueryContentProtectionTask() {}
QueryContentProtectionTask::~QueryContentProtectionTask() {
if (callback_) {
std::move(callback_).Run(Status::KILLED, DISPLAY_CONNECTION_TYPE_NONE,
CONTENT_PROTECTION_METHOD_NONE);
}
}
void QueryContentProtectionTask::Run() {
std::vector<DisplaySnapshot*> hdcp_capable_displays;
......@@ -37,8 +42,7 @@ void QueryContentProtectionTask::Run() {
uint32_t protection_mask;
if (!GetContentProtectionMethods(display->type(), &protection_mask)) {
std::move(callback_).Run(/*success=*/false,
DISPLAY_CONNECTION_TYPE_UNKNOWN,
std::move(callback_).Run(Status::FAILURE, DISPLAY_CONNECTION_TYPE_UNKNOWN,
CONTENT_PROTECTION_METHOD_NONE);
return;
}
......@@ -49,7 +53,7 @@ void QueryContentProtectionTask::Run() {
pending_requests_ = hdcp_capable_displays.size();
if (pending_requests_ == 0) {
std::move(callback_).Run(/*success=*/true, connection_mask_,
std::move(callback_).Run(Status::SUCCESS, connection_mask_,
CONTENT_PROTECTION_METHOD_NONE);
return;
}
......@@ -75,7 +79,8 @@ void QueryContentProtectionTask::OnGetHDCPState(bool success, HDCPState state) {
return;
protection_mask_ &= ~no_protection_mask_;
std::move(callback_).Run(success_, connection_mask_, protection_mask_);
std::move(callback_).Run(success_ ? Status::SUCCESS : Status::FAILURE,
connection_mask_, protection_mask_);
}
} // namespace display
......@@ -11,6 +11,7 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "ui/display/manager/display_configurator.h"
#include "ui/display/manager/display_manager_export.h"
#include "ui/display/types/display_constants.h"
......@@ -19,20 +20,21 @@ namespace display {
class DisplayLayoutManager;
class NativeDisplayDelegate;
class DISPLAY_MANAGER_EXPORT QueryContentProtectionTask {
class DISPLAY_MANAGER_EXPORT QueryContentProtectionTask
: public DisplayConfigurator::ContentProtectionTask {
public:
// |connection_mask| includes mirroring displays, and a protection method is
// only included in |protection_mask| if also enabled on mirroring displays.
using ResponseCallback = base::OnceCallback<
void(bool success, uint32_t connection_mask, uint32_t protection_mask)>;
void(Status status, uint32_t connection_mask, uint32_t protection_mask)>;
QueryContentProtectionTask(DisplayLayoutManager* layout_manager,
NativeDisplayDelegate* native_display_delegate,
int64_t display_id,
ResponseCallback callback);
~QueryContentProtectionTask();
~QueryContentProtectionTask() override;
void Run();
void Run() override;
private:
void OnGetHDCPState(bool success, HDCPState state);
......
......@@ -38,13 +38,15 @@ std::unique_ptr<DisplaySnapshot> CreateDisplaySnapshot(
class QueryContentProtectionTaskTest : public testing::Test {
public:
using Status = QueryContentProtectionTask::Status;
QueryContentProtectionTaskTest() = default;
~QueryContentProtectionTaskTest() override = default;
void ResponseCallback(bool success,
void ResponseCallback(Status status,
uint32_t connection_mask,
uint32_t protection_mask) {
response_ = Response{success, connection_mask, protection_mask};
response_ = Response{status, connection_mask, protection_mask};
}
protected:
......@@ -52,7 +54,7 @@ class QueryContentProtectionTaskTest : public testing::Test {
TestNativeDisplayDelegate display_delegate_{&log_};
struct Response {
bool success;
Status status;
uint32_t connection_mask;
uint32_t protection_mask;
};
......@@ -77,7 +79,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryInternalDisplay) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_TRUE(response_->success);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_INTERNAL, response_->connection_mask);
EXPECT_EQ(0u, response_->protection_mask);
}
......@@ -95,7 +97,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryUnknownDisplay) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_FALSE(response_->success);
EXPECT_EQ(Status::FAILURE, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_UNKNOWN, response_->connection_mask);
EXPECT_EQ(0u, response_->protection_mask);
}
......@@ -114,7 +116,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryDisplayThatCannotGetHdcp) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_FALSE(response_->success);
EXPECT_EQ(Status::FAILURE, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
}
......@@ -131,7 +133,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryDisplayWithHdcpDisabled) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_TRUE(response_->success);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
EXPECT_EQ(0u, response_->protection_mask);
}
......@@ -150,7 +152,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryDisplayWithHdcpEnabled) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_TRUE(response_->success);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
EXPECT_EQ(CONTENT_PROTECTION_METHOD_HDCP, response_->protection_mask);
}
......@@ -169,7 +171,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryInMultiDisplayMode) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_TRUE(response_->success);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
EXPECT_EQ(0u, response_->protection_mask);
}
......@@ -188,7 +190,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryInMirroringMode) {
task.Run();
ASSERT_TRUE(response_);
EXPECT_TRUE(response_->success);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(static_cast<uint32_t>(DISPLAY_CONNECTION_TYPE_HDMI |
DISPLAY_CONNECTION_TYPE_DVI),
response_->connection_mask);
......
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