[Screenshot] retrying installation closes fallback dialog.

The fallback dialog passes its dismissal runnable to be called on a
successful image_editor installation.

Bug: 1117675
Change-Id: I08a536b77ec459741940d20082b270aa4e900f1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363813
Commit-Queue: Jeffrey Cohen <jeffreycohen@chromium.org>
Reviewed-by: Tanya Gupta <tgupta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800262}

chrome/browser/share/android/java/src/org/chromium/chrome/browser/share/screenshot/ScreenshotCoordinator.java

@@ -66,7 +66,7 @@ public class ScreenshotCoordinator {
             launchEditor();
         } else if (sInstallAttempts < MAX_INSTALL_ATTEMPTS) {
             sInstallAttempts++;
-            installEditor(true);
+            installEditor(true, /* onSuccessRunnable= */ null);
         } else {
             launchSharesheet();
         }
@@ -95,17 +95,19 @@ public class ScreenshotCoordinator {
 
     /**
      * Runnable friendly helper function to retry the installation after going to the fallback.
+     * @param onSuccess. Runnable to run on success.
      */
-    protected void retryInstallEditor() {
-        installEditor(false);
+    protected void retryInstallEditor(Runnable onSuccess) {
+        installEditor(false, onSuccess);
     }
 
     /**
      * Installs the DFM and shows UI (i.e. toasts and a retry dialog) informing the
      * user of the installation status.
      * @param showFallback The fallback will be shown on a unsuccessful installation.
+     * @param onSuccessRunnable the runnable to run on a succesfful install.
      */
-    private void installEditor(boolean showFallback) {
+    private void installEditor(boolean showFallback, Runnable onSuccessRunnable) {
         final ModuleInstallUi ui = new ModuleInstallUi(
                 mTab, R.string.image_editor_module_title, new ModuleInstallUi.FailureUiListener() {
                     @Override
@@ -113,7 +115,7 @@ public class ScreenshotCoordinator {
                         if (retry) {
                             // User initiated retries are not counted toward the maximum number
                             // of install attempts per session.
-                            installEditor(showFallback);
+                            installEditor(showFallback, onSuccessRunnable);
                         } else if (showFallback) {
                             launchSharesheet();
                         }
@@ -124,6 +126,9 @@ public class ScreenshotCoordinator {
         ImageEditorModuleProvider.maybeInstallModule((success) -> {
             if (success) {
                 ui.showInstallSuccessUi();
+                if (onSuccessRunnable != null) {
+                    onSuccessRunnable.run();
+                }
                 launchEditor();
             } else if (showFallback) {
                 launchSharesheet();

chrome/browser/share/android/java/src/org/chromium/chrome/browser/share/screenshot/ScreenshotShareSheetCoordinator.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.share.screenshot;
 import android.content.Context;
 import android.graphics.Bitmap;
 
+import org.chromium.base.Callback;
 import org.chromium.chrome.browser.share.share_sheet.ChromeOptionShareCallback;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.ui.modelutil.PropertyKey;
@@ -33,11 +34,11 @@ public class ScreenshotShareSheetCoordinator {
      * @param screenshotShareSheetView the view for the screenshot share sheet.
      * @param tab The tab that launched this screenshot.
      * @param shareSheetCallback The callback to be called on share.
-     * @param deleteRunanble The runnable to be called on retry.
+     * @param installCallback The callback to be called on retry.
      */
     public ScreenshotShareSheetCoordinator(Context context, Bitmap screenshot,
             Runnable deleteRunnable, ScreenshotShareSheetView screenshotShareSheetView, Tab tab,
-            ChromeOptionShareCallback shareSheetCallback, Runnable installCallback) {
+            ChromeOptionShareCallback shareSheetCallback, Callback<Runnable> installCallback) {
         ArrayList<PropertyKey> allProperties =
                 new ArrayList<>(Arrays.asList(ScreenshotShareSheetViewProperties.ALL_KEYS));
         mModel = new PropertyModel(allProperties);

chrome/browser/share/android/java/src/org/chromium/chrome/browser/share/screenshot/ScreenshotShareSheetDialog.java

@@ -12,6 +12,7 @@ import android.os.Bundle;
 
 import androidx.appcompat.app.AlertDialog;
 
+import org.chromium.base.Callback;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.share.share_sheet.ChromeOptionShareCallback;
 import org.chromium.chrome.browser.tab.Tab;
@@ -23,10 +24,9 @@ public class ScreenshotShareSheetDialog extends DialogFragment {
     private Context mContext;
     private ScreenshotShareSheetView mDialogView;
     private Bitmap mScreenshot;
-    private Runnable mDeleteRunnable;
     private Tab mTab;
-    private ChromeOptionShareCallback mShareCallback;
-    private Runnable mInstallCallback;
+    private ChromeOptionShareCallback mChromeOptionShareCallback;
+    private Callback<Runnable> mInstallCallback;
 
     /**
      * The ScreenshotShareSheetDialog constructor.
@@ -36,17 +36,17 @@ public class ScreenshotShareSheetDialog extends DialogFragment {
     /**
      * Initialize the dialog outside of the constructor as fragments require default constructor.
      * @param screenshot The screenshot image to show.
-     * @param deleteRunnable The function to call on delete.
      * @param tab The shared tab.
-     * @param shareSheetCoordnator the base share sheet coordinator
+     * @param chromeOptionShareCallback the callback to trigger on share.
+     * @param installCallback the callback to trigger on install.
      */
-    public void init(Bitmap screenshot, Runnable deleteRunnable, Tab tab,
-            ChromeOptionShareCallback shareSheetCallback, Runnable installCallback) {
+    public void init(Bitmap screenshot, Tab tab,
+            ChromeOptionShareCallback chromeOptionShareCallback,
+            Callback<Runnable> installCallback) {
         mScreenshot = screenshot;
-        mDeleteRunnable = deleteRunnable;
         mInstallCallback = installCallback;
         mTab = tab;
-        mShareCallback = shareSheetCallback;
+        mChromeOptionShareCallback = chromeOptionShareCallback;
     }
 
     @Override
@@ -64,9 +64,9 @@ public class ScreenshotShareSheetDialog extends DialogFragment {
                         R.layout.screenshot_share_sheet, null);
         builder.setView(screenshotShareSheetView);
 
-        ScreenshotShareSheetCoordinator shareCoordinator =
-                new ScreenshotShareSheetCoordinator(mContext, mScreenshot, mDeleteRunnable,
-                        screenshotShareSheetView, mTab, mShareCallback, mInstallCallback);
+        ScreenshotShareSheetCoordinator shareCoordinator = new ScreenshotShareSheetCoordinator(
+                mContext, mScreenshot, this::dismiss, screenshotShareSheetView, mTab,
+                mChromeOptionShareCallback, mInstallCallback);
         return builder.create();
     }
 }

chrome/browser/share/android/java/src/org/chromium/chrome/browser/share/screenshot/ScreenshotShareSheetDialogCoordinator.java

@@ -8,6 +8,7 @@ import android.app.Activity;
 import android.app.FragmentManager;
 import android.graphics.Bitmap;
 
+import org.chromium.base.Callback;
 import org.chromium.chrome.browser.share.share_sheet.ChromeOptionShareCallback;
 import org.chromium.chrome.browser.tab.Tab;
 /**
@@ -23,13 +24,17 @@ public class ScreenshotShareSheetDialogCoordinator {
      *
      * @param context The context to use for user permissions.
      * @param screenshot The screenshot to be shared.
+     * @param tab The shared tab.
+     * @param chromeOptionShareCallback the callback to trigger on share.
+     * @param installCallback the callback to trigger on install.
      */
     public ScreenshotShareSheetDialogCoordinator(Activity activity, Bitmap screenshot, Tab tab,
-            ChromeOptionShareCallback shareCallback, Runnable installCallback) {
+            ChromeOptionShareCallback chromeOptionShareCallback,
+            Callback<Runnable> installCallback) {
         mFragmentManager = activity.getFragmentManager();
         mScreenshot = screenshot;
         mDialog = new ScreenshotShareSheetDialog();
-        mDialog.init(mScreenshot, this::dismiss, tab, shareCallback, installCallback);
+        mDialog.init(mScreenshot, tab, chromeOptionShareCallback, installCallback);
     }
 
     /**
@@ -38,11 +43,4 @@ public class ScreenshotShareSheetDialogCoordinator {
     protected void showShareSheet() {
         mDialog.show(mFragmentManager, null);
     }
-
-    /**
-     * Dismiss the main dialog.
-     */
-    public void dismiss() {
-        mDialog.dismiss();
-    }
 }

chrome/browser/share/android/java/src/org/chromium/chrome/browser/share/screenshot/ScreenshotShareSheetMediator.java

@@ -26,8 +26,8 @@ class ScreenshotShareSheetMediator {
     private final PropertyModel mModel;
     private final Context mContext;
     private final Runnable mSaveRunnable;
-    private final Runnable mDeleteRunnable;
-    private final Runnable mInstallRunnable;
+    private final Runnable mCloseDialogRunnable;
+    private final Callback<Runnable> mInstallCallback;
     private final ChromeOptionShareCallback mChromeOptionShareCallback;
 
     private final Tab mTab;
@@ -36,22 +36,24 @@ class ScreenshotShareSheetMediator {
      * The ScreenshotShareSheetMediator constructor.
      * @param context The context to use.
      * @param propertyModel The property model to use to communicate with views.
-     * @param deleteRunnable The action to take when cancel or delete is called.
+     * @param closeDialogRunnable The action to take to close the dialog.
      * @param saveRunnable The action to take when save is called.
      * @param tab The tab that originated this screenshot.
      * @param chromeOptionShareCallback The callback to share a screenshot via the share sheet.
-     * @param installRunnable The action to take when install is called.
+     * @param installCallback The action to take when install is called, will call runnable on
+     *         success.
      */
     ScreenshotShareSheetMediator(Context context, PropertyModel propertyModel,
-            Runnable deleteRunnable, Runnable saveRunnable, Tab tab,
-            ChromeOptionShareCallback chromeOptionShareCallback, Runnable installRunnable) {
-        mDeleteRunnable = deleteRunnable;
+            Runnable closeDialogRunnable, Runnable saveRunnable, Tab tab,
+            ChromeOptionShareCallback chromeOptionShareCallback,
+            Callback<Runnable> installCallback) {
+        mCloseDialogRunnable = closeDialogRunnable;
         mSaveRunnable = saveRunnable;
         mContext = context;
         mModel = propertyModel;
         mTab = tab;
         mChromeOptionShareCallback = chromeOptionShareCallback;
-        mInstallRunnable = installRunnable;
+        mInstallCallback = installCallback;
         mModel.set(ScreenshotShareSheetViewProperties.NO_ARG_OPERATION_LISTENER,
                 operation -> { performNoArgOperation(operation); });
     }
@@ -67,11 +69,11 @@ class ScreenshotShareSheetMediator {
             share();
         } else if (NoArgOperation.SAVE == operation) {
             mSaveRunnable.run();
-            mDeleteRunnable.run();
+            mCloseDialogRunnable.run();
         } else if (NoArgOperation.DELETE == operation) {
-            mDeleteRunnable.run();
+            mCloseDialogRunnable.run();
         } else if (NoArgOperation.INSTALL == operation) {
-            mInstallRunnable.run();
+            mInstallCallback.onResult(mCloseDialogRunnable);
         }
     }
 
@@ -102,7 +104,7 @@ class ScreenshotShareSheetMediator {
         };
 
         generateTemporaryUriFromBitmap(mContext, title, bitmap, callback);
-        mDeleteRunnable.run();
+        mCloseDialogRunnable.run();
     }
 
     protected void generateTemporaryUriFromBitmap(

chrome/browser/share/android/javatests/src/org/chromium/chrome/browser/share/screenshot/ScreenshotShareSheetMediatorUnitTest.java

@@ -53,7 +53,7 @@ public class ScreenshotShareSheetMediatorUnitTest {
     Runnable mSaveRunnable;
 
     @Mock
-    Runnable mInstallRunnable;
+    Callback<Runnable> mInstallRunnable;
 
     @Mock
     Activity mContext;
@@ -71,9 +71,10 @@ public class ScreenshotShareSheetMediatorUnitTest {
 
         MockScreenshotShareSheetMediator(Context context, PropertyModel propertyModel,
                 Runnable deleteRunnable, Runnable saveRunnable, Tab tab,
-                ChromeOptionShareCallback chromeOptionShareCallback, Runnable installRunnable) {
+                ChromeOptionShareCallback chromeOptionShareCallback,
+                Callback<Runnable> installCallback) {
             super(context, propertyModel, deleteRunnable, saveRunnable, tab,
-                    chromeOptionShareCallback, installRunnable);
+                    chromeOptionShareCallback, installCallback);
         }
         @Override
         protected void generateTemporaryUriFromBitmap(
@@ -150,7 +151,7 @@ public class ScreenshotShareSheetMediatorUnitTest {
                 mModel.get(ScreenshotShareSheetViewProperties.NO_ARG_OPERATION_LISTENER);
         callback.onResult(ScreenshotShareSheetViewProperties.NoArgOperation.INSTALL);
 
-        verify(mInstallRunnable).run();
+        verify(mInstallRunnable).onResult(any());
     }
 
     @After