Added switch for bypassing protected media identifier permission

chrome/browser/media/protected_media_identifier_permission_context.cc

Index: chrome/browser/media/protected_media_identifier_permission_context.cc
diff --git a/chrome/browser/media/protected_media_identifier_permission_context.cc b/chrome/browser/media/protected_media_identifier_permission_context.cc
index 50c2dc51ec53a1f4269eff81c038c44fc6eb40bc..13514a2af890e398410da926fabf8acde6e64ac5 100644
--- a/chrome/browser/media/protected_media_identifier_permission_context.cc
+++ b/chrome/browser/media/protected_media_identifier_permission_context.cc
@@ -6,6 +6,7 @@
 
 #include "base/command_line.h"
 #include "base/metrics/user_metrics.h"
+#include "base/strings/string_split.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/permissions/permission_util.h"
@@ -15,6 +16,8 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
+#include "media/base/media_switches.h"
+#include "net/base/url_util.h"
 #if defined(OS_CHROMEOS)
 #include <utility>
 
@@ -101,6 +104,37 @@ ProtectedMediaIdentifierPermissionContext::GetPermissionStatusInternal(
          content_setting == CONTENT_SETTING_BLOCK ||
          content_setting == CONTENT_SETTING_ASK);
 
+  // For automated testing of protected content - having a prompt that requires
+  // user intervention is problematic. If the domain has been whitelisted as
+  // safe - suppress the request and allow.

[raymes, 2017/04/20 23:12:42]

I think we should link to a bug or something that mentions the specific reason
this was added. Specifically, it's not because we need it for automated tests in
Chrome but because widevine needs it.

[Joey Parrish, 2017/04/20 23:25:56]

We've got b/35045892 filed internally, if that's helpful.  We don't have any
public-facing bug filed on chromium, since we decided to take it upon ourselves
to contribute a patch.

+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+  if (content_setting == CONTENT_SETTING_ASK &&
+      command_line.HasSwitch(
+          switches::kDisableInfobarForProtectedMediaIdentifierForDomain)) {

[assareh1, 2017/04/19 18:13:34]

As we had discussed offline, can you also verify that
"command_line.HasSwitch(switches::kUserDataDir)"?  This should make it likely
that a user is tricked into using this flag on their main profile.

[assareh1, 2017/04/19 18:15:08]

*less* likely  :)

+    // Convert the switch value to a vector of URLs
+    std::vector<std::string> origins;
+    std::string whitelist = command_line.GetSwitchValueASCII(
+        switches::kDisableInfobarForProtectedMediaIdentifierForDomain);
+    for (const std::string& origin : base::SplitString(
+             whitelist, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)) {
+      origins.push_back(origin);
+    }

[raymes, 2017/04/19 23:05:47]

Is this extra complexity really necessary? Was the localhost check not enough?

[assareh1, 2017/04/20 15:37:07]

By whitelisting a user-specified domain vs only localhost, we significantly
expand the usefulness of this flag. In most cases, it is unlikely that the web
service can be hosted on an Android device. Therefore this flag requires
additional tunneling to be useful. If we can whitelist the actual domain being
used in test then we can remove the need for setting up adb tunnels. 

[raymes, 2017/04/20 23:12:42]

Ok. I would suggest rather than comparing the host, that you construct a GURL
from the string and then compare equality.

+
+    // Only if both requesting and embedding are allowed, allow the prompt to
+    // be skipped.
+    bool found_requesting = false;
+    bool found_embedding = false;
+    for (size_t i = 0; i < origins.size(); i++) {

[raymes, 2017/04/20 23:12:42]

Rather than looping through everything again, can you just do it all in the loop
above and not have the vector?

+      found_requesting |= origins.at(i) == requesting_origin.host();
+      found_embedding |= origins.at(i) == embedding_origin.host();
+    }

[raymes, 2017/04/20 23:12:42]

I would suggest just we just compare the requesting origin here - I don't think
it will be too important to use the embedding origin:

if (requesting_origin == GURL(whitelisted_origin))
  return CONTENT_SETTING_ALLOW;

+
+    if (found_requesting && found_embedding) {
+      content_setting = CONTENT_SETTING_ALLOW;
+    }
+  }
+
   return content_setting;
 }