Skip to content

Commit

Permalink
[Autofill Assistant] Handle back button
Browse files Browse the repository at this point in the history 
The Autofill Assistant now handles back button clicks by removing its
UI and stopping.

Bug: b/152954282
Change-Id: I9c86d45c58b036147c5d729801faae2484068fde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207177
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Marian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#773120}
  • Loading branch information
sandromaggi authored and Commit Bot committed May 29, 2020
1 parent 4cdcb33 commit 65708d4
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 13 deletions.
5 changes: 3 additions & 2 deletions ...ava/src/org/chromium/chrome/browser/autofill_assistant/AssistantBottomBarCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class AssistantBottomBarCoordinator implements AssistantPeekHeightCoordinator.De
AssistantBottomBarCoordinator(Activity activity, AssistantModel model,
BottomSheetController controller,
ApplicationViewportInsetSupplier applicationViewportInsetSupplier,
TabObscuringHandler tabObscuringHandler) {
TabObscuringHandler tabObscuringHandler,
AssistantBottomSheetContent.Delegate bottomSheetDelegate) {
mModel = model;
mBottomSheetController = controller;
mTabObscuringHandler = tabObscuringHandler;
Expand All @@ -103,7 +104,7 @@ class AssistantBottomBarCoordinator implements AssistantPeekHeightCoordinator.De
if (currentSheetContent instanceof AssistantBottomSheetContent) {
mContent = (AssistantBottomSheetContent) currentSheetContent;
} else {
mContent = new AssistantBottomSheetContent(activity);
mContent = new AssistantBottomSheetContent(activity, bottomSheetDelegate);
}

// Replace or set the content to the actual Autofill Assistant views.
Expand Down
18 changes: 17 additions & 1 deletion .../java/src/org/chromium/chrome/browser/autofill_assistant/AssistantBottomSheetContent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,24 @@
* practice, this allows to replace the onboarding by the actual Autofill Assistant content).
*/
class AssistantBottomSheetContent implements BottomSheetContent {
interface Delegate {
boolean onBackButtonPressed();
}

private final View mToolbarView;
private final SizeListenableLinearLayout mContentView;
@Nullable
private ScrollView mContentScrollableView;
@Nullable
private Delegate mDelegate;

public AssistantBottomSheetContent(Context context) {
public AssistantBottomSheetContent(Context context, @Nullable Delegate delegate) {
mToolbarView = LayoutInflater.from(context).inflate(
R.layout.autofill_assistant_bottom_sheet_toolbar, /* root= */ null);
mContentView = new SizeListenableLinearLayout(context);
mContentView.setLayoutParams(new ViewGroup.LayoutParams(
ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.WRAP_CONTENT));
mDelegate = delegate;
}

public void setContent(View content, ScrollView scrollableView) {
Expand Down Expand Up @@ -123,4 +130,13 @@ public int getSheetFullHeightAccessibilityStringId() {
public int getSheetClosedAccessibilityStringId() {
return R.string.autofill_assistant_sheet_closed;
}

@Override
public boolean handleBackPress() {
if (mDelegate == null) {
return false;
}

return mDelegate.onBackButtonPressed();
}
}
5 changes: 3 additions & 2 deletions ...sistant/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class AssistantCoordinator {
AssistantCoordinator(ChromeActivity activity, BottomSheetController controller,
TabObscuringHandler tabObscuringHandler,
@Nullable AssistantOverlayCoordinator overlayCoordinator,
AssistantKeyboardCoordinator.Delegate keyboardCoordinatorDelegate) {
AssistantKeyboardCoordinator.Delegate keyboardCoordinatorDelegate,
AssistantBottomSheetContent.Delegate bottomSheetDelegate) {
mActivity = activity;

if (overlayCoordinator != null) {
Expand All @@ -46,7 +47,7 @@ class AssistantCoordinator {

mBottomBarCoordinator = new AssistantBottomBarCoordinator(activity, mModel, controller,
activity.getWindowAndroid().getApplicationBottomInsetProvider(),
tabObscuringHandler);
tabObscuringHandler, bottomSheetDelegate);
mKeyboardCoordinator = new AssistantKeyboardCoordinator(activity,
activity.getWindowAndroid().getKeyboardDelegate(),
activity.getCompositorViewHolder(), mModel, keyboardCoordinatorDelegate);
Expand Down
6 changes: 5 additions & 1 deletion ...va/src/org/chromium/chrome/browser/autofill_assistant/AssistantOnboardingCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ void show(Callback<Boolean> callback) {
mContext, mFullscreenManager, mCompositorViewHolder, mScrimView, overlayModel);
overlayModel.set(AssistantOverlayModel.STATE, AssistantOverlayState.FULL);

mContent = new AssistantBottomSheetContent(mContext);
mContent = new AssistantBottomSheetContent(mContext, () -> {
callback.onResult(/* accept= */ false);
hide();
return true;
});
initContent(callback);
BottomSheetUtils.showContentAndExpand(mController, mContent, mAnimate);
}
Expand Down
12 changes: 11 additions & 1 deletion ...ava/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private AutofillAssistantUiController(ChromeActivity activity, BottomSheetContro
mActivity = activity;
mCoordinator = new AssistantCoordinator(activity, controller, tabObscuringHandler,
onboardingCoordinator == null ? null : onboardingCoordinator.transferControls(),
this::safeNativeOnKeyboardVisibilityChanged);
this::safeNativeOnKeyboardVisibilityChanged, this::safeNativeOnBackButtonClicked);
mActivityTabObserver =
new ActivityTabProvider.ActivityTabTabObserver(activity.getActivityTabProvider()) {
@Override
Expand Down Expand Up @@ -413,6 +413,14 @@ private void safeNativeOnKeyboardVisibilityChanged(boolean visible) {
}
}

private boolean safeNativeOnBackButtonClicked() {
if (mNativeUiController != 0) {
return AutofillAssistantUiControllerJni.get().onBackButtonClicked(
mNativeUiController, AutofillAssistantUiController.this);
}
return false;
}

private void safeNativeSetVisible(boolean visible) {
if (mNativeUiController != 0) {
AutofillAssistantUiControllerJni.get().setVisible(
Expand All @@ -436,6 +444,8 @@ void onCloseButtonClicked(
long nativeUiControllerAndroid, AutofillAssistantUiController caller);
void onKeyboardVisibilityChanged(long nativeUiControllerAndroid,
AutofillAssistantUiController caller, boolean visible);
boolean onBackButtonClicked(
long nativeUiControllerAndroid, AutofillAssistantUiController caller);
void setVisible(long nativeUiControllerAndroid, AutofillAssistantUiController caller,
boolean visible);
}
Expand Down
89 changes: 85 additions & 4 deletions ...chromium/chrome/browser/autofill_assistant/AutofillAssistantChromeTabIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,21 @@
import static android.support.test.espresso.assertion.ViewAssertions.doesNotExist;
import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withClassName;
import static android.support.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static android.support.test.espresso.matcher.ViewMatchers.withText;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.tapElement;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntil;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilKeyboardMatchesCondition;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewAssertionTrue;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewMatchesCondition;

Expand All @@ -36,8 +42,13 @@

import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.chrome.autofill_assistant.R;
import org.chromium.chrome.browser.autofill_assistant.proto.ActionProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ChipProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto.Rectangle;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementReferenceProto;
import org.chromium.chrome.browser.autofill_assistant.proto.FocusElementProto;
import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto;
import org.chromium.chrome.browser.autofill_assistant.proto.SupportedScriptProto;
import org.chromium.chrome.browser.autofill_assistant.proto.SupportedScriptProto.PresentationProto;
Expand Down Expand Up @@ -306,11 +317,24 @@ public void startingNewAutofillAssistantCloseTabResumesRunOnPreviousTab() {

@Test
@MediumTest
public void backButtonTerminatesAutofillAssistant() {
public void backButtonDestroysAutofillAssistantUi() throws Exception {
ChromeTabUtils.loadUrlOnUiThread(
mTestRule.getActivity().getActivityTab(), getURL(TEST_PAGE_B));

ElementReferenceProto element = (ElementReferenceProto) ElementReferenceProto.newBuilder()
.addSelectors("#profile_name")
.build();

ArrayList<ActionProto> list = new ArrayList<>();
list.add(
(ActionProto) ActionProto.newBuilder()
.setFocusElement(FocusElementProto.newBuilder()
.setElement(element)
.setTouchableElementArea(
ElementAreaProto.newBuilder().addTouchable(
Rectangle.newBuilder().addElements(
element))))
.build());
list.add((ActionProto) ActionProto.newBuilder()
.setPrompt(PromptProto.newBuilder().setMessage("Prompt").addChoices(
PromptProto.Choice.newBuilder()))
Expand All @@ -328,18 +352,75 @@ public void backButtonTerminatesAutofillAssistant() {

waitUntilViewMatchesCondition(withText("Prompt"), isCompletelyDisplayed());

// First press on back button minimizes Autofill Assistant. Second click navigates back.
// Force the keyboard to open.
tapElement(mTestRule, "profile_name");
waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ true);

// First press on back button closes the keyboard.
Espresso.pressBack();
waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ false);
onView(withText("Prompt")).check(matches(isCompletelyDisplayed()));

// Second press on back button destroys Autofill Assistant UI.
Espresso.pressBack();
waitUntilViewMatchesCondition(withText(R.string.undo), isCompletelyDisplayed());
onView(withId(R.id.autofill_assistant)).check(doesNotExist());
assertThat(mTestRule.getActivity().getActivityTab().getUrl().getSpec(),
is(getURL(TEST_PAGE_B)));

// Third press on back button navigates back.
Espresso.pressBack();
waitUntil(()
-> mTestRule.getActivity().getActivityTab().getUrl().getSpec().equals(
getURL(TEST_PAGE_A)));
}

@Test
@MediumTest
public void backButtonIsIgnoredInBrowseMode() {
// Same domain, different page, such that navigating back is allowed in the BROWSE state.
ChromeTabUtils.loadUrlOnUiThread(
mTestRule.getActivity().getActivityTab(), getURL(TEST_PAGE_B));

ArrayList<ActionProto> list = new ArrayList<>();
list.add((ActionProto) ActionProto.newBuilder()
.setPrompt(PromptProto.newBuilder()
.setMessage("Prompt")
.setBrowseMode(true)
.addChoices(PromptProto.Choice.newBuilder()))
.build());

AutofillAssistantTestScript script = new AutofillAssistantTestScript(
(SupportedScriptProto) SupportedScriptProto.newBuilder()
.setPath(TEST_PAGE_A)
.setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
ChipProto.newBuilder().setText("Done")))
.build(),
list);
setupScripts(script);
startAutofillAssistantOnTab(TEST_PAGE_B);

// BROWSE state must not automatically collapse the UI.
waitUntilViewMatchesCondition(withText("Prompt"), isCompletelyDisplayed());

// First press on back button collapses Autofill Assistant UI.
Espresso.pressBack();
waitUntilViewMatchesCondition(
withId(R.id.status_message), allOf(withText("Prompt"), not(isDisplayed())));
onView(withId(R.id.autofill_assistant)).check(matches(isDisplayed()));

// Second press on back button navigates back, without removing the Autofill Assistannt UI.
Espresso.pressBack();
waitUntilViewMatchesCondition(withText(containsString("Sorry")), isCompletelyDisplayed());
waitUntil(()
-> mTestRule.getActivity().getActivityTab().getUrl().getSpec().equals(
getURL(TEST_PAGE_A)));
onView(withId(R.id.autofill_assistant)).check(matches(isDisplayed()));
onView(withId(R.id.status_message)).check(matches(withText("Prompt")));
}

@Test
@MediumTest
public void interactingWithLocationBarHidesAutofillAssistant() throws Exception {
public void interactingWithLocationBarHidesAutofillAssistant() {
ArrayList<ActionProto> list = new ArrayList<>();
list.add((ActionProto) ActionProto.newBuilder()
.setPrompt(PromptProto.newBuilder().setMessage("Prompt").addChoices(
Expand Down
8 changes: 6 additions & 2 deletions ...javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ public void testStartAndAccept() throws Exception {
()
-> new AssistantCoordinator(getActivity(), bottomSheetController,
getActivity().getTabObscuringHandler(),
/* overlayCoordinator= */ null, null));
/* overlayCoordinator= */ null,
/* keyboardCoordinatorDelegate= */ null,
/* bottomSheetDelegate= */ null));

// Bottom sheet is shown in the BottomSheet when creating the AssistantCoordinator.
ViewGroup bottomSheetContent =
Expand Down Expand Up @@ -251,7 +253,9 @@ public void testTooltipBubble() throws Exception {
()
-> new AssistantCoordinator(getActivity(), bottomSheetController,
getActivity().getTabObscuringHandler(),
/* overlayCoordinator= */ null, null));
/* overlayCoordinator= */ null,
/* keyboardCoordinatorDelegate= */ null,
/* bottomSheetDelegate= */ null));

// Bottom sheet is shown in the BottomSheet when creating the AssistantCoordinator.
ViewGroup bottomSheetContent =
Expand Down
19 changes: 19 additions & 0 deletions chrome/browser/android/autofill_assistant/ui_controller_android.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,25 @@ void UiControllerAndroid::OnKeyboardVisibilityChanged(
!visible);
}

bool UiControllerAndroid::OnBackButtonClicked(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller) {
// If the keyboard is currently shown, clicking the back button should
// hide the keyboard rather than close autofill assistant.
if (Java_AutofillAssistantUiController_isKeyboardShown(env, java_object_)) {
Java_AutofillAssistantUiController_hideKeyboard(env, java_object_);
return true;
}

// For BROWSE state the back button should react in its default way.
if (ui_delegate_->GetState() == AutofillAssistantState::BROWSE) {
return false;
}

CloseOrCancel(-1, TriggerContext::CreateEmpty());
return true;
}

void UiControllerAndroid::CloseOrCancel(
int action_index,
std::unique_ptr<TriggerContext> trigger_context) {
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/android/autofill_assistant/ui_controller_android.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ class UiControllerAndroid : public ControllerObserver {
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jboolean visible);
bool OnBackButtonClicked(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller);
void SetVisible(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jboolean visible);
Expand Down

0 comments on commit 65708d4

Please sign in to comment.