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

[Autofill Assistant] Slim down AutofillAssistantUiController.

This patch take out initialization and delayed shutdown from
AutofillAssistantUiController:

 * initialization goes into a new class AutofillAssistantFacade, called
   from CustomTabActivity.

 * UiDelegateHolder becomes a toplevel class, responsible for delayed
   shutdown and UI pausing

The following responsibilities remain in AutofillAssistantUiController:

 * bridging Java -> C++ and C++ -> Java
 * account management and authentication

This patch also unifies the way the "give up" message is displayed in
Java, removes the unnecessary C++ OnGiveUp method and always hides the
overlay and the event filter when switching to graceful shutdown mode.

Bug: 806868
Change-Id: Idef806520f5e6e8a6dda4fc843d61bf77c1eafb4
Reviewed-on: https://chromium-review.googlesource.com/c/1333652
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarEgor Pasko <pasko@chromium.org>
Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608007}
parent 5181baf9
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.autofill_assistant;
import android.os.Bundle;
import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
import org.chromium.chrome.browser.preferences.autofill_assistant.AutofillAssistantPreferences;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModel.TabSelectionType;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
/** Facade for starting Autofill Assistant on a custom tab. */
public class AutofillAssistantFacade {
private static final String RFC_3339_FORMAT_WITHOUT_TIMEZONE = "yyyy'-'MM'-'dd'T'HH':'mm':'ss";
/** Prefix for Intent extras relevant to this feature. */
private static final String INTENT_EXTRA_PREFIX =
"org.chromium.chrome.browser.autofill_assistant.";
/** Autofill Assistant Study name. */
private static final String STUDY_NAME = "AutofillAssistant";
/** Variation url parameter name. */
private static final String URL_PARAMETER_NAME = "url";
/** Special parameter that enables the feature. */
private static final String PARAMETER_ENABLED = "ENABLED";
/** Returns true if all conditions are satisfied to start Autofill Assistant. */
public static boolean isConfigured(Bundle intentExtras) {
return getBooleanParameter(intentExtras, PARAMETER_ENABLED)
&& !AutofillAssistantStudy.getUrl().isEmpty()
&& ContextUtils.getAppSharedPreferences().getBoolean(
AutofillAssistantPreferences.PREF_AUTOFILL_ASSISTANT_SWITCH, true);
}
/** Starts Autofill Assistant on the given {@code activity}. */
public static void start(CustomTabActivity activity) {
Map<String, String> parameters = extractParameters(activity.getInitialIntent().getExtras());
parameters.remove(PARAMETER_ENABLED);
AutofillAssistantUiController controller =
new AutofillAssistantUiController(activity, parameters);
AutofillAssistantUiDelegate uiDelegate =
new AutofillAssistantUiDelegate(activity, controller);
UiDelegateHolder delegateHolder = new UiDelegateHolder(controller, uiDelegate);
controller.setUiDelegateHolder(delegateHolder);
initTabObservers(activity, delegateHolder);
AutofillAssistantUiDelegate.Details initialDetails = makeDetailsFromParameters(parameters);
controller.maybeUpdateDetails(initialDetails);
uiDelegate.startOrSkipInitScreen();
}
private static void initTabObservers(
CustomTabActivity activity, UiDelegateHolder delegateHolder) {
// Shut down Autofill Assistant when the tab is detached from the activity.
Tab activityTab = activity.getActivityTab();
activityTab.addObserver(new EmptyTabObserver() {
@Override
public void onActivityAttachmentChanged(Tab tab, boolean isAttached) {
if (!isAttached) {
activityTab.removeObserver(this);
delegateHolder.shutdown();
}
}
});
// Shut down Autofill Assistant when the selected tab (foreground tab) is changed.
TabModel currentTabModel = activity.getTabModelSelector().getCurrentModel();
currentTabModel.addObserver(new EmptyTabModelObserver() {
@Override
public void didSelectTab(Tab tab, @TabSelectionType int type, int lastId) {
currentTabModel.removeObserver(this);
delegateHolder.giveUp();
}
});
}
/** Return the value if the given boolean parameter from the extras. */
private static boolean getBooleanParameter(Bundle extras, String parameterName) {
return extras.getBoolean(INTENT_EXTRA_PREFIX + parameterName, false);
}
/** Returns a map containing the extras starting with {@link #INTENT_EXTRA_PREFIX}. */
private static Map<String, String> extractParameters(Bundle extras) {
Map<String, String> result = new HashMap<>();
for (String key : extras.keySet()) {
if (key.startsWith(INTENT_EXTRA_PREFIX)) {
result.put(key.substring(INTENT_EXTRA_PREFIX.length()), extras.get(key).toString());
}
}
return result;
}
// TODO(crbug.com/806868): Create a fallback when there are no parameters for details.
// TODO(crbug.com/806868): Create a fallback when there are no parameters for details.
private static AutofillAssistantUiDelegate.Details makeDetailsFromParameters(
Map<String, String> parameters) {
String title = "";
String description = "";
Date date = null;
for (String key : parameters.keySet()) {
if (key.contains("E_NAME")) {
title = parameters.get(key);
continue;
}
if (key.contains("R_NAME")) {
description = parameters.get(key);
continue;
}
if (key.contains("DATETIME")) {
try {
// The parameter contains the timezone shift from the current location, that we
// don't care about.
date = new SimpleDateFormat(RFC_3339_FORMAT_WITHOUT_TIMEZONE, Locale.ROOT)
.parse(parameters.get(key));
} catch (ParseException e) {
// Ignore.
}
}
}
return new AutofillAssistantUiDelegate.Details(
title, /* url= */ "", date, description, /* isFinal= */ false);
}
}
...@@ -523,6 +523,8 @@ class AutofillAssistantUiDelegate { ...@@ -523,6 +523,8 @@ class AutofillAssistantUiDelegate {
// hacks to give enough space to display long messages. // hacks to give enough space to display long messages.
mStatusMessageView.setMaxLines(4); mStatusMessageView.setMaxLines(4);
mBottomBar.findViewById(R.id.feedback_button).setVisibility(View.GONE); mBottomBar.findViewById(R.id.feedback_button).setVisibility(View.GONE);
hideOverlay();
mTouchEventFilter.setVisibility(View.GONE);
hideProgressBar(); hideProgressBar();
hideDetails(); hideDetails();
mBottomBarAnimations.hideCarousel(); mBottomBarAnimations.hideCarousel();
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.autofill_assistant;
import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import java.util.ArrayDeque;
import java.util.Queue;
/**
* 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).
*/
class UiDelegateHolder {
/** Display the final message for that long before shutting everything down. */
private static final long GRACEFUL_SHUTDOWN_DELAY_MS = 5_000;
private final AutofillAssistantUiController mUiController;
private final AutofillAssistantUiDelegate mUiDelegate;
private boolean mPaused;
private boolean mHasBeenShutdown;
private boolean mIsShuttingDown;
private SnackbarManager.SnackbarController mDismissSnackbar;
private final Queue<Callback<AutofillAssistantUiDelegate>> mPendingUiOperations =
new ArrayDeque<>();
UiDelegateHolder(
AutofillAssistantUiController controller, AutofillAssistantUiDelegate uiDelegate) {
mUiController = controller;
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.
*/
void performUiOperation(Callback<AutofillAssistantUiDelegate> operation) {
if (mHasBeenShutdown || mIsShuttingDown) {
return;
}
if (mPaused) {
mPendingUiOperations.add(operation);
return;
}
operation.onResult(mUiDelegate);
}
/**
* Handles the dismiss operation.
*
* In normal mode, hides the UI, pauses UI operations and, unless undone within the time
* delay, eventually destroy everything. In graceful shutdown mode, shutdown immediately.
*/
void dismiss(int stringResourceId, Object... formatArgs) {
assert !mHasBeenShutdown;
if (mIsShuttingDown) {
shutdown();
return;
}
if (mDismissSnackbar != null) {
// Remove duplicate calls.
return;
}
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, stringResourceId, formatArgs);
}
/** Displays the give up message and enter graceful shutdown mode. */
void giveUp() {
performUiOperation(uiDelegate -> uiDelegate.showGiveUpMessage());
enterGracefulShutdownMode();
}
/** Enters graceful shutdown mode once we can again perform UI operations. */
void enterGracefulShutdownMode() {
performUiOperation(uiDelegate -> {
mIsShuttingDown = true;
mPendingUiOperations.clear();
uiDelegate.enterGracefulShutdownMode();
ThreadUtils.postOnUiThreadDelayed(this ::shutdown, GRACEFUL_SHUTDOWN_DELAY_MS);
});
}
/**
* Hides the UI and destroys everything.
*
* <p>Shutdown is final: After this call from the C++ side, as it's been deleted and no UI
* operation can run.
*/
void shutdown() {
if (mHasBeenShutdown) {
return;
}
mHasBeenShutdown = true;
mPendingUiOperations.clear();
if (mDismissSnackbar != null) {
mUiDelegate.dismissSnackbar(mDismissSnackbar);
}
mUiDelegate.hide();
mUiController.unsafeDestroy();
}
/**
* Pause all UI operations such that they can potentially be ran later using {@link
* #unpauseUiOperations()}.
*/
private void pauseUiOperations() {
mPaused = true;
}
/**
* Unpause and trigger all UI operations received by {@link #performUiOperation(Callback)}
* since the last {@link #pauseUiOperations()}.
*/
private void unpauseUiOperations() {
mPaused = false;
while (!mPendingUiOperations.isEmpty()) {
mPendingUiOperations.remove().onResult(mUiDelegate);
}
}
}
...@@ -63,7 +63,7 @@ import org.chromium.chrome.browser.ServiceTabLauncher; ...@@ -63,7 +63,7 @@ import org.chromium.chrome.browser.ServiceTabLauncher;
import org.chromium.chrome.browser.WarmupManager; import org.chromium.chrome.browser.WarmupManager;
import org.chromium.chrome.browser.WebContentsFactory; import org.chromium.chrome.browser.WebContentsFactory;
import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate; import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate;
import org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiController; import org.chromium.chrome.browser.autofill_assistant.AutofillAssistantFacade;
import org.chromium.chrome.browser.browserservices.BrowserSessionContentHandler; import org.chromium.chrome.browser.browserservices.BrowserSessionContentHandler;
import org.chromium.chrome.browser.browserservices.BrowserSessionContentUtils; import org.chromium.chrome.browser.browserservices.BrowserSessionContentUtils;
import org.chromium.chrome.browser.browserservices.PostMessageHandler; import org.chromium.chrome.browser.browserservices.PostMessageHandler;
...@@ -189,8 +189,6 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent ...@@ -189,8 +189,6 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent
private WebappCustomTabTimeSpentLogger mWebappTimeSpentLogger; private WebappCustomTabTimeSpentLogger mWebappTimeSpentLogger;
private AutofillAssistantUiController mAutofillAssistantUiController;
@Nullable @Nullable
private ModuleEntryPoint mModuleEntryPoint; private ModuleEntryPoint mModuleEntryPoint;
@Nullable @Nullable
...@@ -700,10 +698,9 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent ...@@ -700,10 +698,9 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent
getActivityTab().getWebContents()); getActivityTab().getWebContents());
} }
if (mAutofillAssistantUiController == null if (ChromeFeatureList.isEnabled(ChromeFeatureList.AUTOFILL_ASSISTANT)
&& ChromeFeatureList.isEnabled(ChromeFeatureList.AUTOFILL_ASSISTANT) && AutofillAssistantFacade.isConfigured(getInitialIntent().getExtras())) {
&& AutofillAssistantUiController.isConfigured(getInitialIntent().getExtras())) { AutofillAssistantFacade.start(this);
mAutofillAssistantUiController = new AutofillAssistantUiController(this);
} }
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && useSeparateTask()) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && useSeparateTask()) {
......
...@@ -120,12 +120,14 @@ chrome_java_sources = [ ...@@ -120,12 +120,14 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetCoordinator.java", "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetCoordinator.java",
"java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java", "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AnimatedProgressBar.java", "java/src/org/chromium/chrome/browser/autofill_assistant/AnimatedProgressBar.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantFacade.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantPaymentRequest.java", "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantPaymentRequest.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantStudy.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java", "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiDelegate.java", "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiDelegate.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantStudy.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/FeedbackContext.java", "java/src/org/chromium/chrome/browser/autofill_assistant/FeedbackContext.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/InitScreenController.java", "java/src/org/chromium/chrome/browser/autofill_assistant/InitScreenController.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/UiDelegateHolder.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/ui/BottomBarAnimations.java", "java/src/org/chromium/chrome/browser/autofill_assistant/ui/BottomBarAnimations.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/ui/PaymentRequestBottomBar.java", "java/src/org/chromium/chrome/browser/autofill_assistant/ui/PaymentRequestBottomBar.java",
"java/src/org/chromium/chrome/browser/autofill_assistant/ui/PaymentRequestUI.java", "java/src/org/chromium/chrome/browser/autofill_assistant/ui/PaymentRequestUI.java",
......
...@@ -404,12 +404,6 @@ void UiControllerAndroid::Destroy(JNIEnv* env, ...@@ -404,12 +404,6 @@ void UiControllerAndroid::Destroy(JNIEnv* env,
ui_delegate_->OnDestroy(); ui_delegate_->OnDestroy();
} }
void UiControllerAndroid::GiveUp(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller) {
ui_delegate_->OnGiveUp();
}
static jlong JNI_AutofillAssistantUiController_Init( static jlong JNI_AutofillAssistantUiController_Init(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& jcaller, const JavaParamRef<jobject>& jcaller,
......
...@@ -77,7 +77,6 @@ class UiControllerAndroid : public UiController, ...@@ -77,7 +77,6 @@ class UiControllerAndroid : public UiController,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& initialUrlString); const base::android::JavaParamRef<jstring>& initialUrlString);
void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj); void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
void GiveUp(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
void OnScriptSelected( void OnScriptSelected(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
......
...@@ -274,6 +274,12 @@ void Controller::OnScriptExecuted(const std::string& script_path, ...@@ -274,6 +274,12 @@ void Controller::OnScriptExecuted(const std::string& script_path,
GetOrCheckScripts(web_contents()->GetLastCommittedURL()); GetOrCheckScripts(web_contents()->GetLastCommittedURL());
} }
void Controller::GiveUp() {
GetUiController()->ShowStatusMessage(
l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP));
GetUiController()->ShutdownGracefully();
}
void Controller::OnClickOverlay() { void Controller::OnClickOverlay() {
GetUiController()->HideOverlay(); GetUiController()->HideOverlay();
// TODO(crbug.com/806868): Stop executing scripts. // TODO(crbug.com/806868): Stop executing scripts.
...@@ -319,12 +325,6 @@ void Controller::OnDestroy() { ...@@ -319,12 +325,6 @@ void Controller::OnDestroy() {
delete this; delete this;
} }
void Controller::OnGiveUp() {
GetUiController()->ShowStatusMessage(
l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP));
GetUiController()->ShutdownGracefully();
}
void Controller::DidAttachInterstitialPage() { void Controller::DidAttachInterstitialPage() {
GetUiController()->Shutdown(); GetUiController()->Shutdown();
} }
...@@ -360,9 +360,7 @@ void Controller::OnNoRunnableScriptsAnymore() { ...@@ -360,9 +360,7 @@ void Controller::OnNoRunnableScriptsAnymore() {
// We're navigated to a page that has no scripts or the scripts have reached a // We're navigated to a page that has no scripts or the scripts have reached a
// state from which they cannot recover through a DOM change. // state from which they cannot recover through a DOM change.
GetUiController()->ShowStatusMessage( GiveUp();
l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP));
GetUiController()->ShutdownGracefully();
return; return;
} }
...@@ -450,7 +448,7 @@ void Controller::DidStartNavigation( ...@@ -450,7 +448,7 @@ void Controller::DidStartNavigation(
!script_tracker_->running() && !navigation_handle->WasServerRedirect() && !script_tracker_->running() && !navigation_handle->WasServerRedirect() &&
!navigation_handle->IsSameDocument() && !navigation_handle->IsSameDocument() &&
!navigation_handle->IsRendererInitiated()) { !navigation_handle->IsRendererInitiated()) {
OnGiveUp(); GiveUp();
return; return;
} }
} }
......
...@@ -80,12 +80,12 @@ class Controller : public ScriptExecutorDelegate, ...@@ -80,12 +80,12 @@ class Controller : public ScriptExecutorDelegate,
void StartPeriodicScriptChecks(); void StartPeriodicScriptChecks();
void StopPeriodicScriptChecks(); void StopPeriodicScriptChecks();
void OnPeriodicScriptCheck(); void OnPeriodicScriptCheck();
void GiveUp();
// Overrides content::UiDelegate: // Overrides content::UiDelegate:
void Start(const GURL& initialUrl) override; void Start(const GURL& initialUrl) override;
void OnClickOverlay() override; void OnClickOverlay() override;
void OnDestroy() override; void OnDestroy() override;
void OnGiveUp() override;
void OnScriptSelected(const std::string& script_path) override; void OnScriptSelected(const std::string& script_path) override;
std::string GetDebugContext() override; std::string GetDebugContext() override;
......
...@@ -23,11 +23,6 @@ class UiDelegate { ...@@ -23,11 +23,6 @@ class UiDelegate {
// detached from the associated activity. // detached from the associated activity.
virtual void OnDestroy() = 0; virtual void OnDestroy() = 0;
// Called when the Autofill Assistant should display a message saying that
// it's shutting down and then shut down, because something happened that
// scripts cannot handle, such as a new window being opened.
virtual void OnGiveUp() = 0;
// Called when a script was selected for execution. // Called when a script was selected for execution.
virtual void OnScriptSelected(const std::string& script_path) = 0; virtual void OnScriptSelected(const std::string& script_path) = 0;
......
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