display: Fix analog display content protection
Content protection would be reported as enabled while mirroring to an
external display with an unsecure (e.g. analog) connection type.
Bug: 929449
Test: display_unittests
Change-Id: I4929ff4fa591616f27eaed01875455c86504e91b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1674404
Commit-Queue: Dominik Laskowski <domlaskowski@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680038}
diff --git a/ui/display/manager/content_protection_manager.cc b/ui/display/manager/content_protection_manager.cc
index 59e8613b..986d961 100644
--- a/ui/display/manager/content_protection_manager.cc
+++ b/ui/display/manager/content_protection_manager.cc
@@ -12,7 +12,6 @@
#include "base/stl_util.h"
#include "ui/display/manager/apply_content_protection_task.h"
#include "ui/display/manager/display_layout_manager.h"
-#include "ui/display/manager/display_util.h"
#include "ui/display/manager/query_content_protection_task.h"
#include "ui/display/types/display_constants.h"
#include "ui/display/types/display_snapshot.h"
@@ -79,10 +78,7 @@
QueryContentProtectionCallback callback) {
DCHECK(disabled() || GetContentProtections(client_id));
- // Exclude virtual displays so that protected content will not be recaptured
- // through the cast stream.
- const DisplaySnapshot* display = GetDisplay(display_id);
- if (disabled() || !display || !IsPhysicalDisplayType(display->type())) {
+ if (disabled() || !GetDisplay(display_id)) {
std::move(callback).Run(/*success=*/false, DISPLAY_CONNECTION_TYPE_NONE,
CONTENT_PROTECTION_METHOD_NONE);
return;
@@ -277,11 +273,6 @@
for (DisplaySnapshot* display : layout_manager_->GetDisplayStates()) {
int64_t display_id = display->display_id();
- if (!IsPhysicalDisplayType(display->type())) {
- NotifyDisplaySecurityObservers(display_id, /*secure=*/false);
- continue;
- }
-
QueueTask(std::make_unique<QueryContentProtectionTask>(
layout_manager_, native_display_delegate_, display_id,
base::BindOnce(&ContentProtectionManager::OnDisplaySecurityQueried,
@@ -300,18 +291,12 @@
(protection_mask != CONTENT_PROTECTION_METHOD_NONE ||
connection_mask == DISPLAY_CONNECTION_TYPE_INTERNAL);
- NotifyDisplaySecurityObservers(display_id, secure);
+ for (Observer& observer : observers_)
+ observer.OnDisplaySecurityChanged(display_id, secure);
}
if (status != Task::Status::KILLED)
DequeueTask();
}
-void ContentProtectionManager::NotifyDisplaySecurityObservers(
- int64_t display_id,
- bool secure) {
- for (Observer& observer : observers_)
- observer.OnDisplaySecurityChanged(display_id, secure);
-}
-
} // namespace display
diff --git a/ui/display/manager/content_protection_manager.h b/ui/display/manager/content_protection_manager.h
index 6364e93..0d4af4cb 100644
--- a/ui/display/manager/content_protection_manager.h
+++ b/ui/display/manager/content_protection_manager.h
@@ -158,7 +158,6 @@
Task::Status status,
uint32_t connection_mask,
uint32_t protection_mask);
- void NotifyDisplaySecurityObservers(int64_t display_id, bool secure);
DisplayLayoutManager* const layout_manager_; // Not owned.
NativeDisplayDelegate* native_display_delegate_ = nullptr; // Not owned.
diff --git a/ui/display/manager/content_protection_manager_unittest.cc b/ui/display/manager/content_protection_manager_unittest.cc
index 0a05e3ce..ee4c0fc 100644
--- a/ui/display/manager/content_protection_manager_unittest.cc
+++ b/ui/display/manager/content_protection_manager_unittest.cc
@@ -18,7 +18,7 @@
namespace {
-constexpr int64_t kDisplayIds[] = {123, 456};
+constexpr int64_t kDisplayIds[] = {123, 456, 789};
const DisplayMode kDisplayMode{gfx::Size(1366, 768), false, 60.0f};
} // namespace
@@ -67,6 +67,12 @@
.SetCurrentMode(kDisplayMode.Clone())
.Build();
+ displays_[2] = FakeDisplaySnapshot::Builder()
+ .SetId(kDisplayIds[2])
+ .SetType(DISPLAY_CONNECTION_TYPE_VGA)
+ .SetCurrentMode(kDisplayMode.Clone())
+ .Build();
+
UpdateDisplays(2);
}
@@ -122,7 +128,7 @@
uint32_t connection_mask_ = DISPLAY_CONNECTION_TYPE_NONE;
uint32_t protection_mask_ = CONTENT_PROTECTION_METHOD_NONE;
- std::unique_ptr<DisplaySnapshot> displays_[2];
+ std::unique_ptr<DisplaySnapshot> displays_[3];
DISALLOW_COPY_AND_ASSIGN(ContentProtectionManagerTest);
};
@@ -674,5 +680,81 @@
observer.security_changes());
}
+TEST_F(ContentProtectionManagerTest, AnalogDisplaySecurity) {
+ UpdateDisplays(3);
+ TestObserver observer(&manager_);
+
+ EXPECT_EQ(SecurityChanges({{kDisplayIds[0], true},
+ {kDisplayIds[1], false},
+ {kDisplayIds[2], false}}),
+ observer.security_changes());
+ observer.Reset();
+
+ auto id = manager_.RegisterClient();
+ EXPECT_TRUE(id);
+
+ native_display_delegate_.set_run_async(true);
+
+ for (int64_t display_id : kDisplayIds) {
+ manager_.ApplyContentProtection(
+ id, display_id, CONTENT_PROTECTION_METHOD_HDCP,
+ base::BindOnce(
+ &ContentProtectionManagerTest::ApplyContentProtectionCallback,
+ base::Unretained(this)));
+ }
+
+ base::RunLoop().RunUntilIdle();
+ EXPECT_TRUE(observer.security_changes().empty());
+
+ EXPECT_TRUE(TriggerDisplaySecurityTimeout());
+ base::RunLoop().RunUntilIdle();
+
+ // Analog display is never secure.
+ EXPECT_EQ(SecurityChanges({{kDisplayIds[0], true},
+ {kDisplayIds[1], true},
+ {kDisplayIds[2], false}}),
+ observer.security_changes());
+ observer.Reset();
+
+ layout_manager_.set_display_state(MULTIPLE_DISPLAY_STATE_MULTI_MIRROR);
+ TriggerDisplayConfiguration();
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(TriggerDisplaySecurityTimeout());
+ base::RunLoop().RunUntilIdle();
+
+ // Internal display is not secure if mirrored to an analog display.
+ EXPECT_EQ(SecurityChanges({{kDisplayIds[0], false},
+ {kDisplayIds[1], false},
+ {kDisplayIds[2], false}}),
+ observer.security_changes());
+ observer.Reset();
+
+ layout_manager_.set_display_state(MULTIPLE_DISPLAY_STATE_MULTI_EXTENDED);
+ TriggerDisplayConfiguration();
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(TriggerDisplaySecurityTimeout());
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_EQ(SecurityChanges({{kDisplayIds[0], true},
+ {kDisplayIds[1], true},
+ {kDisplayIds[2], false}}),
+ observer.security_changes());
+ observer.Reset();
+
+ manager_.UnregisterClient(id);
+
+ // Timer should be stopped when no client requests protection.
+ EXPECT_FALSE(TriggerDisplaySecurityTimeout());
+ base::RunLoop().RunUntilIdle();
+
+ // Observer should be notified when client unregisters.
+ EXPECT_EQ(SecurityChanges({{kDisplayIds[0], true},
+ {kDisplayIds[1], false},
+ {kDisplayIds[2], false}}),
+ observer.security_changes());
+}
+
} // namespace test
} // namespace display
diff --git a/ui/display/manager/query_content_protection_task.cc b/ui/display/manager/query_content_protection_task.cc
index 8488b96..a627324 100644
--- a/ui/display/manager/query_content_protection_task.cc
+++ b/ui/display/manager/query_content_protection_task.cc
@@ -48,8 +48,14 @@
return;
}
+ // Collect displays to be queried based on HDCP capability. For unprotected
+ // displays not inherently secure through an internal connection, record the
+ // existence of an unsecure display to report no protection for all displays
+ // in mirroring mode.
if (protection_mask & CONTENT_PROTECTION_METHOD_HDCP)
hdcp_capable_displays.push_back(display);
+ else if (display->type() != DISPLAY_CONNECTION_TYPE_INTERNAL)
+ no_protection_mask_ |= CONTENT_PROTECTION_METHOD_HDCP;
}
pending_requests_ = hdcp_capable_displays.size();
diff --git a/ui/display/manager/query_content_protection_task_unittest.cc b/ui/display/manager/query_content_protection_task_unittest.cc
index 69b2759c..7404e3b8 100644
--- a/ui/display/manager/query_content_protection_task_unittest.cc
+++ b/ui/display/manager/query_content_protection_task_unittest.cc
@@ -81,7 +81,7 @@
ASSERT_TRUE(response_);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_INTERNAL, response_->connection_mask);
- EXPECT_EQ(0u, response_->protection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
}
TEST_F(QueryContentProtectionTaskTest, QueryUnknownDisplay) {
@@ -99,7 +99,7 @@
ASSERT_TRUE(response_);
EXPECT_EQ(Status::FAILURE, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_UNKNOWN, response_->connection_mask);
- EXPECT_EQ(0u, response_->protection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
}
TEST_F(QueryContentProtectionTaskTest, QueryDisplayThatCannotGetHdcp) {
@@ -135,7 +135,7 @@
ASSERT_TRUE(response_);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
- EXPECT_EQ(0u, response_->protection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
}
TEST_F(QueryContentProtectionTaskTest, QueryDisplayWithHdcpEnabled) {
@@ -173,7 +173,7 @@
ASSERT_TRUE(response_);
EXPECT_EQ(Status::SUCCESS, response_->status);
EXPECT_EQ(DISPLAY_CONNECTION_TYPE_HDMI, response_->connection_mask);
- EXPECT_EQ(0u, response_->protection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
}
TEST_F(QueryContentProtectionTaskTest, QueryInMirroringMode) {
@@ -194,7 +194,63 @@
EXPECT_EQ(static_cast<uint32_t>(DISPLAY_CONNECTION_TYPE_HDMI |
DISPLAY_CONNECTION_TYPE_DVI),
response_->connection_mask);
- EXPECT_EQ(0u, response_->protection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
+}
+
+TEST_F(QueryContentProtectionTaskTest, QueryAnalogDisplay) {
+ std::vector<std::unique_ptr<DisplaySnapshot>> displays;
+ displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_VGA));
+ TestDisplayLayoutManager layout_manager(std::move(displays),
+ MULTIPLE_DISPLAY_STATE_SINGLE);
+
+ QueryContentProtectionTask task(
+ &layout_manager, &display_delegate_, 1,
+ base::Bind(&QueryContentProtectionTaskTest::ResponseCallback,
+ base::Unretained(this)));
+ task.Run();
+
+ ASSERT_TRUE(response_);
+ EXPECT_EQ(Status::SUCCESS, response_->status);
+ EXPECT_EQ(DISPLAY_CONNECTION_TYPE_VGA, response_->connection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
+}
+
+TEST_F(QueryContentProtectionTaskTest, QueryAnalogDisplayMirror) {
+ std::vector<std::unique_ptr<DisplaySnapshot>> displays;
+ displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_HDMI));
+ displays.push_back(CreateDisplaySnapshot(2, DISPLAY_CONNECTION_TYPE_VGA));
+ TestDisplayLayoutManager layout_manager(std::move(displays),
+ MULTIPLE_DISPLAY_STATE_MULTI_MIRROR);
+
+ display_delegate_.set_hdcp_state(HDCP_STATE_ENABLED);
+
+ QueryContentProtectionTask task1(
+ &layout_manager, &display_delegate_, 1,
+ base::Bind(&QueryContentProtectionTaskTest::ResponseCallback,
+ base::Unretained(this)));
+ task1.Run();
+
+ ASSERT_TRUE(response_);
+ EXPECT_EQ(Status::SUCCESS, response_->status);
+ EXPECT_EQ(static_cast<uint32_t>(DISPLAY_CONNECTION_TYPE_HDMI |
+ DISPLAY_CONNECTION_TYPE_VGA),
+ response_->connection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
+
+ response_.reset();
+
+ QueryContentProtectionTask task2(
+ &layout_manager, &display_delegate_, 2,
+ base::Bind(&QueryContentProtectionTaskTest::ResponseCallback,
+ base::Unretained(this)));
+ task2.Run();
+
+ ASSERT_TRUE(response_);
+ EXPECT_EQ(Status::SUCCESS, response_->status);
+ EXPECT_EQ(static_cast<uint32_t>(DISPLAY_CONNECTION_TYPE_HDMI |
+ DISPLAY_CONNECTION_TYPE_VGA),
+ response_->connection_mask);
+ EXPECT_EQ(CONTENT_PROTECTION_METHOD_NONE, response_->protection_mask);
}
} // namespace test
