Unit tests for output protection in output configurator

chromeos/display/output_configurator_unittest.cc

Index: chromeos/display/output_configurator_unittest.cc
diff --git a/chromeos/display/output_configurator_unittest.cc b/chromeos/display/output_configurator_unittest.cc
index 18b2bdc290ff02a305969ec800baf1256d4b9f0c..ea05fdd7eab45229635028a3d72a20f626ac412d 100644
--- a/chromeos/display/output_configurator_unittest.cc
+++ b/chromeos/display/output_configurator_unittest.cc
@@ -72,6 +72,16 @@ std::string GetCTMAction(
       ctm.x_scale, ctm.x_offset, ctm.y_scale, ctm.y_offset);
 }
 
+// Returns a string describing a TestDelegate::GetHDCPState() call.
+std::string GetGetHDCPStateAction(RROutput id) {
+  return base::StringPrintf("get_hdcp(id=%lu)", id);

[Daniel Erat, 2013/09/30 14:01:16]

i wouldn't bother with an action for this Get*() method; checking that
OutputConfigurator has the state that was previously set via set_hdcp_state() is
sufficient to make sure that it's being called. see e.g. GetOutputs(), which is
in a similar situation

[kcwu, 2013/10/01 07:15:51]

Done.

+}
+
+// Returns a string describing a TestDelegate::SetHDCPState() call.
+std::string GetSetHDCPStateAction(RROutput id, HDCPState state) {
+  return base::StringPrintf("set_hdcp(id=%lu, state=%d)", id, state);

[Daniel Erat, 2013/09/30 14:01:16]

nit: remove space after the comma in the string to match other actions

[kcwu, 2013/10/01 07:15:51]

Done.

+}
+
 // Joins a sequence of strings describing actions (e.g. kScreenDim) such
 // that they can be compared against a string returned by
 // TestDelegate::GetActionsAndClear().  The list of actions must be
@@ -95,7 +105,9 @@ class TestDelegate : public OutputConfigurator::Delegate {
  public:
   static const int kXRandREventBase = 10;
 
-  TestDelegate() : configure_crtc_result_(true) {}
+  TestDelegate()
+      : configure_crtc_result_(true),
+        hdcp_state_(HDCP_STATE_UNDESIRED) {}
   virtual ~TestDelegate() {}
 
   const std::vector<OutputConfigurator::OutputSnapshot>& outputs() const {
@@ -110,6 +122,10 @@ class TestDelegate : public OutputConfigurator::Delegate {
     configure_crtc_result_ = result;
   }
 
+  void set_hdcp_state(HDCPState state) {

[Daniel Erat, 2013/09/30 14:01:16]

nit: move this all to one line since it'll fit:

  void set_hdcp_state(HDCPState state) { hdcp_state_ = state; }

[kcwu, 2013/10/01 07:15:51]

Done.

+    hdcp_state_ = state;
+  }
+
   // Returns a comma-separated string describing the actions that were
   // requested since the previous call to GetActionsAndClear() (i.e.
   // results are non-repeatable).
@@ -168,10 +184,13 @@ class TestDelegate : public OutputConfigurator::Delegate {
   }
 
   virtual bool GetHDCPState(RROutput id, HDCPState* state) OVERRIDE {
+    AppendAction(GetGetHDCPStateAction(id));
+    *state = hdcp_state_;
     return true;
   }
 
   virtual bool SetHDCPState(RROutput id, HDCPState state) OVERRIDE {
+    AppendAction(GetSetHDCPStateAction(id, state));
     return true;
   }
 
@@ -204,6 +223,9 @@ class TestDelegate : public OutputConfigurator::Delegate {
   // Return value returned by ConfigureCrtc().
   bool configure_crtc_result_;
 
+  // Result value of GetHDCPState().
+  HDCPState hdcp_state_;
+
   DISALLOW_COPY_AND_ASSIGN(TestDelegate);
 };
 
@@ -340,6 +362,7 @@ class OutputConfiguratorTest : public testing::Test {
     o->current_mode = kSmallModeId;
     o->native_mode = kSmallModeId;
     o->is_internal = true;
+    o->type = OUTPUT_TYPE_INTERNAL;
     o->is_aspect_preserving_scaling = true;
     o->mode_infos[kSmallModeId] = small_mode_info;
     o->has_display_id = true;
@@ -351,6 +374,7 @@ class OutputConfiguratorTest : public testing::Test {
     o->current_mode = kBigModeId;
     o->native_mode = kBigModeId;
     o->is_internal = false;
+    o->type = OUTPUT_TYPE_HDMI;
     o->is_aspect_preserving_scaling = true;
     o->mode_infos[kSmallModeId] = small_mode_info;
     o->mode_infos[kBigModeId] = big_mode_info;
@@ -1090,4 +1114,106 @@ TEST_F(OutputConfiguratorTest, PanelFitting) {
   EXPECT_EQ(kSmallModeHeight, info->height);
 }
 
+TEST_F(OutputConfiguratorTest, OutputProtection) {
+  uint32_t link_mask;
+  uint32_t protection_mask;

[Daniel Erat, 2013/09/30 14:01:16]

nit: move these down to the point where they're first used and initialize them
to 0
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Local_Variables)

[kcwu, 2013/10/01 07:15:51]

Done.

+
+  OutputConfigurator::OutputProtectionClientId id =
+      configurator_.RegisterOutputProtectionClient();
+  EXPECT_NE(0u, id);
+
+  // One output.
+  UpdateOutputs(1, true);
+  configurator_.Start(0);
+  EXPECT_NE(kNoActions, delegate_->GetActionsAndClear());
+  EXPECT_TRUE(configurator_.QueryOutputProtectionStatus(id, &link_mask,
+                                                        &protection_mask));
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_TYPE_INTERNAL), link_mask);
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_PROTECTION_METHOD_NONE),
+            protection_mask);
+  EXPECT_EQ(kNoActions, delegate_->GetActionsAndClear());
+
+  // Two outputs.
+  UpdateOutputs(2, true);
+  configurator_.Start(0);

[Daniel Erat, 2013/09/30 14:01:16]

calling Start() multiple times isn't expected behavior; UpdateOutputs(..., true)
should be enough to pick up the new display. note that doing so will generate
bunch of extra actions, but i'd recommend just adding a bare
GetActionsAndClear() call to clear them.

[kcwu, 2013/10/01 07:15:51]

Done.

+  EXPECT_NE(kNoActions, delegate_->GetActionsAndClear());
+  EXPECT_TRUE(configurator_.QueryOutputProtectionStatus(id, &link_mask,
+                                                        &protection_mask));
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_TYPE_INTERNAL | OUTPUT_TYPE_HDMI),
+            link_mask);
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_PROTECTION_METHOD_NONE),
+            protection_mask);
+  EXPECT_EQ(GetGetHDCPStateAction(outputs_[1].output),
+            delegate_->GetActionsAndClear());
+
+  EXPECT_TRUE(
+      configurator_.EnableOutputProtection(id, OUTPUT_PROTECTION_METHOD_HDCP));
+  EXPECT_EQ(GetSetHDCPStateAction(outputs_[1].output, HDCP_STATE_DESIRED),
+            delegate_->GetActionsAndClear());
+
+  // Enable protection.
+  delegate_->set_hdcp_state(HDCP_STATE_ENABLED);
+  EXPECT_TRUE(configurator_.QueryOutputProtectionStatus(id, &link_mask,
+                                                        &protection_mask));
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_TYPE_INTERNAL | OUTPUT_TYPE_HDMI),
+            link_mask);
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_PROTECTION_METHOD_HDCP),
+            protection_mask);
+  EXPECT_EQ(GetGetHDCPStateAction(outputs_[1].output),
+            delegate_->GetActionsAndClear());
+}
+
+TEST_F(OutputConfiguratorTest, OutputProtectionTwoClients) {
+  uint32_t link_mask;
+  uint32_t protection_mask;
+
+  OutputConfigurator::OutputProtectionClientId client1 =
+      configurator_.RegisterOutputProtectionClient();
+  OutputConfigurator::OutputProtectionClientId client2 =
+      configurator_.RegisterOutputProtectionClient();
+  EXPECT_NE(client1, client2);
+
+  UpdateOutputs(2, true);
+  configurator_.Start(0);
+  EXPECT_NE(kNoActions, delegate_->GetActionsAndClear());
+
+  // Clients never know state enableness for methods that they didn't request.
+  EXPECT_TRUE(
+      configurator_.EnableOutputProtection(client1,
+                                           OUTPUT_PROTECTION_METHOD_HDCP));
+  delegate_->set_hdcp_state(HDCP_STATE_ENABLED);
+
+  EXPECT_TRUE(configurator_.QueryOutputProtectionStatus(client1, &link_mask,
+                                                        &protection_mask));
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_TYPE_INTERNAL | OUTPUT_TYPE_HDMI),
+            link_mask);
+  EXPECT_EQ(OUTPUT_PROTECTION_METHOD_HDCP, protection_mask);
+
+  EXPECT_TRUE(configurator_.QueryOutputProtectionStatus(client2, &link_mask,
+                                                        &protection_mask));
+  EXPECT_EQ(static_cast<uint32_t>(OUTPUT_TYPE_INTERNAL | OUTPUT_TYPE_HDMI),
+            link_mask);
+  EXPECT_EQ(OUTPUT_PROTECTION_METHOD_NONE, protection_mask);
+  EXPECT_EQ(JoinActions(GetSetHDCPStateAction(outputs_[1].output,
+                                              HDCP_STATE_DESIRED).c_str(),

[Daniel Erat, 2013/09/30 14:01:16]

please move the call to check the Set action up to just after you call
EnableOutputProtection()

[kcwu, 2013/10/01 07:15:51]

Done.

+                        GetGetHDCPStateAction(outputs_[1].output).c_str(),
+                        GetGetHDCPStateAction(outputs_[1].output).c_str(),
+                        NULL),
+            delegate_->GetActionsAndClear());
+
+  // Protections will be disabled only if no more clients request for.

[Daniel Erat, 2013/09/30 14:01:16]

nit: s/request for/request them/

+  EXPECT_TRUE(
+      configurator_.EnableOutputProtection(client2,
+                                           OUTPUT_PROTECTION_METHOD_NONE));
+  EXPECT_EQ(GetSetHDCPStateAction(outputs_[1].output,
+                                  HDCP_STATE_DESIRED).c_str(),
+            delegate_->GetActionsAndClear());
+  EXPECT_TRUE(
+      configurator_.EnableOutputProtection(client1,
+                                           OUTPUT_PROTECTION_METHOD_NONE));
+  EXPECT_EQ(GetSetHDCPStateAction(outputs_[1].output,
+                                  HDCP_STATE_UNDESIRED).c_str(),
+            delegate_->GetActionsAndClear());
+}
+
 }  // namespace chromeos