Commit 44e2776d authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Don't show UI after moving tabs when feature is off

Before this change with tab-switching turned off if a script was
running when the user chose "Open in Chrome" from CCT, the UI would
move from CCT to a normal tab, even with the tab-switching feature
turned off.

What should instead have happened is AA shutting down. However, AA
takes a while to shut down when a script is running,

This patch prevents the UI from showing again on the new tab, after it's
been taken out of CCT. It also forces prompt and PR actions to terminate
immediately.

Bug: b/128666286
Change-Id: Ie89f7062e480322b45132ff44ee3f8b1cdb1b9c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524359Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641838}
parent 1fd75367
......@@ -83,7 +83,12 @@ class ActionDelegate {
// scripts, even though we're in the middle of a script. This includes
// allowing access to the touchable elements set previously, in the same
// script.
virtual void Prompt(std::unique_ptr<std::vector<Chip>> chips) = 0;
//
// |on_terminate| is called if the prompt is terminated, by Autofill Assistant
// shutting down. The action should return immediately with client error
// USER_ABORTED_ACTION.
virtual void Prompt(std::unique_ptr<std::vector<Chip>> chips,
base::OnceCallback<void()> on_terminate) = 0;
// Remove all chips from the UI.
virtual void CancelPrompt() = 0;
......
......@@ -54,7 +54,9 @@ class MockActionDelegate : public ActionDelegate {
void(const Selector& selector,
base::OnceCallback<void(bool)> callback));
MOCK_METHOD1(Prompt, void(std::unique_ptr<std::vector<Chip>> chips));
MOCK_METHOD2(Prompt,
void(std::unique_ptr<std::vector<Chip>> chips,
base::OnceCallback<void()> on_terminate));
MOCK_METHOD0(CancelPrompt, void());
void FillAddressForm(const autofill::AutofillProfile* profile,
......
......@@ -130,7 +130,9 @@ void PromptAction::UpdateChips() {
weak_ptr_factory_.GetWeakPtr(), i);
}
SetDefaultChipType(chips.get());
delegate_->Prompt(std::move(chips));
delegate_->Prompt(std::move(chips),
base::BindOnce(&PromptAction::OnTerminated,
weak_ptr_factory_.GetWeakPtr()));
precondition_changed_ = false;
}
......@@ -194,4 +196,14 @@ void PromptAction::OnSuggestionChosen(int choice_index) {
proto_.prompt().choices(choice_index);
std::move(callback_).Run(std::move(processed_action_proto_));
}
void PromptAction::OnTerminated() {
if (!callback_) {
NOTREACHED();
return;
}
UpdateProcessedAction(USER_ABORTED_ACTION);
std::move(callback_).Run(std::move(processed_action_proto_));
}
} // namespace autofill_assistant
......@@ -40,6 +40,7 @@ class PromptAction : public Action {
void OnAutoSelectElementExists(int choice_index, bool exists);
void OnAutoSelectDone();
void OnSuggestionChosen(int choice_index);
void OnTerminated();
ProcessActionCallback callback_;
ActionDelegate* delegate_;
......
......@@ -7,6 +7,7 @@
#include <utility>
#include "base/bind.h"
#include "base/callback.h"
#include "base/test/mock_callback.h"
#include "base/test/scoped_task_environment.h"
#include "components/autofill_assistant/browser/actions/mock_action_delegate.h"
......@@ -43,9 +44,11 @@ class PromptActionTest : public testing::Test {
return std::make_unique<BatchElementChecker>(&mock_web_controller_);
}));
ON_CALL(mock_action_delegate_, Prompt(_))
.WillByDefault(Invoke([this](std::unique_ptr<std::vector<Chip>> chips) {
ON_CALL(mock_action_delegate_, Prompt(_, _))
.WillByDefault(Invoke([this](std::unique_ptr<std::vector<Chip>> chips,
base::OnceCallback<void()> on_terminate) {
chips_ = std::move(chips);
on_terminate_ = std::move(on_terminate);
}));
prompt_proto_ = proto_.mutable_prompt();
}
......@@ -61,6 +64,7 @@ class PromptActionTest : public testing::Test {
ActionProto proto_;
PromptProto* prompt_proto_;
std::unique_ptr<std::vector<Chip>> chips_;
base::OnceCallback<void()> on_terminate_;
};
TEST_F(PromptActionTest, ChoicesMissing) {
......@@ -179,5 +183,19 @@ TEST_F(PromptActionTest, AutoSelectWithButton) {
task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1));
}
TEST_F(PromptActionTest, Terminate) {
auto* ok_proto = prompt_proto_->add_choices();
ok_proto->set_name("Ok");
ok_proto->set_chip_type(HIGHLIGHTED_ACTION);
ok_proto->set_server_payload("ok");
PromptAction action(proto_);
action.ProcessAction(&mock_action_delegate_, callback_.Get());
EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status,
USER_ABORTED_ACTION))));
std::move(on_terminate_).Run();
}
} // namespace
} // namespace autofill_assistant
......@@ -14,6 +14,7 @@
#include "base/task/post_task.h"
#include "base/time/tick_clock.h"
#include "base/values.h"
#include "components/autofill_assistant/browser/features.h"
#include "components/autofill_assistant/browser/metrics.h"
#include "components/autofill_assistant/browser/protocol_utils.h"
#include "components/autofill_assistant/browser/ui_controller.h"
......@@ -857,7 +858,10 @@ void Controller::RenderProcessGone(base::TerminationStatus status) {
void Controller::OnWebContentsFocused(
content::RenderWidgetHost* render_widget_host) {
if (NeedsUI()) {
if (NeedsUI() &&
base::FeatureList::IsEnabled(features::kAutofillAssistantChromeEntry)) {
// Show UI again when re-focused in case the web contents moved activity.
// This is only enabled when tab-switching is enabled.
client_->ShowUI();
}
}
......@@ -887,6 +891,24 @@ void Controller::SetPaymentRequestOptions(
GetUiController()->OnPaymentRequestChanged(payment_request_options_.get());
}
void Controller::CancelPaymentRequest() {
payment_request_info_.reset();
if (!payment_request_options_)
return;
auto callback = std::move(payment_request_options_->callback);
SetPaymentRequestOptions(nullptr);
if (!callback) {
NOTREACHED();
return;
}
auto result = std::make_unique<PaymentInformation>();
result->succeed = false;
std::move(callback).Run(std::move(result));
}
ElementArea* Controller::touchable_element_area() {
if (!touchable_element_area_) {
touchable_element_area_ = std::make_unique<ElementArea>(this);
......
......@@ -100,6 +100,7 @@ class Controller : public ScriptExecutorDelegate,
bool IsCookieExperimentEnabled() const;
void SetPaymentRequestOptions(
std::unique_ptr<PaymentRequestOptions> options) override;
void CancelPaymentRequest() override;
// Overrides autofill_assistant::UiDelegate:
AutofillAssistantState GetState() override;
......
......@@ -8,6 +8,8 @@
#include <utility>
#include "base/test/mock_callback.h"
#include "base/test/scoped_feature_list.h"
#include "components/autofill_assistant/browser/features.h"
#include "components/autofill_assistant/browser/mock_run_once_callback.h"
#include "components/autofill_assistant/browser/mock_service.h"
#include "components/autofill_assistant/browser/mock_ui_controller.h"
......@@ -81,6 +83,8 @@ class ControllerTest : public testing::Test {
~ControllerTest() override {}
void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(
features::kAutofillAssistantChromeEntry);
auto web_controller = std::make_unique<NiceMock<MockWebController>>();
mock_web_controller_ = web_controller.get();
auto service = std::make_unique<NiceMock<MockService>>();
......@@ -182,6 +186,7 @@ class ControllerTest : public testing::Test {
// |thread_bundle_| must be the first field, to make sure that everything runs
// in the same task environment.
content::TestBrowserThreadBundle thread_bundle_;
base::test::ScopedFeatureList scoped_feature_list_;
content::TestBrowserContext browser_context_;
std::unique_ptr<content::WebContents> web_contents_;
base::TimeTicks now_;
......
......@@ -85,4 +85,7 @@ void FakeScriptExecutorDelegate::SetPaymentRequestOptions(
std::unique_ptr<PaymentRequestOptions> options) {
payment_request_options_ = std::move(options);
}
void FakeScriptExecutorDelegate::CancelPaymentRequest() {}
} // namespace autofill_assistant
......@@ -42,6 +42,7 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate {
void SetChips(std::unique_ptr<std::vector<Chip>> chips) override;
void SetPaymentRequestOptions(
std::unique_ptr<PaymentRequestOptions> options) override;
void CancelPaymentRequest() override;
void SetCurrentURL(const GURL& url) { current_url_ = url; }
......
......@@ -249,7 +249,8 @@ void ScriptExecutor::OnGetFullCard(GetFullCardCallback callback,
std::move(callback).Run(std::move(card), cvc);
}
void ScriptExecutor::Prompt(std::unique_ptr<std::vector<Chip>> chips) {
void ScriptExecutor::Prompt(std::unique_ptr<std::vector<Chip>> chips,
base::OnceCallback<void()> on_terminate) {
if (touchable_element_area_) {
// SetChips reproduces the end-of-script appearance and behavior during
// script execution. This includes allowing access to touchable elements,
......@@ -275,9 +276,14 @@ void ScriptExecutor::Prompt(std::unique_ptr<std::vector<Chip>> chips) {
delegate_->EnterState(AutofillAssistantState::PROMPT);
delegate_->SetChips(std::move(chips));
on_terminate_prompt_ = std::move(on_terminate);
}
void ScriptExecutor::CancelPrompt() {
// Delete on_terminate_prompt_ if necessary, without running.
if (on_terminate_prompt_)
std::move(on_terminate_prompt_);
delegate_->SetChips(nullptr);
CleanUpAfterPrompt();
}
......@@ -390,6 +396,16 @@ void ScriptExecutor::Terminate() {
wait_with_interrupts_->Terminate();
at_end_ = TERMINATE;
should_stop_script_ = true;
// Force PR and other prompt-based actions to end.
//
// TODO(b/128300038): get rid of this special case. Instead, delete actions
// without waiting for them to return.
delegate_->CancelPaymentRequest();
if (on_terminate_prompt_) {
std::move(on_terminate_prompt_).Run();
CancelPrompt();
}
}
void ScriptExecutor::Close() {
......
......@@ -117,7 +117,8 @@ class ScriptExecutor : public ActionDelegate {
void GetPaymentInformation(
std::unique_ptr<PaymentRequestOptions> options) override;
void GetFullCard(GetFullCardCallback callback) override;
void Prompt(std::unique_ptr<std::vector<Chip>> chips) override;
void Prompt(std::unique_ptr<std::vector<Chip>> chips,
base::OnceCallback<void()> on_terminate) override;
void CancelPrompt() override;
void FillAddressForm(const autofill::AutofillProfile* profile,
const Selector& selector,
......@@ -311,6 +312,10 @@ class ScriptExecutor : public ActionDelegate {
std::unique_ptr<WaitWithInterrupts> wait_with_interrupts_;
// Callback set by Prompt(). This is called when the prompt is terminated
// without selecting any chips. nullptr unless showing a prompt.
base::OnceCallback<void()> on_terminate_prompt_;
base::WeakPtrFactory<ScriptExecutor> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ScriptExecutor);
};
......
......@@ -54,6 +54,7 @@ class ScriptExecutorDelegate {
virtual void ClearInfoBox() = 0;
virtual void SetPaymentRequestOptions(
std::unique_ptr<PaymentRequestOptions> options) = 0;
virtual void CancelPaymentRequest() = 0;
virtual void SetProgress(int progress) = 0;
virtual void SetProgressVisible(bool visible) = 0;
virtual void SetChips(std::unique_ptr<std::vector<Chip>> chips) = 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