Start wiring up modal dialog experiment in PermissionRequestManager codepath

chrome/browser/permissions/permission_dialog_delegate.cc

Index: chrome/browser/permissions/permission_dialog_delegate.cc
diff --git a/chrome/browser/permissions/permission_dialog_delegate.cc b/chrome/browser/permissions/permission_dialog_delegate.cc
index f7c006e165790571305814fcc33bcc4f51b21b02..c9619a080ca517dc05748f8eabcbe829eb64fc7e 100644
--- a/chrome/browser/permissions/permission_dialog_delegate.cc
+++ b/chrome/browser/permissions/permission_dialog_delegate.cc
@@ -21,11 +21,13 @@
 #include "chrome/browser/notifications/notification_permission_infobar_delegate.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/web_contents.h"
 #include "jni/PermissionDialogController_jni.h"
 #include "jni/PermissionDialogDelegate_jni.h"
 #include "ui/android/window_android.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/window_open_disposition.h"
 
 using base::android::ConvertUTF16ToJavaString;
@@ -40,11 +42,27 @@ const char kModalParamsUserGestureKey[] = "require_gesture";
 // static
 void PermissionDialogDelegate::Create(
     content::WebContents* web_contents,
-    ContentSettingsType type,
-    const GURL& requesting_frame,
-    bool user_gesture,
-    Profile* profile,
-    const PermissionSetCallback& callback) {
+    PermissionPromptAndroid* permission_prompt) {

[raymes, 2017/04/11 02:50:35]

Is it worth DCHECKing that the PermissionRequestManager is enabled in this case?

[Timothy Loh, 2017/05/24 03:30:22]

I'd prefer to not add the dependency.

+  DCHECK(web_contents);
+
+  // If we don't have a tab, just act as though the prompt was dismissed.
+  TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
+  if (!tab) {
+    permission_prompt->Closing();
+    return;
+  }
+
+  // Dispatch the dialog to Java, which manages the lifetime of this object.
+  new PermissionDialogDelegate(tab, permission_prompt);
+}
+
+// static
+void PermissionDialogDelegate::Create(content::WebContents* web_contents,
+                                      ContentSettingsType type,
+                                      const GURL& requesting_frame,
+                                      bool user_gesture,
+                                      Profile* profile,
+                                      const PermissionSetCallback& callback) {
   DCHECK(web_contents);
 
   // If we don't have a tab, just act as though the prompt was dismissed.
@@ -106,43 +124,80 @@ bool PermissionDialogDelegate::RegisterPermissionDialogDelegate(JNIEnv* env) {
 
 ScopedJavaLocalRef<jobject> PermissionDialogDelegate::CreateJavaDelegate(
     JNIEnv* env) {
+  ScopedJavaLocalRef<jstring> primaryButtonText = ConvertUTF16ToJavaString(
+      env, l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW));
+  ScopedJavaLocalRef<jstring> secondaryButtonText = ConvertUTF16ToJavaString(
+      env, l10n_util::GetStringUTF16(IDS_PERMISSION_DENY));
+
+  if (infobar_delegate_) {
+    std::vector<int> content_settings_types{
+        infobar_delegate_->content_settings_types()};
+
+    return Java_PermissionDialogDelegate_create(
+        env, reinterpret_cast<uintptr_t>(this), tab_->GetJavaObject(),
+        base::android::ToJavaIntArray(env, content_settings_types).obj(),
+        ResourceMapper::MapFromChromiumId(infobar_delegate_->GetIconId()),
+        ConvertUTF16ToJavaString(env, infobar_delegate_->GetMessageText()),
+        ConvertUTF16ToJavaString(env, infobar_delegate_->GetLinkText()),
+        primaryButtonText, secondaryButtonText,

[raymes, 2017/04/11 02:50:35]

Does this change the button labels for the old codepath?

[Timothy Loh, 2017/05/24 03:30:22]

I just inlined the logic of GetButtonLabel so the values could be re-used below.

+        infobar_delegate_->ShouldShowPersistenceToggle());
+  }
+
+  // TODO(timloh): Handle grouped media permissions (camera + microphone).
+  DCHECK_EQ(1u, permission_prompt_->permission_count());
+
   std::vector<int> content_settings_types{
-      infobar_delegate_->content_settings_types()};
+      permission_prompt_->GetContentSettingType(0)};
 
   return Java_PermissionDialogDelegate_create(
-      env, reinterpret_cast<uintptr_t>(this),
-      tab_->GetJavaObject(),
+      env, reinterpret_cast<uintptr_t>(this), tab_->GetJavaObject(),
       base::android::ToJavaIntArray(env, content_settings_types).obj(),
-      ResourceMapper::MapFromChromiumId(infobar_delegate_->GetIconId()),
-      ConvertUTF16ToJavaString(env, infobar_delegate_->GetMessageText()),
-      ConvertUTF16ToJavaString(env, infobar_delegate_->GetLinkText()),
-      ConvertUTF16ToJavaString(env, infobar_delegate_->GetButtonLabel(
-                                        PermissionInfoBarDelegate::BUTTON_OK)),
+      ResourceMapper::MapFromChromiumId(
+          permission_prompt_->GetIconIdForPermission(0)),
+      // TODO(timloh): This is the wrong string.
       ConvertUTF16ToJavaString(env,
-                               infobar_delegate_->GetButtonLabel(
-                                   PermissionInfoBarDelegate::BUTTON_CANCEL)),
-      infobar_delegate_->ShouldShowPersistenceToggle());
+                               permission_prompt_->GetMessageTextFragment(0)),
+      // TODO(timloh): Pass the actual link text for EME.
+      ConvertUTF16ToJavaString(env, base::string16()), primaryButtonText,
+      secondaryButtonText,
+      // TODO(timloh): Hook up the persistence toggle.
+      false);
 }
 
 void PermissionDialogDelegate::Accept(JNIEnv* env,
                                       const JavaParamRef<jobject>& obj,
                                       jboolean persist) {
-  if (infobar_delegate_->ShouldShowPersistenceToggle())
-    infobar_delegate_->set_persist(persist);
-  infobar_delegate_->Accept();
+  if (infobar_delegate_) {
+    if (infobar_delegate_->ShouldShowPersistenceToggle())
+      infobar_delegate_->set_persist(persist);
+    infobar_delegate_->Accept();
+    return;
+  }
+
+  permission_prompt_->Accept();

[raymes, 2017/04/11 02:50:35]

nit: Should we add TODOs here and below to hook up the persistence toggle?

[Timothy Loh, 2017/05/24 03:30:22]

Don't think it's needed.

 }
 
 void PermissionDialogDelegate::Cancel(JNIEnv* env,
                                       const JavaParamRef<jobject>& obj,
                                       jboolean persist) {
-  if (infobar_delegate_->ShouldShowPersistenceToggle())
-    infobar_delegate_->set_persist(persist);
-  infobar_delegate_->Cancel();
+  if (infobar_delegate_) {
+    if (infobar_delegate_->ShouldShowPersistenceToggle())
+      infobar_delegate_->set_persist(persist);
+    infobar_delegate_->Cancel();
+    return;
+  }
+
+  permission_prompt_->Deny();
 }
 
 void PermissionDialogDelegate::Dismissed(JNIEnv* env,
                                          const JavaParamRef<jobject>& obj) {
-  infobar_delegate_->InfoBarDismissed();
+  if (infobar_delegate_) {
+    infobar_delegate_->InfoBarDismissed();
+    return;
+  }
+
+  permission_prompt_->Closing();
 }
 
 void PermissionDialogDelegate::LinkClicked(JNIEnv* env,
@@ -151,10 +206,14 @@ void PermissionDialogDelegate::LinkClicked(JNIEnv* env,
   // InfoBarService as an owner() to open the link. That will fail since the
   // wrapped delegate has no owner (it hasn't been added as an infobar).
   if (tab_->web_contents()) {
-    tab_->web_contents()->OpenURL(content::OpenURLParams(
-        infobar_delegate_->GetLinkURL(), content::Referrer(),
-        WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
-        false));
+    if (infobar_delegate_) {
+      tab_->web_contents()->OpenURL(content::OpenURLParams(
+          infobar_delegate_->GetLinkURL(), content::Referrer(),
+          WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
+          false));
+    }
+    // TODO(timloh): Show a 'learn more' link in the PermissionRequestManager
+    // codepath for EME.
   }
 }
 
@@ -165,11 +224,29 @@ void PermissionDialogDelegate::Destroy(JNIEnv* env,
 
 PermissionDialogDelegate::PermissionDialogDelegate(
     TabAndroid* tab,
+    PermissionPromptAndroid* permission_prompt)
+    : tab_(tab),
+      infobar_delegate_(nullptr),
+      permission_prompt_(permission_prompt) {
+  DCHECK(tab_);
+  DCHECK(permission_prompt_);
+  Init();
+}
+
+PermissionDialogDelegate::PermissionDialogDelegate(
+    TabAndroid* tab,
     std::unique_ptr<PermissionInfoBarDelegate> infobar_delegate)
-    : tab_(tab), infobar_delegate_(std::move(infobar_delegate)) {
+    : tab_(tab),
+      infobar_delegate_(std::move(infobar_delegate)),
+      permission_prompt_(nullptr) {
   DCHECK(tab_);
   DCHECK(infobar_delegate_);
+  Init();
+}

[raymes, 2017/04/11 02:50:35]

nit: should we just merge these 2 constructors together and just pass null for
the unneeded param?

[Timothy Loh, 2017/05/24 03:30:22]

Sure, done.

 
+PermissionDialogDelegate::~PermissionDialogDelegate() {}
+
+void PermissionDialogDelegate::Init() {
   // Create our Java counterpart, which manages our lifetime.
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jobject> j_delegate =
@@ -180,5 +257,3 @@ PermissionDialogDelegate::PermissionDialogDelegate(
   // longer needed it will in turn free the native delegate.
   Java_PermissionDialogController_createDialog(env, j_delegate.obj());
 }
-
-PermissionDialogDelegate::~PermissionDialogDelegate() {}