Added switch for bypassing protected media identifier permission

chrome/browser/media/protected_media_identifier_whitelist_unittest.cc

Index: chrome/browser/media/protected_media_identifier_whitelist_unittest.cc
diff --git a/chrome/browser/media/protected_media_identifier_whitelist_unittest.cc b/chrome/browser/media/protected_media_identifier_whitelist_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e56309097e7ce6c6d06e95c6bdeae341e57b6a20
--- /dev/null
+++ b/chrome/browser/media/protected_media_identifier_whitelist_unittest.cc
@@ -0,0 +1,129 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media/protected_media_identifier_whitelist.h"
+#include "base/test/scoped_command_line.h"
+#include "chrome/common/chrome_switches.h"
+#include "media/base/media_switches.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+class ProtectedMediaIdentifierWhitelistTest : public testing::Test {};
+
+TEST_F(ProtectedMediaIdentifierWhitelistTest, NoEffect) {
+  const GURL requesting("https://example.com");
+  const ProtectedMediaIdentifierWhitelist whitelist;
+
+  // The request should need to ask for permission

[Joey Parrish, 2017/05/04 17:06:14]

I don't understand this comment in context of the next 6 lines of code.  It
looks like you are showing that none of the possible settings are modified when
the whitelist is empty.

[Vaage, 2017/05/04 17:42:30]

Done.

+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ALLOW, requesting));
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            whitelist.modifySettingFor(CONTENT_SETTING_BLOCK, requesting));
+
+  // Add the switch value that the whitelist reads from
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitch(switches::kUserDataDir);
+  command_line->AppendSwitchASCII(
+      switches::kDisableInfobarForProtectedMediaIdentifierForDomain,
+      "example.ca,example.edu");
+
+  // The request should not be affected as it was not added to the whitelist
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ALLOW, requesting));
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            whitelist.modifySettingFor(CONTENT_SETTING_BLOCK, requesting));
+}
+
+TEST_F(ProtectedMediaIdentifierWhitelistTest, IgnoresNonAsk) {
+  const GURL requesting("https://example.com");
+  const ProtectedMediaIdentifierWhitelist whitelist;
+
+  // Add the switch value that the whitelist reads from
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitch(switches::kUserDataDir);
+  command_line->AppendSwitchASCII(
+      switches::kDisableInfobarForProtectedMediaIdentifierForDomain,
+      "example.com");
+
+  // The whilelist should not change the settings for any non-ask settings
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ALLOW, requesting));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            whitelist.modifySettingFor(CONTENT_SETTING_BLOCK, requesting));
+}
+
+TEST_F(ProtectedMediaIdentifierWhitelistTest, BypassRequiresUserDataDir) {
+  const GURL requesting("https://example.com");
+  const ProtectedMediaIdentifierWhitelist whitelist;
+
+  // The request should need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+
+  // Add the switch value that the whitelist reads from
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitchASCII(
+      switches::kDisableInfobarForProtectedMediaIdentifierForDomain,
+      "example.ca,example.com,example.edu");
+
+  // The request should still need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+
+  // Set the user data dir so the request should no longer need to ask for
+  // permission
+  command_line->AppendSwitch(switches::kUserDataDir);
+
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+}
+
+TEST_F(ProtectedMediaIdentifierWhitelistTest, BypassWithFlag) {

[Joey Parrish, 2017/05/04 17:06:14]

From reading the test, I don't understand the purpose of it.  The name is also
not helping me understand it.  It appears that this covers exactly the same
things as the previous test.  Did I miss something?

[Vaage, 2017/05/04 17:42:29]

Done.

+  const GURL requesting("https://example.com");
+  const ProtectedMediaIdentifierWhitelist whitelist;
+
+  // The request should need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+
+  // Add the switch value that the whitelist reads from
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitch(switches::kUserDataDir);
+  command_line->AppendSwitchASCII(
+      switches::kDisableInfobarForProtectedMediaIdentifierForDomain,
+      "example.ca,example.com,example.edu");
+
+  // The request should no longer need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+}
+
+TEST_F(ProtectedMediaIdentifierWhitelistTest, BypassWithFlagAndSubdomain) {

[Joey Parrish, 2017/05/04 17:06:14]

I would love to have Xiaohan's feedback on this part.  I don't think it matters
to our testing whether or not subdomains are included.  Are there reasons they
should or should not be included?

[Vaage, 2017/05/04 17:42:29]

I found it easier to work with if I use the domain check as it meant that I
would only need to list the root domain if I was using sub domains and the check
appeared to be a more versatile check. The other methods of checking required
the string being converted to a URL and the check was more complicated . This
seemed like the more friendly and versatile method.

+  const GURL requesting("https://subdomain.example.com");
+  const ProtectedMediaIdentifierWhitelist whitelist;
+
+  // The request should need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+
+  // Add the switch value that the whitelist reads from
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitch(switches::kUserDataDir);
+  command_line->AppendSwitchASCII(
+      switches::kDisableInfobarForProtectedMediaIdentifierForDomain,
+      "example.com");
+
+  // The request should no longer need to ask for permission
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            whitelist.modifySettingFor(CONTENT_SETTING_ASK, requesting));
+}