Commit 37026d99 authored by Peng Huang's avatar Peng Huang Committed by Commit Bot

Revert "Fix use of invalid DisplaySnapshot pointer while updating HDCP"

This reverts commit 3bb52934.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1064732

Original change's description:
> Fix use of invalid DisplaySnapshot pointer while updating HDCP
> 
> HDCP updates are asynchronous. Do not pass display state pointers
> between async calls since display configuration may have changed.
> 
> Instead query the display state and validate that the requested
> displays are valid.
> 
> BUG=1058030
> TEST=display_unittest
> 
> Change-Id: I7e037a44f06156b5b1da3f2672330ba8a18ca950
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118771
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Daniel Nicoara <dnicoara@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#753203}

TBR=oshima@chromium.org,dnicoara@chromium.org

Change-Id: I57fd6ee3708900914c15e8b844dc8ad772ec35e1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1058030
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2121084Reviewed-by: default avatarPeng Huang <penghuang@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753351}
parent 0ada1f96
...@@ -53,24 +53,23 @@ void ApplyContentProtectionTask::Run() { ...@@ -53,24 +53,23 @@ void ApplyContentProtectionTask::Run() {
// updated the state. // updated the state.
for (DisplaySnapshot* display : hdcp_capable_displays) { for (DisplaySnapshot* display : hdcp_capable_displays) {
native_display_delegate_->GetHDCPState( native_display_delegate_->GetHDCPState(
*display, *display, base::BindOnce(&ApplyContentProtectionTask::OnGetHDCPState,
base::BindOnce(&ApplyContentProtectionTask::OnGetHDCPState, weak_ptr_factory_.GetWeakPtr(), display));
weak_ptr_factory_.GetWeakPtr(), display->display_id()));
} }
} }
void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id, void ApplyContentProtectionTask::OnGetHDCPState(DisplaySnapshot* display,
bool success, bool success,
HDCPState state) { HDCPState state) {
success_ &= success; success_ &= success;
bool hdcp_enabled = state != HDCP_STATE_UNDESIRED; bool hdcp_enabled = state != HDCP_STATE_UNDESIRED;
bool hdcp_requested = bool hdcp_requested = GetDesiredProtectionMask(display->display_id()) &
GetDesiredProtectionMask(display_id) & CONTENT_PROTECTION_METHOD_HDCP; CONTENT_PROTECTION_METHOD_HDCP;
if (hdcp_enabled != hdcp_requested) { if (hdcp_enabled != hdcp_requested) {
hdcp_requests_.emplace_back( hdcp_requests_.emplace_back(
display_id, hdcp_requested ? HDCP_STATE_DESIRED : HDCP_STATE_UNDESIRED); display, hdcp_requested ? HDCP_STATE_DESIRED : HDCP_STATE_UNDESIRED);
} }
pending_requests_--; pending_requests_--;
...@@ -92,25 +91,9 @@ void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id, ...@@ -92,25 +91,9 @@ void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id,
return; return;
} }
std::vector<DisplaySnapshot*> displays = layout_manager_->GetDisplayStates();
std::vector<DisplaySnapshot*> hdcped_displays;
// Lookup the displays again since display configuration may have changed.
for (const auto& pair : hdcp_requests_) { for (const auto& pair : hdcp_requests_) {
auto it = std::find_if(displays.begin(), displays.end(),
[id = pair.first](DisplaySnapshot* display) {
return id == display->display_id();
});
if (it == displays.end()) {
std::move(callback_).Run(Status::FAILURE);
return;
}
hdcped_displays.push_back(*it);
}
for (size_t i = 0; i < hdcp_requests_.size(); ++i) {
native_display_delegate_->SetHDCPState( native_display_delegate_->SetHDCPState(
*hdcped_displays[i], hdcp_requests_[i].second, *pair.first, pair.second,
base::BindOnce(&ApplyContentProtectionTask::OnSetHDCPState, base::BindOnce(&ApplyContentProtectionTask::OnSetHDCPState,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
} }
......
...@@ -36,7 +36,7 @@ class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask ...@@ -36,7 +36,7 @@ class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask
void Run() override; void Run() override;
private: private:
void OnGetHDCPState(int64_t display_id, bool success, HDCPState state); void OnGetHDCPState(DisplaySnapshot* display, bool success, HDCPState state);
void OnSetHDCPState(bool success); void OnSetHDCPState(bool success);
uint32_t GetDesiredProtectionMask(int64_t display_id) const; uint32_t GetDesiredProtectionMask(int64_t display_id) const;
...@@ -47,7 +47,7 @@ class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask ...@@ -47,7 +47,7 @@ class DISPLAY_MANAGER_EXPORT ApplyContentProtectionTask
const ContentProtectionManager::ContentProtections requests_; const ContentProtectionManager::ContentProtections requests_;
ResponseCallback callback_; ResponseCallback callback_;
std::vector<std::pair<int64_t, HDCPState>> hdcp_requests_; std::vector<std::pair<DisplaySnapshot*, HDCPState>> hdcp_requests_;
bool success_ = true; bool success_ = true;
size_t pending_requests_ = 0; size_t pending_requests_ = 0;
......
...@@ -11,8 +11,6 @@ ...@@ -11,8 +11,6 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/run_loop.h"
#include "base/test/task_environment.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/display/fake/fake_display_snapshot.h" #include "ui/display/fake/fake_display_snapshot.h"
#include "ui/display/manager/display_layout_manager.h" #include "ui/display/manager/display_layout_manager.h"
...@@ -48,8 +46,6 @@ class ApplyContentProtectionTaskTest : public testing::Test { ...@@ -48,8 +46,6 @@ class ApplyContentProtectionTaskTest : public testing::Test {
void ResponseCallback(Response response) { response_ = response; } void ResponseCallback(Response response) { response_ = response; }
protected: protected:
base::test::SingleThreadTaskEnvironment task_environment_;
Response response_ = Response::KILLED; Response response_ = Response::KILLED;
ActionLogger log_; ActionLogger log_;
TestNativeDisplayDelegate display_delegate_{&log_}; TestNativeDisplayDelegate display_delegate_{&log_};
...@@ -171,34 +167,5 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyNoProtectionToExternalDisplay) { ...@@ -171,34 +167,5 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyNoProtectionToExternalDisplay) {
EXPECT_EQ(kNoActions, log_.GetActionsAndClear()); EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
} }
TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpWhileConfiguringDisplays) {
// Run async so the test can simulate a display change in the middle of
// updating HDCP state.
display_delegate_.set_run_async(true);
std::vector<std::unique_ptr<DisplaySnapshot>> displays;
displays.push_back(CreateDisplaySnapshot(DISPLAY_CONNECTION_TYPE_HDMI));
TestDisplayLayoutManager layout_manager(std::move(displays),
MULTIPLE_DISPLAY_STATE_SINGLE);
ContentProtectionManager::ContentProtections request;
request[1] = CONTENT_PROTECTION_METHOD_HDCP;
ApplyContentProtectionTask task(
&layout_manager, &display_delegate_, request,
base::BindOnce(&ApplyContentProtectionTaskTest::ResponseCallback,
base::Unretained(this)));
task.Run();
// Content protection task asked for HDCP state. The response is queued on the
// task runner. At this point clear the display state. Content protection task
// should re-query state and respond with failure since the display is no
// longer present.
layout_manager.set_displays({});
base::RunLoop().RunUntilIdle();
EXPECT_EQ(Response::FAILURE, response_);
EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
}
} // namespace test } // namespace test
} // namespace display } // namespace display
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