Reland "Remove ApplicationStatus usage from PhotoPicker."

This relands commit 78a7ace4.

The original change somehow passed the CQ, and was reported to be
flaky, but I believe it was deterministically failing. The failure
was on a new assert in SelectFileDialog. This CL fixes the issue
by moving the onPhotoPickerDismissed call into PhotoPickerListener,
so that the tests, which don't involve SelectFileDialog, won't call into
it. This also has the nice effect of removing the direct dependency
from photo_picker on SelectFileDialog.

Original change's description:
> Remove ApplicationStatus usage from PhotoPicker.
>
> ApplicationStatus only works in //chrome. See crbug.com/470582
>
> Bug: 1110930
> Change-Id: I71a6a5701aa40434588830cfcc6dee8ff6f5143a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378633
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803247}

TBR=dtrainor@chromium.org

Bug: 1123848,1110930

Change-Id: Id5c0ae00b85d97cb5121acf1ece8d37ac8e2d021
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2387624Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804053}

chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java

@@ -94,6 +94,7 @@ import org.chromium.content_public.common.ContentSwitches;
 import org.chromium.ui.ContactsPickerListener;
 import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.Clipboard;
+import org.chromium.ui.base.PhotoPicker;
 import org.chromium.ui.base.PhotoPickerDelegate;
 import org.chromium.ui.base.PhotoPickerListener;
 import org.chromium.ui.base.SelectFileDialog;
@@ -215,23 +216,17 @@ public class ProcessInitializationHandler {
 
         if (ChromeFeatureList.isEnabled(ChromeFeatureList.NEW_PHOTO_PICKER)) {
             SelectFileDialog.setPhotoPickerDelegate(new PhotoPickerDelegate() {
-                private PhotoPickerDialog mDialog;
-
                 @Override
-                public void showPhotoPicker(WindowAndroid windowAndroid,
+                public PhotoPicker showPhotoPicker(WindowAndroid windowAndroid,
                         PhotoPickerListener listener, boolean allowMultiple,
                         List<String> mimeTypes) {
-                    mDialog = new PhotoPickerDialog(windowAndroid,
+                    PhotoPickerDialog dialog = new PhotoPickerDialog(windowAndroid,
                             windowAndroid.getContext().get().getContentResolver(), listener,
                             allowMultiple, mimeTypes);
-                    mDialog.getWindow().getAttributes().windowAnimations =
+                    dialog.getWindow().getAttributes().windowAnimations =
                             R.style.PickerDialogAnimation;
-                    mDialog.show();
-                }
-
-                @Override
-                public void onPhotoPickerDismissed() {
-                    mDialog = null;
+                    dialog.show();
+                    return dialog;
                 }
 
                 @Override

components/browser_ui/photo_picker/android/DEPS

@@ -6,10 +6,3 @@ include_rules = [
   "+sandbox",
   "+ui/android",
 ]
-
-specific_include_rules = {
-  "PhotoPickerDialog\.java": [
-    # TODO(crbug.com/470582): remove this exemption.
-    "+base/android/java/src/org/chromium/base/ApplicationStatus.java",
-  ]
-}

components/browser_ui/photo_picker/android/java/src/org/chromium/components/browser_ui/photo_picker/PhotoPickerDialog.java

@@ -4,17 +4,13 @@
 
 package org.chromium.components.browser_ui.photo_picker;
 
-import android.app.Activity;
 import android.content.ContentResolver;
 import android.net.Uri;
 
 import androidx.annotation.VisibleForTesting;
 import androidx.appcompat.app.AlertDialog;
 
-import org.chromium.base.ActivityState;
-import org.chromium.base.ApplicationStatus;
-import org.chromium.base.ApplicationStatus.ActivityStateListener;
-import org.chromium.base.ContextUtils;
+import org.chromium.ui.base.PhotoPicker;
 import org.chromium.ui.base.PhotoPickerListener;
 import org.chromium.ui.base.WindowAndroid;
 
@@ -25,7 +21,7 @@ import java.util.List;
  * <input type=file accept=image > form element.
  */
 public class PhotoPickerDialog
-        extends AlertDialog implements PhotoPickerToolbar.PhotoPickerToolbarDelegate {
+        extends AlertDialog implements PhotoPickerToolbar.PhotoPickerToolbarDelegate, PhotoPicker {
     // Our window.
     private WindowAndroid mWindowAndroid;
 
@@ -68,6 +64,11 @@ public class PhotoPickerDialog
             mListener.onPhotoPickerUserAction(action, photos);
         }
 
+        @Override
+        public void onPhotoPickerDismissed() {
+            mListener.onPhotoPickerDismissed();
+        }
+
         /**
          * Returns whether the user picked an external intent to launch.
          */
@@ -115,23 +116,7 @@ public class PhotoPickerDialog
         if (!mListenerWrapper.externalIntentSelected() || mDoneWaitingForExternalIntent) {
             super.dismiss();
             mCategoryView.onDialogDismissed();
-        } else {
-            ApplicationStatus.registerStateListenerForActivity(new ActivityStateListener() {
-                @Override
-                public void onActivityStateChange(Activity activity, int newState) {
-                    // When an external intent, such as the Camera intent, is launched, this
-                    // listener will first receive the PAUSED event. Normally, STOPPED is the next
-                    // event, as the Camera intent appears. But if the user presses Back quickly
-                    // after the PAUSED event, the STOPPED event will not arrive, and this listener
-                    // gets RESUMED instead. However, we are already in teardown mode, so the
-                    // safe thing to do is to close the dialog.
-                    if (newState == ActivityState.STOPPED || newState == ActivityState.RESUMED) {
-                        mDoneWaitingForExternalIntent = true;
-                        ApplicationStatus.unregisterActivityStateListener(this);
-                        dismiss();
-                    }
-                }
-            }, ContextUtils.activityFromContext(mWindowAndroid.getContext().get()));
+            mListenerWrapper.onPhotoPickerDismissed();
         }
     }
 
@@ -143,6 +128,14 @@ public class PhotoPickerDialog
         cancel();
     }
 
+    // PhotoPicker:
+
+    @Override
+    public void onExternalIntentCompleted() {
+        mDoneWaitingForExternalIntent = true;
+        dismiss();
+    }
+
     @VisibleForTesting
     public PickerCategoryView getCategoryViewForTesting() {
         return mCategoryView;

components/browser_ui/photo_picker/android/java/src/org/chromium/components/browser_ui/photo_picker/PhotoPickerDialogTest.java

@@ -29,7 +29,6 @@ import org.chromium.base.MathUtils;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.DisableIf;
-import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.MinAndroidSdkLevel;
 import org.chromium.base.test.util.UrlUtils;
@@ -55,7 +54,6 @@ import java.util.concurrent.TimeUnit;
 /**
  * Tests for the PhotoPickerDialog class.
  */
-@DisabledTest(message = "https://crbug.com/1123848")
 @RunWith(BaseJUnit4ClassRunner.class)
 public class PhotoPickerDialogTest extends DummyUiActivityTestCase
         implements PhotoPickerListener, SelectionObserver<PickerBitmap>,
@@ -99,6 +97,9 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
     // The list of currently selected photos (built piecemeal).
     private List<PickerBitmap> mCurrentPhotoSelection;
 
+    // True when {@link onPhotoPickerDismissed} has been called.
+    private boolean mDismissed;
+
     // A callback that fires when something is selected in the dialog.
     public final CallbackHelper mOnSelectionCallback = new CallbackHelper();
 
@@ -134,6 +135,7 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
 
     @After
     public void tearDown() throws Exception {
+        Assert.assertTrue(TestThreadUtils.runOnUiThreadBlocking(() -> { return mDismissed; }));
         TestThreadUtils.runOnUiThreadBlocking(() -> { mWindowAndroid.destroy(); });
     }
 
@@ -185,6 +187,12 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
         mOnActionCallback.notifyCalled();
     }
 
+    @Override
+    public void onPhotoPickerDismissed() {
+        Assert.assertFalse(mDismissed);
+        mDismissed = true;
+    }
+
     // DecoderServiceHost.DecoderStatusCallback:
 
     @Override
@@ -379,8 +387,6 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
 
         Assert.assertNull(mLastSelectedPhotos);
         Assert.assertEquals(PhotoPickerAction.CANCEL, mLastActionRecorded);
-
-        dismissDialog();
     }
 
     @Test
@@ -409,8 +415,6 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
         Assert.assertEquals(1, mLastSelectedPhotos.length);
         Assert.assertEquals(PhotoPickerAction.PHOTOS_SELECTED, mLastActionRecorded);
         Assert.assertEquals(mTestFiles.get(1).getUri().getPath(), mLastSelectedPhotos[0].getPath());
-
-        dismissDialog();
     }
 
     @Test
@@ -446,8 +450,6 @@ public class PhotoPickerDialogTest extends DummyUiActivityTestCase
         Assert.assertEquals(mTestFiles.get(0).getUri().getPath(), mLastSelectedPhotos[0].getPath());
         Assert.assertEquals(mTestFiles.get(2).getUri().getPath(), mLastSelectedPhotos[1].getPath());
         Assert.assertEquals(mTestFiles.get(4).getUri().getPath(), mLastSelectedPhotos[2].getPath());
-
-        dismissDialog();
     }
 
     @Test

components/browser_ui/photo_picker/android/java/src/org/chromium/components/browser_ui/photo_picker/PickerCategoryView.java

@@ -37,7 +37,6 @@ import org.chromium.components.browser_ui.widget.selectable_list.SelectableListL
 import org.chromium.components.browser_ui.widget.selectable_list.SelectionDelegate;
 import org.chromium.net.MimeTypeFilter;
 import org.chromium.ui.base.PhotoPickerListener;
-import org.chromium.ui.base.SelectFileDialog;
 import org.chromium.ui.base.WindowAndroid;
 
 import java.util.ArrayList;
@@ -275,6 +274,8 @@ public class PickerCategoryView extends RelativeLayout
             mDecoderServiceHost.unbind(mWindowAndroid.getContext().get());
             mDecoderServiceHost = null;
         }
+
+        mDialog = null;
     }
 
     /**
@@ -282,6 +283,7 @@ public class PickerCategoryView extends RelativeLayout
      * @param uri The uri of the video to start playing.
      */
     public void startVideoPlaybackAsync(Uri uri) {
+        if (mDialog == null) return;
         mVideoPlayer.startVideoPlaybackAsync(uri, mDialog.getWindow().getDecorView());
     }
 
@@ -637,8 +639,7 @@ public class PickerCategoryView extends RelativeLayout
     private void executeAction(
             @PhotoPickerListener.PhotoPickerAction int action, Uri[] photos, int umaId) {
         mListener.onPhotoPickerUserAction(action, photos);
-        mDialog.dismiss();
-        SelectFileDialog.onPhotoPickerDismissed();
+        if (mDialog != null) mDialog.dismiss();
         recordFinalUmaStats(umaId);
     }
 

ui/android/BUILD.gn

@@ -255,6 +255,7 @@ android_library("ui_no_recycler_view_java") {
     "java/src/org/chromium/ui/base/IntentWindowAndroid.java",
     "java/src/org/chromium/ui/base/LocalizationUtils.java",
     "java/src/org/chromium/ui/base/PermissionCallback.java",
+    "java/src/org/chromium/ui/base/PhotoPicker.java",
     "java/src/org/chromium/ui/base/PhotoPickerDelegate.java",
     "java/src/org/chromium/ui/base/PhotoPickerListener.java",
     "java/src/org/chromium/ui/base/ResourceBundle.java",

ui/android/java/src/org/chromium/ui/base/PhotoPicker.java

+// Copyright 2020 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.
+
+package org.chromium.ui.base;
+
+/**
+ * An interface for the custom image file picker.
+ * See {@link SelectFileDialog}.
+ */
+public interface PhotoPicker {
+    /**
+     * Called after use of the PhotoPicker results in an external intent.
+     * When the PhotoPicker is used to choose the camera, for example, {@link SelectFileDialog} will
+     * launch a camera intent. When that intent is done, this will be called. This allows the
+     * PhotoPicker to defer dismissing until the Camera intent has been shown and completed.
+     */
+    void onExternalIntentCompleted();
+}

ui/android/java/src/org/chromium/ui/base/PhotoPickerDelegate.java

@@ -15,15 +15,11 @@ public interface PhotoPickerDelegate {
      *                 picker.
      * @param allowMultiple Whether the dialog should allow multiple images to be selected.
      * @param mimeTypes A list of mime types to show in the dialog.
+     * @return the PhotoPicker object.
      */
-    void showPhotoPicker(WindowAndroid windowAndroid, PhotoPickerListener listener,
+    PhotoPicker showPhotoPicker(WindowAndroid windowAndroid, PhotoPickerListener listener,
             boolean allowMultiple, List<String> mimeTypes);
 
-    /**
-     * Called when the photo picker dialog has been dismissed.
-     */
-    void onPhotoPickerDismissed();
-
     /**
      * Returns whether video decoding support is supported in the photo picker.
      */

ui/android/java/src/org/chromium/ui/base/PhotoPickerListener.java

@@ -41,4 +41,7 @@ public interface PhotoPickerListener {
      * @param photos The photos that were selected.
      */
     void onPhotoPickerUserAction(@PhotoPickerAction int action, Uri[] photos);
+
+    /** Called when the dialog has been dismissed. */
+    void onPhotoPickerDismissed();
 }

ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java

@@ -115,6 +115,9 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
     /** A delegate for the photo picker. */
     private static PhotoPickerDelegate sPhotoPickerDelegate;
 
+    /** The active photo picker, or null if none is active. */
+    private static PhotoPicker sPhotoPicker;
+
     /**
      * Allows setting a delegate to override the default Android stock photo picker.
      * @param delegate A {@link PhotoPickerDelegate} instance.
@@ -123,14 +126,6 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
         sPhotoPickerDelegate = delegate;
     }
 
-    /**
-     * Called when the photo picker dialog has been dismissed.
-     */
-    public static void onPhotoPickerDismissed() {
-        if (sPhotoPickerDelegate == null) return;
-        sPhotoPickerDelegate.onPhotoPickerDismissed();
-    }
-
     SelectFileDialog(long nativeSelectFileDialog) {
         mNativeSelectFileDialog = nativeSelectFileDialog;
     }
@@ -438,6 +433,12 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
         }
     }
 
+    @Override
+    public void onPhotoPickerDismissed() {
+        assert sPhotoPicker != null;
+        sPhotoPicker = null;
+    }
+
     private class GetCameraIntentTask extends AsyncTask<Uri> {
         private Boolean mDirectToCamera;
         private WindowAndroid mWindow;
@@ -508,6 +509,8 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
         return photoFile;
     }
 
+    // WindowAndroid.IntentCallback:
+
     /**
      * Callback method to handle the intent results and pass on the path to the native
      * SelectFileDialog.
@@ -518,6 +521,10 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
     @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
     @Override
     public void onIntentCompleted(WindowAndroid window, int resultCode, Intent results) {
+        if (sPhotoPicker != null) {
+            sPhotoPicker.onExternalIntentCompleted();
+        }
+
         if (resultCode != Activity.RESULT_OK) {
             onFileNotSelected();
             return;
@@ -857,7 +864,9 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
     private static boolean showPhotoPicker(WindowAndroid windowAndroid,
             PhotoPickerListener listener, boolean allowMultiple, List<String> mimeTypes) {
         if (sPhotoPickerDelegate == null) return false;
-        sPhotoPickerDelegate.showPhotoPicker(windowAndroid, listener, allowMultiple, mimeTypes);
+        assert sPhotoPicker == null;
+        sPhotoPicker = sPhotoPickerDelegate.showPhotoPicker(
+                windowAndroid, listener, allowMultiple, mimeTypes);
         return true;
     }