Skip to content

Commit

Permalink
Photo Picker dialog: Simplify and consolidate the dialog dismiss path…
Browse files Browse the repository at this point in the history
… & UMA.

This should also fix a crash seen.

Bug: 758605
Change-Id: I50030c86ee3cdc1b5ca1b25163336a72fd973f0a
Reviewed-on: https://chromium-review.googlesource.com/668398
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502253}
  • Loading branch information
finnurbreki authored and Commit Bot committed Sep 15, 2017
1 parent 23d21f4 commit 387cce3
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ public void showPhotoPicker(Context context, PhotoPickerListener listener,
}

@Override
public void dismissPhotoPicker() {
mDialog.dismiss();
public void onPhotoPickerDismissed() {
mDialog = null;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.chromium.chrome.browser.widget.selection.SelectableListLayout;
import org.chromium.chrome.browser.widget.selection.SelectionDelegate;
import org.chromium.ui.PhotoPickerListener;
import org.chromium.ui.UiUtils;

import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -210,8 +211,7 @@ public void initialize(PhotoPickerDialog dialog, PhotoPickerListener listener,
mDialog.setOnCancelListener(new DialogInterface.OnCancelListener() {
@Override
public void onCancel(DialogInterface dialog) {
recordFinalUmaStats(ACTION_CANCEL);
mListener.onPickerUserAction(PhotoPickerListener.Action.CANCEL, null);
executeAction(PhotoPickerListener.Action.CANCEL, null, ACTION_CANCEL);
}
});
}
Expand Down Expand Up @@ -257,14 +257,10 @@ public void onViewRecycled(RecyclerView.ViewHolder holder) {
@Override
public void onClick(View view) {
if (view.getId() == R.id.done) {
recordFinalUmaStats(ACTION_PHOTO_PICKED);
notifyPhotosSelected();
} else {
recordFinalUmaStats(ACTION_CANCEL);
mListener.onPickerUserAction(PhotoPickerListener.Action.CANCEL, null);
executeAction(PhotoPickerListener.Action.CANCEL, null, ACTION_CANCEL);
}

mDialog.dismiss();
}

/**
Expand Down Expand Up @@ -319,16 +315,14 @@ public boolean isMultiSelectAllowed() {
* Notifies the listener that the user selected to launch the gallery.
*/
public void showGallery() {
recordFinalUmaStats(ACTION_BROWSE);
mListener.onPickerUserAction(PhotoPickerListener.Action.LAUNCH_GALLERY, null);
executeAction(PhotoPickerListener.Action.LAUNCH_GALLERY, null, ACTION_BROWSE);
}

/**
* Notifies the listener that the user selected to launch the camera intent.
*/
public void showCamera() {
recordFinalUmaStats(ACTION_NEW_PHOTO);
mListener.onPickerUserAction(PhotoPickerListener.Action.LAUNCH_CAMERA, null);
executeAction(PhotoPickerListener.Action.LAUNCH_CAMERA, null, ACTION_NEW_PHOTO);
}

/**
Expand Down Expand Up @@ -381,7 +375,7 @@ private void notifyPhotosSelected() {
photos[i++] = bitmap.getFilePath();
}

mListener.onPickerUserAction(PhotoPickerListener.Action.PHOTOS_SELECTED, photos);
executeAction(PhotoPickerListener.Action.PHOTOS_SELECTED, photos, ACTION_PHOTO_PICKED);
}

/**
Expand Down Expand Up @@ -421,6 +415,19 @@ public void getItemOffsets(
}
}

/**
* Report back what the user selected in the dialog, report UMA and clean up.
* @param action The action taken.
* @param photos The photos that were selected (if any).
* @param umaId The UMA value to record with the action.
*/
private void executeAction(PhotoPickerListener.Action action, String[] photos, int umaId) {
mListener.onPickerUserAction(action, photos);
mDialog.dismiss();
UiUtils.onPhotoPickerDismissed();
recordFinalUmaStats(umaId);
}

/**
* Record UMA statistics (what action was taken in the dialog and other performance stats).
* @param action The action the user took in the dialog.
Expand Down
10 changes: 5 additions & 5 deletions ui/android/java/src/org/chromium/ui/UiUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ void showPhotoPicker(Context context, PhotoPickerListener listener, boolean allo
List<String> mimeTypes);

/**
* Called when the photo picker dialog should be dismissed.
* Called when the photo picker dialog has been dismissed.
*/
void dismissPhotoPicker();
void onPhotoPickerDismissed();
}

// PhotoPickerDelegate:
Expand Down Expand Up @@ -131,11 +131,11 @@ public static boolean showPhotoPicker(Context context, PhotoPickerListener liste
}

/**
* Called when the photo picker dialog should be dismissed.
* Called when the photo picker dialog has been dismissed.
*/
public static void dismissPhotoPicker() {
public static void onPhotoPickerDismissed() {
if (sPhotoPickerDelegate == null) return;
sPhotoPickerDelegate.dismissPhotoPicker();
sPhotoPickerDelegate.onPhotoPickerDismissed();
}

// KeyboardShowingDelegate:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,6 @@ public static String ensureMimeType(String type) {

@Override
public void onPickerUserAction(Action action, String[] photos) {
UiUtils.dismissPhotoPicker();

switch (action) {
case CANCEL:
onFileNotSelected();
Expand Down

0 comments on commit 387cce3

Please sign in to comment.