[Image Descriptions] Add more dialog dismissal causes on Android

This is a follow-up CL to add clearer dialog dismissal causes for the
image descriptions dialog on Android.

With this CL we now differentiate between dialogs that are dismissed by
a user action on the dialog, vs dialogs that are dismissed due to
changes in state, such as web contents being hidden or destroyed.

This CL adds a new dialog dismissal cause for web contents destroyed.

Original CL: crrev.com/c/2596417

AX-Relnotes: N/A
Bug: 1057169
Change-Id: Id20d161d003dc9161fa9b3af792f6cc20b8aafe9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630728
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Reviewed-by: Mark Schillaci <mschillaci@google.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846371}

chrome/browser/image_descriptions/android/java/src/org/chromium/chrome/browser/image_descriptions/ImageDescriptionsDialog.java

@@ -47,6 +47,7 @@ public class ImageDescriptionsDialog
     private boolean mShouldShowDontAskAgainOption;
     private boolean mOnlyOnWifiState;
     private boolean mDontAskAgainState;
+    private @DialogDismissalCause int mDismissalCause;
     private WebContents mWebContents;
     private Profile mProfile;
     private Context mContext;
@@ -64,6 +65,7 @@ public class ImageDescriptionsDialog
         mShouldShowDontAskAgainOption = shouldShowDontAskAgainOption;
         mOnlyOnWifiState = true;
         mDontAskAgainState = false;
+        mDismissalCause = DialogDismissalCause.UNKNOWN;
 
         // Inflate our custom view layout for this dialog.
         LayoutInflater inflater = LayoutInflater.from(mContext);
@@ -97,24 +99,33 @@ public class ImageDescriptionsDialog
         mWebContentsObserver = new WebContentsObserver(mWebContents) {
             @Override
             public void wasHidden() {
-                unregisterObserver();
+                mDismissalCause = DialogDismissalCause.TAB_SWITCHED;
+                unregisterObserverAndDismiss();
             }
 
             @Override
             public void navigationEntryCommitted() {
-                unregisterObserver();
+                mDismissalCause = DialogDismissalCause.NAVIGATE;
+                unregisterObserverAndDismiss();
             }
 
             @Override
             public void onTopLevelNativeWindowChanged(@Nullable WindowAndroid windowAndroid) {
                 // Dismiss the dialog when the associated WebContents is detached from the window.
-                if (windowAndroid == null) unregisterObserver();
+                if (windowAndroid == null) {
+                    mDismissalCause = DialogDismissalCause.NOT_ATTACHED_TO_WINDOW;
+                    unregisterObserverAndDismiss();
+                }
             }
 
             @Override
             public void destroy() {
                 super.destroy();
-                dismissEarly();
+                // If no dismissal cause has been set, web contents were destroyed.
+                if (mDismissalCause == DialogDismissalCause.UNKNOWN) {
+                    mDismissalCause = DialogDismissalCause.WEB_CONTENTS_DESTROYED;
+                }
+                dismiss();
             }
         };
 
@@ -152,7 +163,6 @@ public class ImageDescriptionsDialog
 
     @Override
     public void onClick(PropertyModel model, int buttonType) {
-        int dismissalCause;
         int toastMessage = -1;
 
         // User has elected to get image descriptions
@@ -174,16 +184,16 @@ public class ImageDescriptionsDialog
                 toastMessage = R.string.image_descriptions_toast_just_once;
             }
 
-            dismissalCause = DialogDismissalCause.POSITIVE_BUTTON_CLICKED;
+            mDismissalCause = DialogDismissalCause.POSITIVE_BUTTON_CLICKED;
         } else {
-            dismissalCause = DialogDismissalCause.NEGATIVE_BUTTON_CLICKED;
+            mDismissalCause = DialogDismissalCause.NEGATIVE_BUTTON_CLICKED;
         }
 
         // Make a toast, if necessary.
         if (toastMessage != -1) Toast.makeText(mContext, toastMessage, Toast.LENGTH_LONG).show();
 
-        // Dismiss the dialog.
-        dismiss(dismissalCause);
+        // Dismiss the dialog and unregister observer.
+        unregisterObserverAndDismiss();
     }
 
     @Override
@@ -201,10 +211,10 @@ public class ImageDescriptionsDialog
     }
 
     /**
-     * Helper method to unregister |mWebContentsObserver| during changes in state to |mWebContents|.
-     * The call to #destroy() will also dismiss the dialog.
+     * Helper method to unregister |mWebContentsObserver| during changes in state to |mWebContents|
+     * or on user action. The call to #destroy() will also dismiss the dialog.
      */
-    private void unregisterObserver() {
+    private void unregisterObserverAndDismiss() {
         mWebContentsObserver.destroy();
     }
 
@@ -216,18 +226,9 @@ public class ImageDescriptionsDialog
     }
 
     /**
-     * Helper method to dismiss this dialog from changes to state of the |mWebContents|.
-     * Consider proactively dismissing the dialog to be a negative button click.
-     */
-    private void dismissEarly() {
-        dismiss(DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
-    }
-
-    /**
-     * Helper method to dismiss this dialog.
-     * @param dialogDismissableCause        DialogDismissalCause, e.g. positive or negative
+     * Helper method to dismiss this dialog. Dismisses the dialog with cause |mDismissalCause|.
      */
-    private void dismiss(int dialogDismissableCause) {
-        mModalDialogManager.dismissDialog(mPropertyModel, dialogDismissableCause);
+    private void dismiss() {
+        mModalDialogManager.dismissDialog(mPropertyModel, mDismissalCause);
     }
 }

ui/android/java/src/org/chromium/ui/modaldialog/DialogDismissalCause.java

@@ -14,12 +14,12 @@ import java.lang.annotation.RetentionPolicy;
         DialogDismissalCause.DISMISSED_BY_NATIVE,
         DialogDismissalCause.NAVIGATE_BACK_OR_TOUCH_OUTSIDE, DialogDismissalCause.TAB_SWITCHED,
         DialogDismissalCause.TAB_DESTROYED, DialogDismissalCause.ACTIVITY_DESTROYED,
-        DialogDismissalCause.NOT_ATTACHED_TO_WINDOW, DialogDismissalCause.NAVIGATE})
+        DialogDismissalCause.NOT_ATTACHED_TO_WINDOW, DialogDismissalCause.NAVIGATE,
+        DialogDismissalCause.WEB_CONTENTS_DESTROYED})
 @Retention(RetentionPolicy.SOURCE)
 public @interface DialogDismissalCause {
-    // Please do not remove or change the order of the existing values, and add new value at the end
-    // of the enum. Dismissal causes that are fully controlled by clients (i.e. are not used inside
-    // the dialog manager or the dialog presenters) are marked "Controlled by client" on comments.
+    // Dismissal causes that are fully controlled by clients (i.e. are not used inside the
+    // dialog manager or the dialog presenters) are marked "Controlled by client" on comments.
 
     /** No specified reason for the dialog dismissal. */
     int UNKNOWN = 0;
@@ -43,4 +43,6 @@ public @interface DialogDismissalCause {
     int NOT_ATTACHED_TO_WINDOW = 9;
     /** User has navigated, e.g. by typing a URL in the location bar. */
     int NAVIGATE = 10;
+    /** Controlled by client: The web contents associated with the dialog is destroyed. */
+    int WEB_CONTENTS_DESTROYED = 11;
 }