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}

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 @@ void ContentProtectionManager::QueryContentProtection(
     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 @@ void ContentProtectionManager::QueueDisplaySecurityQueries() {
   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 @@ void ContentProtectionManager::OnDisplaySecurityQueried(
                         (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

ui/display/manager/content_protection_manager.h

@@ -158,7 +158,6 @@ class DISPLAY_MANAGER_EXPORT ContentProtectionManager
                                 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.

ui/display/manager/content_protection_manager_unittest.cc

@@ -18,7 +18,7 @@ namespace test {
 
 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 @@ class ContentProtectionManagerTest : public testing::Test {
                        .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 @@ class ContentProtectionManagerTest : public testing::Test {
   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 @@ TEST_F(ContentProtectionManagerTest, NoSecurityPollingIfInternalDisplayOnly) {
             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

ui/display/manager/query_content_protection_task.cc

@@ -48,8 +48,14 @@ void QueryContentProtectionTask::Run() {
       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();

ui/display/manager/query_content_protection_task_unittest.cc

@@ -81,7 +81,7 @@ TEST_F(QueryContentProtectionTaskTest, QueryInternalDisplay) {
   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 @@ TEST_F(QueryContentProtectionTaskTest, QueryUnknownDisplay) {
   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 @@ TEST_F(QueryContentProtectionTaskTest, QueryDisplayWithHdcpDisabled) {
   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 @@ TEST_F(QueryContentProtectionTaskTest, QueryInMultiDisplayMode) {
   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 @@ TEST_F(QueryContentProtectionTaskTest, QueryInMirroringMode) {
   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