media: Refactor PlatformVerificationFlow.

chrome/browser/media/protected_media_identifier_permission_context.cc

 // Copyright 2013 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_permission_context.h"
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/common/permission_request_id.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 
 #if defined(OS_CHROMEOS)
 #include <utility>
 
 #include "chrome/browser/chromeos/attestation/platform_verification_dialog.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chromeos/settings/cros_settings_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/user_prefs/user_prefs.h"
 #include "ui/views/widget/widget.h"
 
 using chromeos::attestation::PlatformVerificationDialog;
-using chromeos::attestation::PlatformVerificationFlow;
 #endif
 
 ProtectedMediaIdentifierPermissionContext::
     ProtectedMediaIdentifierPermissionContext(Profile* profile)
     : PermissionContextBase(profile,
                             CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER)
 #if defined(OS_CHROMEOS)
       ,
       weak_factory_(this)
 #endif
 {
 }
 
 ProtectedMediaIdentifierPermissionContext::
     ~ProtectedMediaIdentifierPermissionContext() {
 }
 
+#if defined(OS_CHROMEOS)
+// static
+void ProtectedMediaIdentifierPermissionContext::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* prefs) {
+  prefs->RegisterBooleanPref(prefs::kRAConsentGranted,
+                             false,  // Default value.
+                             user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+}
+#endif
+
 void ProtectedMediaIdentifierPermissionContext::RequestPermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
 
+  DVLOG(1) << __FUNCTION__ << ": (" << requesting_origin.spec() << ", "
+           << embedding_origin.spec() << ")";
+
   if (!requesting_origin.is_valid() || !embedding_origin.is_valid() ||
       !IsProtectedMediaIdentifierEnabled()) {
     NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
                         false /* persist */, CONTENT_SETTING_BLOCK);
     return;
   }
 
 #if defined(OS_CHROMEOS)
-  // On ChromeOS, we don't use PermissionContextBase::RequestPermission() which
-  // uses the standard permission infobar/bubble UI. See http://crbug.com/454847
-  // Instead, we check the content setting and show the existing platform
-  // verification UI.
-  // TODO(xhwang): Remove when http://crbug.com/454847 is fixed.
-  ContentSetting content_setting =
-      GetPermissionStatus(requesting_origin, embedding_origin);
-
-  if (content_setting == CONTENT_SETTING_ALLOW ||
-      content_setting == CONTENT_SETTING_BLOCK) {
+  ContentSetting content_setting = PermissionContextBase::GetPermissionStatus(
+      requesting_origin, embedding_origin);
+  if (content_setting == CONTENT_SETTING_BLOCK) {
     NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                        false /* persist */, content_setting);
+                        false /* persist */, CONTENT_SETTING_BLOCK);
     return;
   }
 
+  // Consent required if user has never given consent for this origin, or if
+  // user has never given consent to attestation for content protection on this
+  // device.
+  bool consent_required =
+      content_setting != CONTENT_SETTING_ALLOW ||
+      !profile()->GetPrefs()->GetBoolean(prefs::kRAConsentGranted);
+
+  if (!consent_required) {

[ddorwin, 2015/03/12 03:58:12]

nit: All the negative logic is a bit confusing. Perhaps invert it to
full_consent_given or something.

[xhwang, 2015/03/12 17:35:26]

Done.

+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_ALLOW);
+    return;
+  }
+
   // Since the dialog is modal, we only support one prompt per |web_contents|.
   // Reject the new one if there is already one pending. See
   // http://crbug.com/447005
   if (pending_requests_.count(web_contents)) {
     callback.Run(CONTENT_SETTING_DEFAULT);
     return;
   }
 
+  // On ChromeOS, we don't use PermissionContextBase::RequestPermission() which
+  // uses the standard permission infobar/bubble UI. See http://crbug.com/454847
+  // Instead, we check the content setting and show the existing platform

[ddorwin, 2015/03/12 03:58:12]

... setting (above) and ... ?

[xhwang, 2015/03/12 17:35:27]

Done.

+  // verification UI.
+  // TODO(xhwang): Remove when http://crbug.com/454847 is fixed.
   views::Widget* widget = PlatformVerificationDialog::ShowDialog(
       web_contents, requesting_origin,
       base::Bind(&ProtectedMediaIdentifierPermissionContext::
-                     OnPlatformVerificationResult,
+                     OnPlatformVerificationConsentResponse,
                  weak_factory_.GetWeakPtr(), web_contents, id,
                  requesting_origin, embedding_origin, callback));
   pending_requests_.insert(
       std::make_pair(web_contents, std::make_pair(widget, id)));
 #else
   PermissionContextBase::RequestPermission(web_contents, id, requesting_origin,
                                            user_gesture, callback);
 #endif
 }
 
 ContentSetting ProtectedMediaIdentifierPermissionContext::GetPermissionStatus(
       const GURL& requesting_origin,
       const GURL& embedding_origin) const {
+  DVLOG(1) << __FUNCTION__ << ": (" << requesting_origin.spec() << ", "
+           << embedding_origin.spec() << ")";
+
   if (!IsProtectedMediaIdentifierEnabled())
     return CONTENT_SETTING_BLOCK;
 
+#if defined(OS_CHROMEOS)
+  // Block if user never granted RA consent on this device. It's possible that
+  // user dismissed the dialog triggered by RequestPermission() and the content
+  // setting is set to "allow" by server sync. In this case, we should block.
+  if (!profile()->GetPrefs()->GetBoolean(prefs::kRAConsentGranted)) {
+    DVLOG(1) << "RA consent never granted on this device.";
+    return CONTENT_SETTING_BLOCK;

[ddorwin, 2015/03/12 03:58:12]

Why do we block in this case rather than return NONE?

Also, we seem to handle this in two locations - here and line 88.

[xhwang, 2015/03/12 17:35:26]

Good point. Changed to ASK.
 
Yes, this is the tricky part which is explained in the comment above. For
RequestPermission, we'll show the UI when kRAConsentGranted is false. For
GetPermissionStatus, we'll return ASK. Since the behavior is different, it's
hard to put this check in one single place.

+  }
+#endif
+
   return PermissionContextBase::GetPermissionStatus(requesting_origin,
                                                     embedding_origin);
 }
 
 void ProtectedMediaIdentifierPermissionContext::CancelPermissionRequest(
     content::WebContents* web_contents,
     const PermissionRequestID& id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
 #if defined(OS_CHROMEOS)
   PendingRequestMap::iterator request = pending_requests_.find(web_contents);
   if (request == pending_requests_.end() || !request->second.second.Equals(id))
     return;
 
-  // Close the |widget_|. OnPlatformVerificationResult() will be fired
+  // Close the |widget_|. OnPlatformVerificationConsentResponse() will be fired
   // during this process, but since |web_contents| is removed from
   // |pending_requests_|, the callback will simply be dropped.
   views::Widget* widget = request->second.first;
   pending_requests_.erase(request);
   widget->Close();
 #else
   PermissionContextBase::CancelPermissionRequest(web_contents, id);
 #endif
 }
 
@@ skipping 10 matching lines @@
   if (content_settings) {
     content_settings->OnProtectedMediaIdentifierPermissionSet(
         requesting_frame.GetOrigin(), allowed);
   }
 }
 
 // TODO(xhwang): We should consolidate the "protected content" related pref
 // across platforms.
 bool ProtectedMediaIdentifierPermissionContext::
     IsProtectedMediaIdentifierEnabled() const {
-  bool enabled = false;
-
 #if defined(OS_ANDROID)
-  enabled = profile()->GetPrefs()->GetBoolean(
-      prefs::kProtectedMediaIdentifierEnabled);
+  if (!profile()->GetPrefs()->GetBoolean(
+          prefs::kProtectedMediaIdentifierEnabled)) {
+    DVLOG(1) << "Protected media identifier disabled by a user master switch.";
+    return false;
+  }
 #endif
 
 #if defined(OS_CHROMEOS)

[ddorwin, 2015/03/12 03:58:12]

Should this be #elif?

[xhwang, 2015/03/12 17:35:26]

Done.

-  // This could be disabled by the device policy.
+  // Platform verification is not allowed in incognito or guest mode.
+  if (profile()->IsOffTheRecord() || profile()->IsGuestSession()) {
+    DVLOG(1) << "Protected media identifier disabled in incognito or guest "
+                "mode.";
+    return false;
+  }
+
+  // This could be disabled by user's master switch or by the device policy.

[ddorwin, 2015/03/12 03:58:12]

It appears we check in the opposite order. If so, please update.

[xhwang, 2015/03/12 17:35:26]

Done.

   bool enabled_for_device = false;
-  enabled = chromeos::CrosSettings::Get()->GetBoolean(
-                chromeos::kAttestationForContentProtectionEnabled,
-                &enabled_for_device) &&
-            enabled_for_device &&
-            profile()->GetPrefs()->GetBoolean(prefs::kEnableDRM);
+  if (!chromeos::CrosSettings::Get()->GetBoolean(
+          chromeos::kAttestationForContentProtectionEnabled,
+          &enabled_for_device) ||
+      !enabled_for_device ||
+      !profile()->GetPrefs()->GetBoolean(prefs::kEnableDRM)) {
+    DVLOG(1) << "Protected media identifier disabled by the user or by device "
+                "policy.";
+    return false;
+  }
 #endif
 
-  DVLOG_IF(1, !enabled)
-      << "Protected media identifier disabled by the user or by device policy.";
-  return enabled;
+  return true;
 }
 
 #if defined(OS_CHROMEOS)
-void ProtectedMediaIdentifierPermissionContext::OnPlatformVerificationResult(
-    content::WebContents* web_contents,
-    const PermissionRequestID& id,
-    const GURL& requesting_origin,
-    const GURL& embedding_origin,
-    const BrowserPermissionCallback& callback,
-    chromeos::attestation::PlatformVerificationFlow::ConsentResponse response) {
+static void RecordRAConsentGranted(content::WebContents* web_contents) {
+  PrefService* pref_service =
+      user_prefs::UserPrefs::Get(web_contents->GetBrowserContext());
+  if (!pref_service) {
+    LOG(ERROR) << "Failed to get user prefs.";
+    return;
+  }
+  pref_service->SetBoolean(prefs::kRAConsentGranted, true);
+}
+
+void ProtectedMediaIdentifierPermissionContext::
+    OnPlatformVerificationConsentResponse(
+        content::WebContents* web_contents,
+        const PermissionRequestID& id,
+        const GURL& requesting_origin,
+        const GURL& embedding_origin,
+        const BrowserPermissionCallback& callback,
+        chromeos::attestation::PlatformVerificationDialog::ConsentResponse
+            response) {
   // The request may have been canceled. Drop the callback in that case.
   PendingRequestMap::iterator request = pending_requests_.find(web_contents);
   if (request == pending_requests_.end())
     return;
 
   DCHECK(request->second.second.Equals(id));
   pending_requests_.erase(request);
 
   ContentSetting content_setting = CONTENT_SETTING_DEFAULT;
   bool persist = false; // Whether the ContentSetting should be saved.
   switch (response) {
-    case PlatformVerificationFlow::CONSENT_RESPONSE_NONE:
+    case PlatformVerificationDialog::CONSENT_RESPONSE_NONE:
       content_setting = CONTENT_SETTING_DEFAULT;
       persist = false;
       break;
-    case PlatformVerificationFlow::CONSENT_RESPONSE_ALLOW:
+    case PlatformVerificationDialog::CONSENT_RESPONSE_ALLOW:
+      VLOG(1) << "Platform verification accepted by user.";
+      content::RecordAction(
+          base::UserMetricsAction("PlatformVerificationAccepted"));

[ddorwin, 2015/03/12 03:58:12]

Should we record None too? Would be interesting to know how many hit 'X'/"esc".

[xhwang, 2015/03/12 17:35:26]

This and PlatformVerificationRejected are actually in Counts, not in Histograms.
I also feel a Histogram (with a NONE entry) would be more useful. +Darren.

[Darren Krahn, 2015/03/13 22:47:11]

I don't care either way. IIUC, user actions are typically measured with 'action'
metrics -- we could just add a 'cancel' action. A histogram works too, though.

+      RecordRAConsentGranted(web_contents);
       content_setting = CONTENT_SETTING_ALLOW;
       persist = true;
       break;
-    case PlatformVerificationFlow::CONSENT_RESPONSE_DENY:
+    case PlatformVerificationDialog::CONSENT_RESPONSE_DENY:
+      VLOG(1) << "Platform verification denied by user.";
+      content::RecordAction(
+          base::UserMetricsAction("PlatformVerificationRejected"));
       content_setting = CONTENT_SETTING_BLOCK;
       persist = true;
       break;
   }
 
   NotifyPermissionSet(
       id, requesting_origin, embedding_origin, callback,
       persist, content_setting);
 }
 #endif