Hide modal permission prompts on Android upon tab navigation/destruction

chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java

Index: chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java b/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
index d38dc9b3f037a8e1b40ff6c7ab74d08d726a0701..c3c3234505d1e677b170b0975d68e6f63f8a148e 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
@@ -89,6 +89,7 @@ public class PermissionDialogController implements AndroidPermissionRequester.Re
      */
     private void queueDialog(PermissionDialogDelegate delegate) {
         mRequestQueue.add(delegate);
+        delegate.setDialogController(this);
         scheduleDisplay();
     }
 
@@ -101,6 +102,11 @@ public class PermissionDialogController implements AndroidPermissionRequester.Re
         return mDialog;
     }
 
+    @VisibleForTesting
+    public int getQueueLengthForTesting() {
+        return mRequestQueue.size();
+    }
+
     @Override
     public void onAndroidPermissionAccepted() {
         mDialogDelegate.onAccept(mSwitchView.isChecked());
@@ -193,9 +199,10 @@ public class PermissionDialogController implements AndroidPermissionRequester.Re
         mDialog.setOnDismissListener(new DialogInterface.OnDismissListener() {
             @Override
             public void onDismiss(DialogInterface dialog) {
-                // For some reason this is ocassionally null. See crbug.com/708562.
+                // Null if dismiss initiated by C++, or for some unknown reason (crbug.com/708562).

[Timothy Loh, 2017/05/24 04:59:34]

Updated this comment too.

[Bernhard Bauer, 2017/05/25 09:57:17]

So, this is sometimes null and we don't know why? That doesn't fill me with
confidence :-/

[Timothy Loh, 2017/05/26 05:08:03]

Well that's what the existing comment said; I couldn't see how it would
otherwise end up null but I thought it'd be better to preserve the comment.

                 if (mDialogDelegate == null) {
                     scheduleDisplay();
+                    return;
                 }
 
                 mDialog = null;
@@ -253,6 +260,18 @@ public class PermissionDialogController implements AndroidPermissionRequester.Re
         return fullString;
     }
 
+    public void dismissFromNative(PermissionDialogDelegate delegate) {
+        if (mDialogDelegate == delegate) {

[dominickn, 2017/05/24 05:02:53]

What I meant here was using PermissionDialogDelegate#mNativeDelegatePtr as the
equality condition (that's a long that's the value of the pointer to the
native-side C++ object)

[Timothy Loh, 2017/05/25 03:11:22]

I think referential equality is OK here; implementing .equals() correctly means
we'd have to check all of the other members as well and also implement
.hashCode(), and if we don't use .equals() the else branch below won't match.
dismissFromNative() on the PDD also nulls out the native pointer which I don't
think we should assume that happens after it calls dismissFromNative on the PDC.

[dominickn, 2017/05/25 04:49:30]

Acknowledged.

+            mDialogDelegate = null;
+            AlertDialog dialog = mDialog;
+            mDialog = null;
+            dialog.dismiss();
+        } else {
+            assert mRequestQueue.contains(delegate);
+            mRequestQueue.remove(delegate);
+        }
+    }
+
     private void destroyDelegate() {
         mDialogDelegate.destroy();
         mDialogDelegate = null;