Skip to content

Commit

Permalink
[UPM] Fix password manager GMS UI loading dialog early dismissal
Browse files Browse the repository at this point in the history 
Client should now wait for the dialog to become dismissable before
launching new activity.

The dialog is dismissed after launching the pending intent plus a small
delay that enough for the new Activity to become visible. Otherwise
previous activity could become visible for a short period of time.

It is ok for the old activity to be immediately destroyed even before
the loading dialog was dismissed as the DialogManager dismisses all
dialogs when destroyed.

Loading timeout now only limits the dialog visibility period. This
could result in longer loading if the dialog was not shown initially
due to higher priority dialog currently visible.

https://screencast.googleplex.com/cast/NjUzMDkxNDUyOTA1MDYyNHw2MjY0ZDRjZS1kNA

Bug: 1318894
Change-Id: I0dd3d09e7a0102d95b8227ca476d6a552a363c45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3637059
Commit-Queue: Maxim Anufriev <maxan@google.com>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002072}
  • Loading branch information
Maxim Anufriev authored and Chromium LUCI CQ committed May 11, 2022
1 parent 7cb7052 commit 0ab9850
Show file tree
Hide file tree
Showing 6 changed files with 333 additions and 252 deletions.
81 changes: 61 additions & 20 deletions ...oid/java/src/org/chromium/chrome/browser/loading_modal/LoadingModalDialogCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import android.content.Context;
import android.os.Handler;
import android.os.Looper;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.RelativeLayout;
Expand Down Expand Up @@ -34,27 +35,42 @@ public class LoadingModalDialogCoordinator {
private final View mButtonsView;

// Used to indicate the current loading dialog state.
@IntDef({State.READY, State.LOADING_DELAYED, State.LOADING_SHOWN, State.FINISHED_SHOWN,
State.FINISHED, State.CANCELLED, State.TIMEOUT_SHOWN, State.TIMEOUT})
@IntDef({State.READY, State.PENDING, State.SHOWN, State.FINISHED, State.CANCELLED,
State.TIMED_OUT})
@Retention(RetentionPolicy.SOURCE)
public @interface State {
/** Loading is not started, the dialog is not shown. */
int READY = 0;
/** Loading in progress, the dialog is delayed. */
int LOADING_DELAYED = 1;
/** Loading in progress, the dialog is visible. */
int LOADING_SHOWN = 2;
/** Loading finished, the dialog dismissal is delayed. */
int FINISHED_SHOWN = 3;
/** Loading finished, the dialog is dismissed. */
int FINISHED = 4;
/** The dialog is scheduled to be shown after the default delay. */
int PENDING = 1;
/** The dialog is visible. */
int SHOWN = 2;
/**
* Dialog is dismissed by the client as the loading operation finished. It may be still
* visible for a short period to prevent UI flickering.
*/
int FINISHED = 3;
/** User dismissed the dialog before the loading finished. */
int CANCELLED = 5;
/** Loading timeout occurred but the dialog is still visible to prevent flickering. */
int TIMEOUT_SHOWN = 6;
int CANCELLED = 4;
/** Loading timeout occurred and the dialog was automatically dismissed. */
int TIMEOUT = 7;
int NUM_ENTRIES = 8;
int TIMED_OUT = 5;
int NUM_ENTRIES = 6;
}

/**
* An observer of the LoadingModalDialogCoordinator intended to broadcast notifications
* about the loading dialog dismissals and the readiness to be immediately dismissed.
*/
public interface Observer {
/**
* A notification that the dialog could be dismissed without causing the UI to flicker.
*/
default void onDismissable(){};

/**
* A notification that the dialog was dismissed with given final state.
*/
default void onDismissedWithState(@State int finalState){};
}

/**
Expand All @@ -66,7 +82,7 @@ public class LoadingModalDialogCoordinator {
*/
public static LoadingModalDialogCoordinator create(
ObservableSupplier<ModalDialogManager> modalDialogManagerSupplier, Context context) {
return create(modalDialogManagerSupplier, context, new Handler());
return create(modalDialogManagerSupplier, context, new Handler(Looper.getMainLooper()));
}

@VisibleForTesting
Expand Down Expand Up @@ -95,7 +111,10 @@ private LoadingModalDialogCoordinator(@NonNull LoadingModalDialogMediator dialog
mButtonsView = buttonsView;
}

/** Shows the loading modal dialog. */
/**
* Schedules the dialog to be shown after delay. The dialog will not be shown if
* {@link #finishLoading()} called before it become visible.
*/
public void show() {
PropertyModel dialogModel =
new PropertyModel.Builder(ModalDialogProperties.ALL_KEYS)
Expand All @@ -107,12 +126,17 @@ public void show() {
.build();
mButtonsView.findViewById(R.id.cancel_loading_modal)
.setOnClickListener(view -> mMediator.onClick(dialogModel, ButtonType.NEGATIVE));
mMediator.showDialog(dialogModel);
mMediator.show(dialogModel);
}

/** Dismisses the loading modal dialog. */
/**
* Dismisses the currently visible dialog or cancelling the pending dialog if it is not visible
* yet. If dialog is already visible for at least {@link #MINIMUM_SHOW_TIME_MS}, it will be
* dismissed immediately. Otherwise it will be dismissed after being visible for that period of
* time.
*/
public void dismiss() {
mMediator.dismissDialog();
mMediator.dismiss();
}

/**
Expand All @@ -135,4 +159,21 @@ void disableTimeoutForTesting() {
View getButtonsView() {
return mButtonsView;
}

/**
* Indicates if the dailog could be immediately dismissed.
*/
public boolean isImmediatelyDismissable() {
return mMediator.isImmediatelyDismissable();
}

/**
* Add the listener that will be notified when the dialog is cancelled, timed out or is ready to
* be dismissed.
*
* @param listener {@link Observer} that will be notified.
*/
public void addObserver(Observer listener) {
mMediator.addObserver(listener);
}
}
142 changes: 81 additions & 61 deletions ...ndroid/java/src/org/chromium/chrome/browser/loading_modal/LoadingModalDialogMediator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.os.Handler;
import android.os.SystemClock;

import org.chromium.base.ObserverList;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager;
Expand All @@ -24,10 +25,14 @@ class LoadingModalDialogMediator
implements ModalDialogProperties.Controller, ModalDialogManagerObserver {
private static final long SHOW_DELAY_TIME_MS = 500L;
private static final long MINIMUM_SHOW_TIME_MS = 500L;
private static final long LOAD_TIMEOUT_MS = 5000L;
// The load timeout limits the dialog visibility period, along with the show delay time it
// results in 5000ms total loading timeout.
private static final long LOAD_TIMEOUT_MS = 4500L;

private final Handler mHandler;
private final ObservableSupplier<ModalDialogManager> mDialogManagerSupplier;
private final ObserverList<LoadingModalDialogCoordinator.Observer> mObservers =
new ObserverList<>();

private ModalDialogManager mDialogManager;
private PropertyModel mModel;
Expand All @@ -41,14 +46,22 @@ class LoadingModalDialogMediator
/** ModalDialogProperties.Controller implementation */
@Override
public void onClick(PropertyModel model, @ButtonType int buttonType) {
dismissDialogImmediately(DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
mState = LoadingModalDialogCoordinator.State.CANCELLED;
dismissDialogWithCause(DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
}

@Override
public void onDismiss(PropertyModel model, @DialogDismissalCause int dismissalCause) {
mDialogManager.removeObserver(this);
mHandler.removeCallbacksAndMessages(null);
mState = getFinalStateByDismissalCause(dismissalCause);
if (mState == LoadingModalDialogCoordinator.State.CANCELLED
|| mState == LoadingModalDialogCoordinator.State.TIMED_OUT) {
for (LoadingModalDialogCoordinator.Observer observer : mObservers) {
observer.onDismissedWithState(mState);
}
}
mObservers.clear();
}

/**
Expand All @@ -63,8 +76,10 @@ public void onDismiss(PropertyModel model, @DialogDismissalCause int dismissalCa
@Override
public void onDialogAdded(PropertyModel model) {
if (model != mModel) return;
mState = LoadingModalDialogCoordinator.State.LOADING_SHOWN;
mShownAtMs = Long.valueOf(SystemClock.elapsedRealtime());
mState = LoadingModalDialogCoordinator.State.SHOWN;
mShownAtMs = SystemClock.elapsedRealtime();
postDelayed(this::onDismissDelayPassed, MINIMUM_SHOW_TIME_MS);
if (!mDisableTimeout) postDelayed(this::onTimeoutOccurred, LOAD_TIMEOUT_MS);
}

LoadingModalDialogMediator(
Expand All @@ -76,38 +91,50 @@ public void onDialogAdded(PropertyModel model) {
mHandler = handler;
}

/**
* Add the listener that will be notified about the loading dialog cancellation and readiness to
* be immediately dismissed.
*
* @param listener {@link LoadingModalDialogCoordinator.Observer} that will be notified.
*/
void addObserver(LoadingModalDialogCoordinator.Observer listener) {
mObservers.addObserver(listener);
}

/**
* Schedules the dialog to be shown after {@link #SHOW_DELAY_TIME_MS} milliseconds.
* The dialog will not be shown if {@link #dismissDialog()} called before it become visible.
* The dialog will not be shown if {@link #dismiss()} called before it become visible.
*
* @param model The {@link PropertyModel} describing the dialog to be shown.
*
*/
void showDialog(PropertyModel model) {
void show(PropertyModel model) {
assert mState == LoadingModalDialogCoordinator.State.READY;

ModalDialogManager dialogManager = mDialogManagerSupplier.get();
if (dialogManager == null) return;

mDialogManager = dialogManager;
mModel = model;
mState = LoadingModalDialogCoordinator.State.LOADING_DELAYED;
postDelayed(this::showDialogImmediately, SHOW_DELAY_TIME_MS);

if (mDisableTimeout) return;
Runnable timeoutDismissRunnable =
() -> dismissDialogWithCause(DialogDismissalCause.CLIENT_TIMEOUT);
postDelayed(timeoutDismissRunnable, LOAD_TIMEOUT_MS);
mState = LoadingModalDialogCoordinator.State.PENDING;
postDelayed(this::onShowDelayPassed, SHOW_DELAY_TIME_MS);
}

/**
* Dismisses the currently visible dialog or cancelling the pending dialog if it is not visible
* yet. If dialog is already visible for at least {@link #MINIMUM_SHOW_TIME_MS}, it will be
* dismissed immediately. Otherwise it will be dismissed after being visible for that period of
* time. This method should be called when the loading finishes.
* time.
*/
void dismissDialog() {
dismissDialogWithCause(DialogDismissalCause.ACTION_ON_DIALOG_COMPLETED);
void dismiss() {
if (mState == LoadingModalDialogCoordinator.State.PENDING) {
mHandler.removeCallbacks(this::onShowDelayPassed);
}

mState = LoadingModalDialogCoordinator.State.FINISHED;
if (isImmediatelyDismissable()) {
dismissDialogWithCause(DialogDismissalCause.ACTION_ON_DIALOG_COMPLETED);
}
}

/**
Expand All @@ -126,52 +153,60 @@ void disableTimeout() {
mDisableTimeout = true;
}

private void dismissDialogWithCause(@DialogDismissalCause int dismissalCause) {
if (mState != LoadingModalDialogCoordinator.State.LOADING_DELAYED
&& mState != LoadingModalDialogCoordinator.State.LOADING_SHOWN) {
return;
/**
* Indicates if the dailog could be immediately dismissed.
*/
boolean isImmediatelyDismissable() {
switch (mState) {
case LoadingModalDialogCoordinator.State.PENDING:
case LoadingModalDialogCoordinator.State.TIMED_OUT:
case LoadingModalDialogCoordinator.State.CANCELLED:
return true;
case LoadingModalDialogCoordinator.State.SHOWN:
case LoadingModalDialogCoordinator.State.FINISHED:
return mSkipDelay
|| SystemClock.elapsedRealtime() - mShownAtMs >= MINIMUM_SHOW_TIME_MS;
case LoadingModalDialogCoordinator.State.READY:
return false;
}
throw new AssertionError();
}

mHandler.removeCallbacksAndMessages(null);
private void onShowDelayPassed() {
if (mState == LoadingModalDialogCoordinator.State.PENDING) {
showDialogImmediately();
}
}

final long currentTimeMs = SystemClock.elapsedRealtime();
if (mState == LoadingModalDialogCoordinator.State.LOADING_SHOWN
&& mShownAtMs + MINIMUM_SHOW_TIME_MS > currentTimeMs) {
// Dialog dismiss should be postponed to prevent UI flicker.
switch (dismissalCause) {
case DialogDismissalCause.ACTION_ON_DIALOG_COMPLETED:
mState = LoadingModalDialogCoordinator.State.FINISHED_SHOWN;
break;
case DialogDismissalCause.CLIENT_TIMEOUT:
mState = LoadingModalDialogCoordinator.State.TIMEOUT_SHOWN;
break;
default:
assert false : "Unexpected dismissal cause: " + dismissalCause;
break;
}
Runnable dismissRunnable = () -> dismissDialogImmediately(dismissalCause);
postDelayed(dismissRunnable, mShownAtMs + MINIMUM_SHOW_TIME_MS - currentTimeMs);
} else {
// Dialog is not yet shown or has been visible long enough.
dismissDialogImmediately(dismissalCause);
private void onDismissDelayPassed() {
for (LoadingModalDialogCoordinator.Observer observer : mObservers) observer.onDismissable();
if (mState == LoadingModalDialogCoordinator.State.FINISHED) {
dismissDialogWithCause(DialogDismissalCause.ACTION_ON_DIALOG_COMPLETED);
}
}

private void onTimeoutOccurred() {
if (mState != LoadingModalDialogCoordinator.State.SHOWN) return;

mState = LoadingModalDialogCoordinator.State.TIMED_OUT;
dismissDialogWithCause(DialogDismissalCause.CLIENT_TIMEOUT);
}

/** Immediately shows the dialog. */
private void showDialogImmediately() {
if (mState != LoadingModalDialogCoordinator.State.LOADING_DELAYED) return;
assert mState == LoadingModalDialogCoordinator.State.PENDING;
mDialogManager.addObserver(this);
mDialogManager.showDialog(mModel, ModalDialogManager.ModalDialogType.TAB);
}

/**
* Immediately dismisses the dialog.
* Immediately dismisses the dialog with {@link DialogDismissalCause}.
*
* @param dismissalCause The {@link DialogDismissalCause} that describes why the dialog is
* dismissed.
*/
private void dismissDialogImmediately(@DialogDismissalCause int dismissalCause) {
if (!canBeImmediatelyDismissed()) return;
private void dismissDialogWithCause(@DialogDismissalCause int dismissalCause) {
assert isImmediatelyDismissable();
mDialogManager.dismissDialog(mModel, dismissalCause);
}

Expand All @@ -183,28 +218,13 @@ private void postDelayed(Runnable r, long delay) {
}
}

private boolean canBeImmediatelyDismissed() {
switch (mState) {
case LoadingModalDialogCoordinator.State.LOADING_DELAYED:
case LoadingModalDialogCoordinator.State.LOADING_SHOWN:
case LoadingModalDialogCoordinator.State.FINISHED_SHOWN:
case LoadingModalDialogCoordinator.State.TIMEOUT_SHOWN:
return true;
case LoadingModalDialogCoordinator.State.READY:
case LoadingModalDialogCoordinator.State.FINISHED:
case LoadingModalDialogCoordinator.State.CANCELLED:
return false;
}
throw new AssertionError();
}

private @LoadingModalDialogCoordinator.State int getFinalStateByDismissalCause(
@DialogDismissalCause int dismissalCause) {
switch (dismissalCause) {
case DialogDismissalCause.ACTION_ON_DIALOG_COMPLETED:
return LoadingModalDialogCoordinator.State.FINISHED;
case DialogDismissalCause.CLIENT_TIMEOUT:
return LoadingModalDialogCoordinator.State.TIMEOUT;
return LoadingModalDialogCoordinator.State.TIMED_OUT;
default:
return LoadingModalDialogCoordinator.State.CANCELLED;
}
Expand Down

0 comments on commit 0ab9850

Please sign in to comment.