Commit c9241628 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Fix the stop action.

Before this change, the stop action, which indirectly calls
AutofillAssistantUiController.onShutdown, would just leave the UI open,
even after deleting the native instances. This isn't what should happen;
stop/shutdown should close the UI. This also causes crashes, if the user
tries to dismiss the UI.

With this change, the UI is hidden by stop/shutdown before the native
instances are deleted.

Bug: 806868
Change-Id: Ia5d4d2e8973d0a68fb5bca4c3841429d91d1b98f
Reviewed-on: https://chromium-review.googlesource.com/c/1309735
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604555}
parent cdb01a48
......@@ -116,7 +116,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
public void onActivityAttachmentChanged(Tab tab, boolean isAttached) {
if (!isAttached) {
activityTab.removeObserver(this);
nativeDestroy(mUiControllerAndroid);
mUiDelegateHolder.shutdown();
}
}
});
......@@ -129,7 +129,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
currentTabModel.removeObserver(this);
// Assume newly selected tab is always different from the last one.
nativeDestroy(mUiControllerAndroid);
mUiDelegateHolder.shutdown();
// TODO(crbug.com/806868): May start a new Autofill Assistant instance for the newly
// selected Tab.
}
......@@ -138,29 +138,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
@Override
public void onDismiss() {
mUiDelegateHolder.performUiOperation(uiDelegate -> {
uiDelegate.hide();
mUiDelegateHolder.pauseUiOperations();
// Show the UI back when unpaused.
mUiDelegateHolder.performUiOperation(AutofillAssistantUiDelegate::show);
// We show a snackbar with "undo" button for a few seconds, and shutdown only if it is
// not cancelled.
uiDelegate.showAutofillAssistantStoppedSnackbar(
new SnackbarManager.SnackbarController() {
@Override
public void onAction(Object actionData) {
// Shutdown was cancelled.
mUiDelegateHolder.unpauseUiOperations();
}
@Override
public void onDismissNoAction(Object actionData) {
nativeDestroy(mUiControllerAndroid);
}
});
});
mUiDelegateHolder.showDismissSnackbar();
}
@Override
......@@ -222,7 +200,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
@CalledByNative
private void onShutdown() {
nativeDestroy(mUiControllerAndroid);
mUiDelegateHolder.shutdown();
}
@CalledByNative
......@@ -323,10 +301,12 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
* Class holder for the AutofillAssistantUiDelegate to make sure we don't make UI changes when
* we are in a pause state (i.e. few seconds before stopping completely).
*/
private static class UiDelegateHolder {
private class UiDelegateHolder {
private final AutofillAssistantUiDelegate mUiDelegate;
private boolean mShouldQueueUiOperations = false;
private boolean mHasBeenShutdown = false;
private SnackbarManager.SnackbarController mDismissSnackbar;
private final ArrayList<Callback<AutofillAssistantUiDelegate>> mPendingUiOperations =
new ArrayList<>();
......@@ -334,11 +314,69 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
mUiDelegate = uiDelegate;
}
/**
* Perform a UI operation:
* - directly if we are not in a pause state.
* - later if the shutdown is cancelled.
* - never if Autofill Assistant is shut down.
*/
public void performUiOperation(Callback<AutofillAssistantUiDelegate> operation) {
assert !mHasBeenShutdown;
if (mShouldQueueUiOperations) {
mPendingUiOperations.add(operation);
return;
}
operation.onResult(mUiDelegate);
}
/**
* Hides the UI, pauses UI operations and, unless undone within the time delay, eventually
* destroy everything.
*/
public void showDismissSnackbar() {
assert !mHasBeenShutdown;
pauseUiOperations();
mUiDelegate.hide();
mDismissSnackbar = new SnackbarManager.SnackbarController() {
@Override
public void onAction(Object actionData) {
// Shutdown was cancelled.
mDismissSnackbar = null;
mUiDelegate.show();
unpauseUiOperations();
}
@Override
public void onDismissNoAction(Object actionData) {
shutdown();
}
};
mUiDelegate.showAutofillAssistantStoppedSnackbar(mDismissSnackbar);
}
/** Hides the UI and destroys everything. */
public void shutdown() {
if (mHasBeenShutdown) {
return;
}
mHasBeenShutdown = true;
mPendingUiOperations.clear();
if (mDismissSnackbar != null) {
mUiDelegate.dismissSnackbar(mDismissSnackbar);
}
mUiDelegate.hide();
nativeDestroy(mUiControllerAndroid);
}
/**
* Pause all UI operations such that they can potentially be ran later using {@link
* #unpauseUiOperations()}.
*/
public void pauseUiOperations() {
private void pauseUiOperations() {
mShouldQueueUiOperations = true;
}
......@@ -346,28 +384,13 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
* Unpause and trigger all UI operations received by {@link #performUiOperation(Callback)}
* since the last {@link #pauseUiOperations()}.
*/
public void unpauseUiOperations() {
private void unpauseUiOperations() {
mShouldQueueUiOperations = false;
for (int i = 0; i < mPendingUiOperations.size(); i++) {
mPendingUiOperations.get(i).onResult(mUiDelegate);
}
mPendingUiOperations.clear();
}
/**
* Perform a UI operation:
* - directly if we are not in a pause state.
* - later if the shutdown is cancelled.
* - never if Autofill Assistant is shut down.
*/
public void performUiOperation(Callback<AutofillAssistantUiDelegate> operation) {
if (mShouldQueueUiOperations) {
mPendingUiOperations.add(operation);
return;
}
operation.onResult(mUiDelegate);
}
}
@CalledByNative
......
......@@ -353,6 +353,10 @@ class AutofillAssistantUiDelegate {
mActivity.getSnackbarManager().showSnackbar(snackBar);
}
public void dismissSnackbar(SnackbarManager.SnackbarController controller) {
mActivity.getSnackbarManager().dismissSnackbars(controller);
}
/** Called to show overlay. */
public void showOverlay() {
mOverlay.setVisibility(View.VISIBLE);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment