media: Invoke PlatformVerificationDialog from ProtectedMediaIdentifierPermissionContext.

This is a follow-up CL of r314351.

Currently this is invoked from PermissionContextBase. Since this is for
protected media identifier only, it's better to put it in
ProtectedMediaIdentifierPermissionContext.

Also included in this fix:
- Pass |requesting_origin| to PlatformVerificationDialog::ShowDialog().
  Currently we are using the origin of the GetLastCommittedURL(), which is
  wrong.
- Add ProtectedMediaIdentifierPermissionContext::CancelPermissionRequest() so
  that we can cancel the prompt and drop the callback properly. Otherwise, this
  will cause a DCHECK in PermissionServiceImp when closing the browser while the
  prompt is shown.

BUG=446263
TEST=Manually tested various cases.

Review URL: https://codereview.chromium.org/881983003

Cr-Commit-Position: refs/heads/master@{#314924}

chrome/browser/chromeos/attestation/platform_verification_dialog.cc

@@ -37,16 +37,20 @@ const int kDialogMaxWidthInPixel = 400;
 }  // namespace
 
 // static
-void PlatformVerificationDialog::ShowDialog(
+views::Widget* PlatformVerificationDialog::ShowDialog(
     content::WebContents* web_contents,
+    const GURL& requesting_origin,
     const PlatformVerificationFlow::Delegate::ConsentCallback& callback) {
-  GURL url = web_contents->GetLastCommittedURL();
   // In the case of an extension or hosted app, the origin of the request is
   // best described by the extension / app name.
   const extensions::Extension* extension =
-      extensions::ExtensionRegistry::Get(web_contents->GetBrowserContext())->
-          enabled_extensions().GetExtensionOrAppByURL(url);
-  std::string origin = extension ? extension->name() : url.GetOrigin().spec();
+      extensions::ExtensionRegistry::Get(web_contents->GetBrowserContext())
+          ->enabled_extensions()
+          .GetExtensionOrAppByURL(web_contents->GetLastCommittedURL());
+
+  // TODO(xhwang): We should only show the name if the request if from the
+  // extension's true frame. See http://crbug.com/455821
+  std::string origin = extension ? extension->name() : requesting_origin.spec();
 
   PlatformVerificationDialog* dialog = new PlatformVerificationDialog(
       web_contents,
@@ -60,6 +64,8 @@ void PlatformVerificationDialog::ShowDialog(
   views::Widget* widget = views::DialogDelegate::CreateDialogWidget(
       dialog, NULL, popup_manager->GetHostView());
   popup_manager->ShowModalDialog(widget->GetNativeView(), web_contents);
+
+  return widget;
 }
 
 PlatformVerificationDialog::~PlatformVerificationDialog() {

chrome/browser/chromeos/attestation/platform_verification_dialog.h

@@ -25,9 +25,13 @@ class PlatformVerificationDialog : public views::DialogDelegateView,
                                    public views::StyledLabelListener,
                                    public content::WebContentsObserver {
  public:
-  // Initializes a tab-modal dialog for |web_contents| and shows it.
-  static void ShowDialog(
+  // Initializes a tab-modal dialog for |web_contents| and |requesting_origin|
+  // and shows it. Returns a non-owning pointer to the widget so that caller can
+  // close the dialog and cancel the request. The returned widget is only
+  // guaranteed to be valid before |callback| is called.
+  static views::Widget* ShowDialog(
       content::WebContents* web_contents,
+      const GURL& requesting_origin,
       const PlatformVerificationFlow::Delegate::ConsentCallback& callback);
 
  protected:

chrome/browser/chromeos/attestation/platform_verification_flow.cc

@@ -79,9 +79,11 @@ class DefaultDelegate : public PlatformVerificationFlow::Delegate {
 
   void ShowConsentPrompt(
       content::WebContents* web_contents,
+      const GURL& requesting_origin,
       const PlatformVerificationFlow::Delegate::ConsentCallback& callback)
       override {
-    PlatformVerificationDialog::ShowDialog(web_contents, callback);
+    PlatformVerificationDialog::ShowDialog(web_contents, requesting_origin,
+                                           callback);
   }
 
   PrefService* GetPrefs(content::WebContents* web_contents) override {
@@ -235,10 +237,16 @@ void PlatformVerificationFlow::CheckConsent(const ChallengeContext& context,
       this,
       context,
       consent_required);
-  if (consent_required)
-    delegate_->ShowConsentPrompt(context.web_contents, consent_callback);
-  else
+  if (consent_required) {
+    // TODO(xhwang): Using delegate_->GetURL() here is not right. The consent
+    // may be requested by a frame from a different origin. This will be solved
+    // when http://crbug.com/454847 is fixed.
+    delegate_->ShowConsentPrompt(
+        context.web_contents,
+        delegate_->GetURL(context.web_contents).GetOrigin(), consent_callback);
+  } else {
     consent_callback.Run(CONSENT_RESPONSE_NONE);
+  }
 }
 
 void PlatformVerificationFlow::RegisterProfilePrefs(

chrome/browser/chromeos/attestation/platform_verification_flow.h

@@ -92,9 +92,10 @@ class PlatformVerificationFlow
 
     // Invokes consent UI within the context of |web_contents| and calls
     // |callback| when the user responds.
-    // Precondition: The last committed URL for |web_contents| has a valid
-    //               origin.
+    // |requesting_origin| or the extension/app name will be shown on the prompt
+    // if the request comes from a web page or an extension/app, respectively.
     virtual void ShowConsentPrompt(content::WebContents* web_contents,
+                                   const GURL& requesting_origin,
                                    const ConsentCallback& callback) = 0;
 
     // Gets prefs associated with the given |web_contents|.  If no prefs are

chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc

@@ -75,6 +75,7 @@ class FakeDelegate : public PlatformVerificationFlow::Delegate {
 
   void ShowConsentPrompt(
       content::WebContents* web_contents,
+      const GURL& requesting_origin,
       const PlatformVerificationFlow::Delegate::ConsentCallback& callback)
       override {
     num_consent_calls_++;

chrome/browser/content_settings/permission_context_base.cc

@@ -18,12 +18,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/attestation/platform_verification_dialog.h"
-using chromeos::attestation::PlatformVerificationDialog;
-using chromeos::attestation::PlatformVerificationFlow;
-#endif
-
 PermissionContextBase::PermissionContextBase(
     Profile* profile,
     const ContentSettingsType permission_type)
@@ -132,19 +126,6 @@ void PermissionContextBase::DecidePermission(
   PermissionContextUmaUtil::PermissionRequested(
       permission_type_, requesting_origin);
 
-#if defined(OS_CHROMEOS)
-  // TODO(xhwang): This is to use the existing platform verification UI. Remove
-  // it when the infobar/bubble UI can satisfy our requirements.
-  if (permission_type_ == CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) {
-    PlatformVerificationDialog::ShowDialog(
-        web_contents,
-        base::Bind(&PermissionContextBase::OnPlatformVerificationResult,
-                   weak_factory_.GetWeakPtr(), id, requesting_origin,
-                   embedding_origin, callback));
-    return;
-  }
-#endif
-
   if (PermissionBubbleManager::Enabled()) {
     if (pending_bubbles_.get(id.ToString()) != NULL)
       return;
@@ -248,25 +229,3 @@ void PermissionContextBase::UpdateContentSetting(const GURL& requesting_origin,
       ContentSettingsPattern::FromURLNoWildcard(embedding_origin),
       permission_type_, std::string(), content_setting);
 }
-
-#if defined(OS_CHROMEOS)
-void PermissionContextBase::OnPlatformVerificationResult(
-    const PermissionRequestID& id,
-    const GURL& requesting_origin,
-    const GURL& embedding_origin,
-    const BrowserPermissionCallback& callback,
-    chromeos::attestation::PlatformVerificationFlow::ConsentResponse response) {
-  if (response == PlatformVerificationFlow::CONSENT_RESPONSE_NONE) {
-    // Deny request and do not save to content settings.
-    PermissionDecided(id, requesting_origin, embedding_origin, callback,
-                      false,   // Do not save to content settings.
-                      false);  // Do not allow the permission.
-    return;
-  }
-
-  PermissionDecided(
-      id, requesting_origin, embedding_origin, callback,
-      true,  // Save to content settings.
-      response == PlatformVerificationFlow::CONSENT_RESPONSE_ALLOW);
-}
-#endif

chrome/browser/content_settings/permission_context_base.h

@@ -15,10 +15,6 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "url/gurl.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/attestation/platform_verification_flow.h"
-#endif
-
 class PermissionQueueController;
 class PermissionRequestID;
 class Profile;
@@ -131,16 +127,6 @@ class PermissionContextBase : public KeyedService {
   // Called when a bubble is no longer used so it can be cleaned up.
   void CleanUpBubble(const PermissionRequestID& id);
 
-#if defined(OS_CHROMEOS)
-  void OnPlatformVerificationResult(
-      const PermissionRequestID& id,
-      const GURL& requesting_origin,
-      const GURL& embedding_origin,
-      const BrowserPermissionCallback& callback,
-      chromeos::attestation::PlatformVerificationFlow::ConsentResponse
-          response);
-#endif
-
   Profile* profile_;
   const ContentSettingsType permission_type_;
   scoped_ptr<PermissionQueueController> permission_queue_controller_;

chrome/browser/media/protected_media_identifier_permission_context.cc

@@ -13,14 +13,34 @@
 #include "content/public/browser/web_contents.h"
 
 #if defined(OS_CHROMEOS)
+#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 "ui/views/widget/widget.h"
+
+using chromeos::attestation::PlatformVerificationDialog;
+using chromeos::attestation::PlatformVerificationFlow;
+#endif
+
+#if defined(OS_CHROMEOS)
+namespace {
+PermissionRequestID GetInvalidPendingId() {
+  return PermissionRequestID(-1, -1, -1, GURL());
+}
+}
 #endif
 
 ProtectedMediaIdentifierPermissionContext::
     ProtectedMediaIdentifierPermissionContext(Profile* profile)
     : PermissionContextBase(profile,
-                            CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) {
+                            CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER)
+#if defined(OS_CHROMEOS)
+      ,
+      pending_id_(GetInvalidPendingId()),
+      widget_(nullptr),
+      weak_factory_(this)
+#endif
+{
 }
 
 ProtectedMediaIdentifierPermissionContext::
@@ -30,23 +50,61 @@ ProtectedMediaIdentifierPermissionContext::
 void ProtectedMediaIdentifierPermissionContext::RequestPermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
-    const GURL& requesting_frame_origin,
+    const GURL& requesting_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (!IsProtectedMediaIdentifierEnabled()) {
-    NotifyPermissionSet(id,
-                        requesting_frame_origin,
-                        web_contents->GetLastCommittedURL().GetOrigin(),
-                        callback, false, false);
+  GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
+
+  if (!requesting_origin.is_valid() || !embedding_origin.is_valid() ||
+      !IsProtectedMediaIdentifierEnabled()) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, false /* granted */);
     return;
   }
 
-  PermissionContextBase::RequestPermission(web_contents, id,
-                                           requesting_frame_origin,
-                                           user_gesture,
-                                           callback);
+#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);
+
+  switch (content_setting) {
+    case CONTENT_SETTING_BLOCK:
+      NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                          false /* persist */, false /* granted */);
+      return;
+    case CONTENT_SETTING_ALLOW:
+      NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                          false /* persist */, true /* granted */);
+      return;
+    default:
+      break;
+  }
+
+  // We only support one prompt and one pending permission request.
+  // Reject the new one if there is already one pending. See
+  // http://crbug.com/447005
+  if (!pending_id_.Equals(GetInvalidPendingId())) {
+    callback.Run(false);
+    return;
+  }
+
+  pending_id_ = id;
+  widget_ = PlatformVerificationDialog::ShowDialog(
+      web_contents, requesting_origin,
+      base::Bind(&ProtectedMediaIdentifierPermissionContext::
+                     OnPlatformVerificationResult,
+                 weak_factory_.GetWeakPtr(), id, requesting_origin,
+                 embedding_origin, callback));
+#else
+  PermissionContextBase::RequestPermission(web_contents, id, requesting_origin,
+                                           user_gesture, callback);
+#endif
 }
 
 ContentSetting ProtectedMediaIdentifierPermissionContext::GetPermissionStatus(
@@ -59,6 +117,26 @@ ContentSetting ProtectedMediaIdentifierPermissionContext::GetPermissionStatus(
                                                     embedding_origin);
 }
 
+void ProtectedMediaIdentifierPermissionContext::CancelPermissionRequest(
+    content::WebContents* web_contents,
+    const PermissionRequestID& id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+#if defined(OS_CHROMEOS)
+  if (!widget_ || !pending_id_.Equals(id))
+    return;
+
+  // Close the |widget_|. OnPlatformVerificationResult() will be fired
+  // during this process, but since |pending_id_| is cleared, the callback will
+  // be dropped.
+  pending_id_ = GetInvalidPendingId();
+  widget_->Close();
+  return;
+#else
+  PermissionContextBase::CancelPermissionRequest(web_contents, id);
+#endif
+}
+
 void ProtectedMediaIdentifierPermissionContext::UpdateTabContext(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
@@ -73,7 +151,6 @@ void ProtectedMediaIdentifierPermissionContext::UpdateTabContext(
     content_settings->OnProtectedMediaIdentifierPermissionSet(
         requesting_frame.GetOrigin(), allowed);
   }
-
 }
 
 // TODO(xhwang): We should consolidate the "protected content" related pref
@@ -101,3 +178,34 @@ bool ProtectedMediaIdentifierPermissionContext::
       << "Protected media identifier disabled by the user or by device policy.";
   return enabled;
 }
+
+#if defined(OS_CHROMEOS)
+void ProtectedMediaIdentifierPermissionContext::OnPlatformVerificationResult(
+    const PermissionRequestID& id,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    const BrowserPermissionCallback& callback,
+    chromeos::attestation::PlatformVerificationFlow::ConsentResponse response) {
+  DCHECK(widget_);
+  widget_ = nullptr;
+
+  // The request may have been canceled. Drop the callback here.
+  if (!pending_id_.Equals(id))
+    return;
+
+  pending_id_ = GetInvalidPendingId();
+
+  if (response == PlatformVerificationFlow::CONSENT_RESPONSE_NONE) {
+    // Deny request and do not save to content settings.
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false,   // Do not save to content settings.
+                        false);  // Do not allow the permission.
+    return;
+  }
+
+  NotifyPermissionSet(
+      id, requesting_origin, embedding_origin, callback,
+      true,  // Save to content settings.
+      response == PlatformVerificationFlow::CONSENT_RESPONSE_ALLOW);
+}
+#endif

chrome/browser/media/protected_media_identifier_permission_context.h

@@ -6,10 +6,20 @@
 #define CHROME_BROWSER_MEDIA_PROTECTED_MEDIA_IDENTIFIER_PERMISSION_CONTEXT_H_
 
 #include "chrome/browser/content_settings/permission_context_base.h"
+#include "components/content_settings/core/common/permission_request_id.h"
+
+#if defined(OS_CHROMEOS)
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/attestation/platform_verification_dialog.h"
+#include "chrome/browser/chromeos/attestation/platform_verification_flow.h"
+#endif
 
-class PermissionRequestID;
 class Profile;
 
+namespace views {
+class Widget;
+}
+
 namespace content {
 class RenderViewHost;
 class WebContents;
@@ -26,12 +36,14 @@ class ProtectedMediaIdentifierPermissionContext
   // valid iframes. It also adds special logic when called through an extension.
   void RequestPermission(content::WebContents* web_contents,
                          const PermissionRequestID& id,
-                         const GURL& requesting_frame_origin,
+                         const GURL& requesting_origin,
                          bool user_gesture,
                          const BrowserPermissionCallback& callback) override;
   ContentSetting GetPermissionStatus(
       const GURL& requesting_origin,
       const GURL& embedding_origin) const override;
+  void CancelPermissionRequest(content::WebContents* web_contents,
+                               const PermissionRequestID& id) override;
 
  private:
   ~ProtectedMediaIdentifierPermissionContext() override;
@@ -44,6 +56,26 @@ class ProtectedMediaIdentifierPermissionContext
   // user in the master switch in content settings, or by the device policy.
   bool IsProtectedMediaIdentifierEnabled() const;
 
+#if defined(OS_CHROMEOS)
+  void OnPlatformVerificationResult(
+      const PermissionRequestID& id,
+      const GURL& requesting_origin,
+      const GURL& embedding_origin,
+      const BrowserPermissionCallback& callback,
+      chromeos::attestation::PlatformVerificationFlow::ConsentResponse
+          response);
+
+  // ID for the pending permission request. Invalid when no request is pending,
+  // or the request has been canceled.
+  PermissionRequestID pending_id_;
+
+  views::Widget* widget_;
+
+  // Must be the last member, to ensure that it will be
+  // destroyed first, which will invalidate weak pointers
+  base::WeakPtrFactory<ProtectedMediaIdentifierPermissionContext> weak_factory_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(ProtectedMediaIdentifierPermissionContext);
 };