display: Refactor ApplyContentProtectionTask

This CL modernizes style and removes repetitive code.

Bug: 929449
Test: display_unittests
Change-Id: I55e360d833a5dae345c071f0ba431cd89d68f39e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553504Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Auto-Submit: Dominik Laskowski <domlaskowski@chromium.org>
Commit-Queue: Dominik Laskowski <domlaskowski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648732}

ui/display/manager/apply_content_protection_task.cc

@@ -4,8 +4,12 @@
 
 #include "ui/display/manager/apply_content_protection_task.h"
 
+#include <utility>
+#include <vector>
+
 #include "base/bind.h"
 #include "ui/display/manager/display_layout_manager.h"
+#include "ui/display/manager/display_util.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/display/types/native_display_delegate.h"
 
@@ -17,24 +21,12 @@ bool GetHDCPCapableDisplays(
     const DisplayLayoutManager& layout_manager,
     std::vector<DisplaySnapshot*>* hdcp_capable_displays) {
   for (DisplaySnapshot* display : layout_manager.GetDisplayStates()) {
-    switch (display->type()) {
-      case DISPLAY_CONNECTION_TYPE_UNKNOWN:
-        return false;
-      // DisplayPort, DVI, and HDMI all support HDCP.
-      case DISPLAY_CONNECTION_TYPE_DISPLAYPORT:
-      case DISPLAY_CONNECTION_TYPE_DVI:
-      case DISPLAY_CONNECTION_TYPE_HDMI:
-        hdcp_capable_displays->push_back(display);
-        break;
-      case DISPLAY_CONNECTION_TYPE_INTERNAL:
-      case DISPLAY_CONNECTION_TYPE_VGA:
-      case DISPLAY_CONNECTION_TYPE_NETWORK:
-        // No protections for these types. Do nothing.
-        break;
-      case DISPLAY_CONNECTION_TYPE_NONE:
-        NOTREACHED();
-        break;
-    }
+    uint32_t protection_mask;
+    if (!GetContentProtectionMethods(display->type(), &protection_mask))
+      return false;
+
+    if (protection_mask & CONTENT_PROTECTION_METHOD_HDCP)
+      hdcp_capable_displays->push_back(display);
   }
 
   return true;
@@ -45,28 +37,25 @@ bool GetHDCPCapableDisplays(
 ApplyContentProtectionTask::ApplyContentProtectionTask(
     DisplayLayoutManager* layout_manager,
     NativeDisplayDelegate* native_display_delegate,
-    const DisplayConfigurator::ContentProtections& requests,
-    const ResponseCallback& callback)
+    DisplayConfigurator::ContentProtections requests,
+    ResponseCallback callback)
     : layout_manager_(layout_manager),
       native_display_delegate_(native_display_delegate),
-      requests_(requests),
-      callback_(callback),
-      query_status_(true),
-      pending_requests_(0),
-      weak_ptr_factory_(this) {}
+      requests_(std::move(requests)),
+      callback_(std::move(callback)) {}
 
 ApplyContentProtectionTask::~ApplyContentProtectionTask() {}
 
 void ApplyContentProtectionTask::Run() {
   std::vector<DisplaySnapshot*> hdcp_capable_displays;
   if (!GetHDCPCapableDisplays(*layout_manager_, &hdcp_capable_displays)) {
-    callback_.Run(false);
+    std::move(callback_).Run(/*success=*/false);
     return;
   }
 
   pending_requests_ = hdcp_capable_displays.size();
   if (pending_requests_ == 0) {
-    callback_.Run(true);
+    std::move(callback_).Run(/*success=*/true);
     return;
   }
 
@@ -75,24 +64,24 @@ void ApplyContentProtectionTask::Run() {
   for (DisplaySnapshot* display : hdcp_capable_displays) {
     native_display_delegate_->GetHDCPState(
         *display,
-        base::Bind(&ApplyContentProtectionTask::OnHDCPStateUpdate,
-                   weak_ptr_factory_.GetWeakPtr(), display->display_id()));
+        base::BindOnce(&ApplyContentProtectionTask::OnGetHDCPState,
+                       weak_ptr_factory_.GetWeakPtr(), display->display_id()));
   }
 }
 
-void ApplyContentProtectionTask::OnHDCPStateUpdate(int64_t display_id,
-                                                   bool success,
-                                                   HDCPState state) {
-  query_status_ &= success;
-  display_hdcp_state_map_[display_id] = state;
+void ApplyContentProtectionTask::OnGetHDCPState(int64_t display_id,
+                                                bool success,
+                                                HDCPState state) {
+  success_ &= success;
+  hdcp_states_[display_id] = state;
   pending_requests_--;
 
   // Wait for all the requests before continuing.
   if (pending_requests_ != 0)
     return;
 
-  if (!query_status_) {
-    callback_.Run(false);
+  if (!success_) {
+    std::move(callback_).Run(/*success=*/false);
     return;
   }
 
@@ -102,7 +91,7 @@ void ApplyContentProtectionTask::OnHDCPStateUpdate(int64_t display_id,
 void ApplyContentProtectionTask::ApplyProtections() {
   std::vector<DisplaySnapshot*> hdcp_capable_displays;
   if (!GetHDCPCapableDisplays(*layout_manager_, &hdcp_capable_displays)) {
-    callback_.Run(false);
+    std::move(callback_).Run(/*success=*/false);
     return;
   }
 
@@ -111,18 +100,18 @@ void ApplyContentProtectionTask::ApplyProtections() {
   for (DisplaySnapshot* display : hdcp_capable_displays) {
     uint32_t desired_mask = GetDesiredProtectionMask(display->display_id());
 
-    auto it = display_hdcp_state_map_.find(display->display_id());
+    auto it = hdcp_states_.find(display->display_id());
     // If the display can't be found, the display configuration changed.
-    if (it == display_hdcp_state_map_.end()) {
-      callback_.Run(false);
+    if (it == hdcp_states_.end()) {
+      std::move(callback_).Run(/*success=*/false);
       return;
     }
 
     bool hdcp_enabled = it->second != HDCP_STATE_UNDESIRED;
     bool hdcp_requested = desired_mask & CONTENT_PROTECTION_METHOD_HDCP;
     if (hdcp_enabled != hdcp_requested) {
-      hdcp_requests.push_back(std::make_pair(
-          display, hdcp_requested ? HDCP_STATE_DESIRED : HDCP_STATE_UNDESIRED));
+      hdcp_requests.emplace_back(
+          display, hdcp_requested ? HDCP_STATE_DESIRED : HDCP_STATE_UNDESIRED);
     }
   }
 
@@ -130,24 +119,24 @@ 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) {
-    callback_.Run(true);
+    std::move(callback_).Run(/*success=*/true);
     return;
   }
 
   for (const auto& pair : hdcp_requests) {
     native_display_delegate_->SetHDCPState(
         *pair.first, pair.second,
-        base::Bind(&ApplyContentProtectionTask::OnHDCPStateApplied,
-                   weak_ptr_factory_.GetWeakPtr()));
+        base::BindOnce(&ApplyContentProtectionTask::OnSetHDCPState,
+                       weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
-void ApplyContentProtectionTask::OnHDCPStateApplied(bool success) {
-  query_status_ &= success;
+void ApplyContentProtectionTask::OnSetHDCPState(bool success) {
+  success_ &= success;
   pending_requests_--;
 
   if (pending_requests_ == 0)
-    callback_.Run(query_status_);
+    std::move(callback_).Run(success_);
 }
 
 uint32_t ApplyContentProtectionTask::GetDesiredProtectionMask(
@@ -157,7 +146,7 @@ uint32_t ApplyContentProtectionTask::GetDesiredProtectionMask(
   // In non-mirror mode, only request of client's display needs to be
   // fulfilled.
   if (layout_manager_->IsMirroring()) {
-    for (auto pair : requests_)
+    for (const auto& pair : requests_)
       desired_mask |= pair.second;
   } else {
     auto it = requests_.find(display_id);

ui/display/manager/apply_content_protection_task.h

@@ -5,12 +5,10 @@
 #ifndef UI_DISPLAY_MANAGER_APPLY_CONTENT_PROTECTION_TASK_H_
 #define UI_DISPLAY_MANAGER_APPLY_CONTENT_PROTECTION_TASK_H_
 
-#include <stddef.h>
-#include <stdint.h>
-
-#include <map>
-#include <vector>
+#include <cstddef>
+#include <cstdint>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "ui/display/manager/display_configurator.h"
@@ -24,61 +22,47 @@ class NativeDisplayDelegate;
 // manner:
 // 1) Run()
 //   a) Query NativeDisplayDelegate for HDCP state on capable displays
-//   b) OnHDCPStateUpdate() called for each display as response to (a)
+//   b) OnGetHDCPState() called for each display as response to (a)
 // 2) ApplyProtections()
 //   a) Compute preferred HDCP state for capable displays
 //   b) Call into NativeDisplayDelegate to set HDCP state on capable displays
-//   c) OnHDCPStateApplied() called for each display as reponse to (b)
+//   c) OnSetHDCPState() called for each display as response to (b)
 // 3) Call |callback_| to signal end of task.
 //
 // 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 {
  public:
-  typedef base::Callback<void(bool)> ResponseCallback;
+  using ResponseCallback = base::OnceCallback<void(bool success)>;
 
-  ApplyContentProtectionTask(
-      DisplayLayoutManager* layout_manager,
-      NativeDisplayDelegate* native_display_delegate,
-      const DisplayConfigurator::ContentProtections& requests,
-      const ResponseCallback& callback);
+  ApplyContentProtectionTask(DisplayLayoutManager* layout_manager,
+                             NativeDisplayDelegate* native_display_delegate,
+                             DisplayConfigurator::ContentProtections requests,
+                             ResponseCallback callback);
   ~ApplyContentProtectionTask();
 
   void Run();
 
  private:
-  // Callback to NatvieDisplayDelegate::GetHDCPState()
-  void OnHDCPStateUpdate(int64_t display_id, bool success, HDCPState state);
-
-  // Callback to NativeDisplayDelegate::SetHDCPState()
-  void OnHDCPStateApplied(bool success);
+  void OnGetHDCPState(int64_t display_id, bool success, HDCPState state);
+  void OnSetHDCPState(bool success);
 
   void ApplyProtections();
 
   uint32_t GetDesiredProtectionMask(int64_t display_id) const;
 
-  DisplayLayoutManager* layout_manager_;  // Not owned.
-
-  NativeDisplayDelegate* native_display_delegate_;  // Not owned.
+  DisplayLayoutManager* const layout_manager_;            // Not owned.
+  NativeDisplayDelegate* const native_display_delegate_;  // Not owned.
 
-  DisplayConfigurator::ContentProtections requests_;
-
-  // Callback used to respond once the task finishes.
+  const DisplayConfigurator::ContentProtections requests_;
   ResponseCallback callback_;
 
-  // Mapping between display IDs and the HDCP state returned by
-  // NativeDisplayDelegate.
-  std::map<int64_t, HDCPState> display_hdcp_state_map_;
-
-  // Tracks the status of the NativeDisplayDelegate responses. This will be true
-  // if all the queries were successful, false otherwise.
-  bool query_status_;
+  base::flat_map<int64_t /* display_id */, HDCPState> hdcp_states_;
 
-  // Tracks the number of NativeDisplayDelegate requests sent but not answered
-  // yet.
-  size_t pending_requests_;
+  bool success_ = true;
+  size_t pending_requests_ = 0;
 
-  base::WeakPtrFactory<ApplyContentProtectionTask> weak_ptr_factory_;
+  base::WeakPtrFactory<ApplyContentProtectionTask> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ApplyContentProtectionTask);
 };

ui/display/manager/apply_content_protection_task_unittest.cc

@@ -37,28 +37,29 @@ std::unique_ptr<DisplaySnapshot> CreateDisplaySnapshot(
 
 class ApplyContentProtectionTaskTest : public testing::Test {
  public:
-  enum Response {
+  enum class Response {
     ERROR,
     SUCCESS,
     NOT_CALLED,
   };
 
-  ApplyContentProtectionTaskTest()
-      : response_(NOT_CALLED), display_delegate_(&log_) {}
-  ~ApplyContentProtectionTaskTest() override {}
+  ApplyContentProtectionTaskTest() = default;
+  ~ApplyContentProtectionTaskTest() override = default;
 
-  void ResponseCallback(bool success) { response_ = success ? SUCCESS : ERROR; }
+  void ResponseCallback(bool success) {
+    response_ = success ? Response::SUCCESS : Response::ERROR;
+  }
 
  protected:
-  Response response_;
+  Response response_ = Response::NOT_CALLED;
   ActionLogger log_;
-  TestNativeDisplayDelegate display_delegate_;
+  TestNativeDisplayDelegate display_delegate_{&log_};
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ApplyContentProtectionTaskTest);
 };
 
-TEST_F(ApplyContentProtectionTaskTest, ApplyWithNoHDCPCapableDisplay) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToInternalDisplay) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(
       CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_INTERNAL));
@@ -73,11 +74,11 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyWithNoHDCPCapableDisplay) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(SUCCESS, response_);
+  EXPECT_EQ(Response::SUCCESS, response_);
   EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
 }
 
-TEST_F(ApplyContentProtectionTaskTest, ApplyWithHDMIDisplay) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToExternalDisplay) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_HDMI));
   TestDisplayLayoutManager layout_manager(std::move(displays),
@@ -91,7 +92,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyWithHDMIDisplay) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(SUCCESS, response_);
+  EXPECT_EQ(Response::SUCCESS, response_);
   EXPECT_EQ(
       JoinActions(GetSetHDCPStateAction(*layout_manager.GetDisplayStates()[0],
                                         HDCP_STATE_DESIRED)
@@ -100,7 +101,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyWithHDMIDisplay) {
       log_.GetActionsAndClear());
 }
 
-TEST_F(ApplyContentProtectionTaskTest, ApplyWithUnknownDisplay) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToUnknownDisplay) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_UNKNOWN));
   TestDisplayLayoutManager layout_manager(std::move(displays),
@@ -114,11 +115,11 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyWithUnknownDisplay) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(ERROR, response_);
+  EXPECT_EQ(Response::ERROR, response_);
   EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
 }
 
-TEST_F(ApplyContentProtectionTaskTest, FailGettingHDCPState) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToDisplayThatCannotGetHdcp) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_HDMI));
   TestDisplayLayoutManager layout_manager(std::move(displays),
@@ -133,11 +134,11 @@ TEST_F(ApplyContentProtectionTaskTest, FailGettingHDCPState) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(ERROR, response_);
+  EXPECT_EQ(Response::ERROR, response_);
   EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
 }
 
-TEST_F(ApplyContentProtectionTaskTest, FailSettingHDCPState) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyHdcpToDisplayThatCannotSetHdcp) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_HDMI));
   TestDisplayLayoutManager layout_manager(std::move(displays),
@@ -152,7 +153,7 @@ TEST_F(ApplyContentProtectionTaskTest, FailSettingHDCPState) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(ERROR, response_);
+  EXPECT_EQ(Response::ERROR, response_);
   EXPECT_EQ(
       JoinActions(GetSetHDCPStateAction(*layout_manager.GetDisplayStates()[0],
                                         HDCP_STATE_DESIRED)
@@ -161,7 +162,7 @@ TEST_F(ApplyContentProtectionTaskTest, FailSettingHDCPState) {
       log_.GetActionsAndClear());
 }
 
-TEST_F(ApplyContentProtectionTaskTest, ApplyNoopProtection) {
+TEST_F(ApplyContentProtectionTaskTest, ApplyNoProtectionToExternalDisplay) {
   std::vector<std::unique_ptr<DisplaySnapshot>> displays;
   displays.push_back(CreateDisplaySnapshot(1, DISPLAY_CONNECTION_TYPE_HDMI));
   TestDisplayLayoutManager layout_manager(std::move(displays),
@@ -176,7 +177,7 @@ TEST_F(ApplyContentProtectionTaskTest, ApplyNoopProtection) {
                  base::Unretained(this)));
   task.Run();
 
-  EXPECT_EQ(SUCCESS, response_);
+  EXPECT_EQ(Response::SUCCESS, response_);
   EXPECT_EQ(kNoActions, log_.GetActionsAndClear());
 }