Commit 6d708617 authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Delay showing UI for lite scripts.

With this CL, STARTING and BROWSE controller states no longer require
the UI to be shown while running lite scripts. Effectively, this delays
the attachment of the UI until the first PROMPT is shown.

Note that this is specifically for the beginning of lite scripts. It is
not yet possible to dynamically detach the UI mid-flow - this CL is only
about the delayed initial attachment of the UI.

Bug: b/162065211
Change-Id: Ia91498e0570fbaff76dd81c65f3462de26c04425
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359070
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799188}
parent c5bcc248
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.autofill_assistant; package org.chromium.chrome.browser.autofill_assistant;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantArguments.PARAMETER_TRIGGER_SCRIPT_USED;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import org.chromium.base.Callback; import org.chromium.base.Callback;
...@@ -14,6 +16,7 @@ import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; ...@@ -14,6 +16,7 @@ import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map;
/** /**
* Configures and runs a lite script and returns the result to the caller. * Configures and runs a lite script and returns the result to the caller.
...@@ -35,14 +38,18 @@ class AutofillAssistantLiteScriptCoordinator { ...@@ -35,14 +38,18 @@ class AutofillAssistantLiteScriptCoordinator {
void startLiteScript(String firstTimeUserScriptPath, String returningUserScriptPath, void startLiteScript(String firstTimeUserScriptPath, String returningUserScriptPath,
Callback<Boolean> onFinishedCallback) { Callback<Boolean> onFinishedCallback) {
AutofillAssistantLiteService liteService = new AutofillAssistantLiteService(mWebContents, String usedScriptPath =
AutofillAssistantPreferencesUtil.isAutofillAssistantFirstTimeLiteScriptUser() AutofillAssistantPreferencesUtil.isAutofillAssistantFirstTimeLiteScriptUser()
? firstTimeUserScriptPath ? firstTimeUserScriptPath
: returningUserScriptPath, : returningUserScriptPath;
finishedState -> handleLiteScriptResult(finishedState, onFinishedCallback)); AutofillAssistantLiteService liteService =
new AutofillAssistantLiteService(mWebContents, usedScriptPath,
finishedState -> handleLiteScriptResult(finishedState, onFinishedCallback));
AutofillAssistantServiceInjector.setServiceToInject(liteService); AutofillAssistantServiceInjector.setServiceToInject(liteService);
Map<String, String> parameters = new HashMap<>();
parameters.put(PARAMETER_TRIGGER_SCRIPT_USED, usedScriptPath);
AutofillAssistantClient.fromWebContents(mWebContents) AutofillAssistantClient.fromWebContents(mWebContents)
.start(/* initialUrl= */ "", /* parameters= */ new HashMap<>(), .start(/* initialUrl= */ "", /* parameters= */ parameters,
/* experimentIds= */ "", /* callerAccount= */ "", /* userName= */ "", /* experimentIds= */ "", /* callerAccount= */ "", /* userName= */ "",
/* isChromeCustomTab= */ true, /* onboardingCoordinator= */ null); /* isChromeCustomTab= */ true, /* onboardingCoordinator= */ null);
AutofillAssistantServiceInjector.setServiceToInject(null); AutofillAssistantServiceInjector.setServiceToInject(null);
......
...@@ -49,12 +49,12 @@ const char kProgressBarExperiment[] = "4400697"; ...@@ -49,12 +49,12 @@ const char kProgressBarExperiment[] = "4400697";
// //
// Note that the UI might be shown in RUNNING state, even if it doesn't require // Note that the UI might be shown in RUNNING state, even if it doesn't require
// it. // it.
bool StateNeedsUI(AutofillAssistantState state) { bool StateNeedsUiInRegularScript(AutofillAssistantState state) {
switch (state) { switch (state) {
case AutofillAssistantState::STARTING:
case AutofillAssistantState::PROMPT: case AutofillAssistantState::PROMPT:
case AutofillAssistantState::AUTOSTART_FALLBACK_PROMPT: case AutofillAssistantState::AUTOSTART_FALLBACK_PROMPT:
case AutofillAssistantState::MODAL_DIALOG: case AutofillAssistantState::MODAL_DIALOG:
case AutofillAssistantState::STARTING:
case AutofillAssistantState::BROWSE: case AutofillAssistantState::BROWSE:
return true; return true;
...@@ -64,9 +64,25 @@ bool StateNeedsUI(AutofillAssistantState state) { ...@@ -64,9 +64,25 @@ bool StateNeedsUI(AutofillAssistantState state) {
case AutofillAssistantState::RUNNING: case AutofillAssistantState::RUNNING:
return false; return false;
} }
}
NOTREACHED(); // Same as |StateNeedsUiInRegularScript|, but does not show UI in STARTING or
return false; // BROWSE state.
bool StateNeedsUiInLiteScript(AutofillAssistantState state) {
switch (state) {
case AutofillAssistantState::PROMPT:
case AutofillAssistantState::AUTOSTART_FALLBACK_PROMPT:
case AutofillAssistantState::MODAL_DIALOG:
return true;
case AutofillAssistantState::STARTING:
case AutofillAssistantState::BROWSE:
case AutofillAssistantState::INACTIVE:
case AutofillAssistantState::TRACKING:
case AutofillAssistantState::STOPPED:
case AutofillAssistantState::RUNNING:
return false;
}
} }
// Check whether a domain is a subdomain of another domain. // Check whether a domain is a subdomain of another domain.
...@@ -1887,6 +1903,13 @@ void Controller::WriteUserData( ...@@ -1887,6 +1903,13 @@ void Controller::WriteUserData(
} }
} }
bool Controller::StateNeedsUI(AutofillAssistantState state) {
if (!trigger_context_ || !trigger_context_->is_lite_script()) {
return StateNeedsUiInRegularScript(state);
}
return StateNeedsUiInLiteScript(state);
}
ElementArea* Controller::touchable_element_area() { ElementArea* Controller::touchable_element_area() {
if (!touchable_element_area_) { if (!touchable_element_area_) {
touchable_element_area_ = std::make_unique<ElementArea>(this); touchable_element_area_ = std::make_unique<ElementArea>(this);
......
...@@ -335,6 +335,8 @@ class Controller : public ScriptExecutorDelegate, ...@@ -335,6 +335,8 @@ class Controller : public ScriptExecutorDelegate,
void RecordDropOutOrShutdown(Metrics::DropOutReason reason); void RecordDropOutOrShutdown(Metrics::DropOutReason reason);
void PerformDelayedShutdownIfNecessary(); void PerformDelayedShutdownIfNecessary();
bool StateNeedsUI(AutofillAssistantState state);
ClientSettings settings_; ClientSettings settings_;
Client* const client_; Client* const client_;
const base::TickClock* const tick_clock_; const base::TickClock* const tick_clock_;
......
...@@ -16,6 +16,10 @@ const char kOverlayColorParameterName[] = "OVERLAY_COLORS"; ...@@ -16,6 +16,10 @@ const char kOverlayColorParameterName[] = "OVERLAY_COLORS";
// TODO(b/151401974): Eliminate duplicate parameter definitions. // TODO(b/151401974): Eliminate duplicate parameter definitions.
const char kPasswordChangeUsernameParameterName[] = "PASSWORD_CHANGE_USERNAME"; const char kPasswordChangeUsernameParameterName[] = "PASSWORD_CHANGE_USERNAME";
// Parameter that contains the path of the lite script currently being run, if
// any.
const char kLiteScriptPathParamaterName[] = "TRIGGER_SCRIPT_USED";
// static // static
std::unique_ptr<TriggerContext> TriggerContext::CreateEmpty() { std::unique_ptr<TriggerContext> TriggerContext::CreateEmpty() {
return std::make_unique<TriggerContextImpl>(); return std::make_unique<TriggerContextImpl>();
...@@ -34,6 +38,10 @@ std::unique_ptr<TriggerContext> TriggerContext::Merge( ...@@ -34,6 +38,10 @@ std::unique_ptr<TriggerContext> TriggerContext::Merge(
return std::make_unique<MergedTriggerContext>(contexts); return std::make_unique<MergedTriggerContext>(contexts);
} }
bool TriggerContext::is_lite_script() const {
return GetParameter(kLiteScriptPathParamaterName).has_value();
}
TriggerContext::TriggerContext() {} TriggerContext::TriggerContext() {}
TriggerContext::~TriggerContext() {} TriggerContext::~TriggerContext() {}
......
...@@ -40,6 +40,9 @@ class TriggerContext { ...@@ -40,6 +40,9 @@ class TriggerContext {
// Returns all parameters as a map. // Returns all parameters as a map.
virtual std::map<std::string, std::string> GetParameters() const = 0; virtual std::map<std::string, std::string> GetParameters() const = 0;
// Returns true if this is a lite script run.
bool is_lite_script() const;
// Returns the value of a specific parameter, if present. // Returns the value of a specific parameter, if present.
virtual base::Optional<std::string> GetParameter( virtual base::Optional<std::string> GetParameter(
const std::string& name) const = 0; const std::string& name) const = 0;
...@@ -108,9 +111,7 @@ class TriggerContextImpl : public TriggerContext { ...@@ -108,9 +111,7 @@ class TriggerContextImpl : public TriggerContext {
std::string experiment_ids_; std::string experiment_ids_;
bool cct_ = false; bool cct_ = false;
bool direct_action_ = false; bool direct_action_ = false;
bool onboarding_shown_ = false; bool onboarding_shown_ = false;
std::string caller_account_hash_ = ""; std::string caller_account_hash_ = "";
......
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