Commit 603f811c authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Allow ShowGenericUi to break on navigation.

Also adds support for interrupts (currently untested). Interrupts in
general appear to have very poor coverage. I created b/161652848 to
track this independently.

Bug: b/145043394
Change-Id: I48a4cdaf9a0bf45b0e206f8bf2787a13122c45d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306037Reviewed-by: default avatarLuca Hunkeler <hluca@google.com>
Reviewed-by: default avatarSandro Maggi <sandromaggi@google.com>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#790357}
parent 4a0b5e1b
...@@ -118,8 +118,15 @@ void ShowGenericUiAction::InternalProcessAction( ...@@ -118,8 +118,15 @@ void ShowGenericUiAction::InternalProcessAction(
} }
} }
base::OnceCallback<void()> end_on_navigation_callback;
if (proto_.show_generic_ui().end_on_navigation()) {
end_on_navigation_callback =
base::BindOnce(&ShowGenericUiAction::OnNavigationEnded,
weak_ptr_factory_.GetWeakPtr());
}
delegate_->Prompt(/* user_actions = */ nullptr, delegate_->Prompt(/* user_actions = */ nullptr,
/* disable_force_expand_sheet = */ false); /* disable_force_expand_sheet = */ false,
std::move(end_on_navigation_callback));
delegate_->SetGenericUi( delegate_->SetGenericUi(
std::make_unique<GenericUserInterfaceProto>( std::make_unique<GenericUserInterfaceProto>(
proto_.show_generic_ui().generic_user_interface()), proto_.show_generic_ui().generic_user_interface()),
...@@ -166,12 +173,13 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) { ...@@ -166,12 +173,13 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) {
preconditions_.emplace_back(std::make_unique<ElementPrecondition>( preconditions_.emplace_back(std::make_unique<ElementPrecondition>(
element_check.element_condition())); element_check.element_condition()));
} }
if (std::any_of( if (proto_.show_generic_ui().allow_interrupt() ||
std::any_of(
preconditions_.begin(), preconditions_.end(), preconditions_.begin(), preconditions_.end(),
[&](const auto& precondition) { return !precondition->empty(); })) { [&](const auto& precondition) { return !precondition->empty(); })) {
has_pending_wait_for_dom_ = true; has_pending_wait_for_dom_ = true;
delegate_->WaitForDom( delegate_->WaitForDom(
base::TimeDelta::Max(), false, base::TimeDelta::Max(), proto_.show_generic_ui().allow_interrupt(),
base::BindRepeating(&ShowGenericUiAction::RegisterChecks, base::BindRepeating(&ShowGenericUiAction::RegisterChecks,
weak_ptr_factory_.GetWeakPtr()), weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&ShowGenericUiAction::OnDoneWaitForDom, base::BindOnce(&ShowGenericUiAction::OnDoneWaitForDom,
...@@ -179,6 +187,12 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) { ...@@ -179,6 +187,12 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) {
} }
} }
void ShowGenericUiAction::OnNavigationEnded() {
processed_action_proto_->mutable_show_generic_ui_result()
->set_navigation_ended(true);
OnEndActionInteraction(ClientStatus(ACTION_APPLIED));
}
void ShowGenericUiAction::RegisterChecks( void ShowGenericUiAction::RegisterChecks(
BatchElementChecker* checker, BatchElementChecker* checker,
base::OnceCallback<void(const ClientStatus&)> wait_for_dom_callback) { base::OnceCallback<void(const ClientStatus&)> wait_for_dom_callback) {
...@@ -203,6 +217,9 @@ void ShowGenericUiAction::OnPreconditionResult( ...@@ -203,6 +217,9 @@ void ShowGenericUiAction::OnPreconditionResult(
size_t precondition_index, size_t precondition_index,
const ClientStatus& status, const ClientStatus& status,
const std::vector<std::string>& ignored_payloads) { const std::vector<std::string>& ignored_payloads) {
if (should_end_action_) {
return;
}
delegate_->GetUserModel()->SetValue(proto_.show_generic_ui() delegate_->GetUserModel()->SetValue(proto_.show_generic_ui()
.periodic_element_checks() .periodic_element_checks()
.element_checks(precondition_index) .element_checks(precondition_index)
...@@ -243,6 +260,12 @@ void ShowGenericUiAction::OnEndActionInteraction(const ClientStatus& status) { ...@@ -243,6 +260,12 @@ void ShowGenericUiAction::OnEndActionInteraction(const ClientStatus& status) {
} }
void ShowGenericUiAction::EndAction(const ClientStatus& status) { void ShowGenericUiAction::EndAction(const ClientStatus& status) {
if (!callback_) {
// Avoid race condition: it is possible that a breaking navigation event
// occurs immediately before or after the action would end naturally.
return;
}
delegate_->ClearGenericUi(); delegate_->ClearGenericUi();
delegate_->CleanUpAfterPrompt(); delegate_->CleanUpAfterPrompt();
UpdateProcessedAction(status); UpdateProcessedAction(status);
......
...@@ -46,6 +46,7 @@ class ShowGenericUiAction : public Action, ...@@ -46,6 +46,7 @@ class ShowGenericUiAction : public Action,
void EndAction(const ClientStatus& status); void EndAction(const ClientStatus& status);
void OnViewInflationFinished(const ClientStatus& status); void OnViewInflationFinished(const ClientStatus& status);
void OnNavigationEnded();
// From autofill::PersonalDataManagerObserver. // From autofill::PersonalDataManagerObserver.
void OnPersonalDataChanged() override; void OnPersonalDataChanged() override;
......
...@@ -455,5 +455,72 @@ TEST_F(ShowGenericUiActionTest, ElementPreconditionMissesIdentifier) { ...@@ -455,5 +455,72 @@ TEST_F(ShowGenericUiActionTest, ElementPreconditionMissesIdentifier) {
Run(); Run();
} }
TEST_F(ShowGenericUiActionTest, EndActionOnNavigation) {
ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _))
.WillByDefault(
Invoke([&](std::unique_ptr<GenericUserInterfaceProto> generic_ui,
base::OnceCallback<void(const ClientStatus&)>&
end_action_callback,
base::OnceCallback<void(const ClientStatus&)>&
view_inflation_finished_callback) {
std::move(view_inflation_finished_callback)
.Run(ClientStatus(ACTION_APPLIED));
}));
EXPECT_CALL(mock_action_delegate_, Prompt(_, _, _, _))
.WillOnce(Invoke(
[](std::unique_ptr<std::vector<UserAction>> user_actions,
bool disable_force_expand_sheet,
base::OnceCallback<void()> end_navigation_callback,
bool browse_mode) { std::move(end_navigation_callback).Run(); }));
EXPECT_CALL(mock_action_delegate_, CleanUpAfterPrompt()).Times(1);
EXPECT_CALL(
callback_,
Run(Pointee(
AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::show_generic_ui_result,
Property(&ShowGenericUiProto::Result::navigation_ended,
true))))));
proto_.set_end_on_navigation(true);
Run();
}
TEST_F(ShowGenericUiActionTest, BreakingNavigationBeforeUiIsSet) {
// End action immediately with ACTION_APPLIED after it goes into prompt.
EXPECT_CALL(mock_action_delegate_, Prompt(_, _, _, _))
.WillOnce(Invoke(
[](std::unique_ptr<std::vector<UserAction>> user_actions,
bool disable_force_expand_sheet,
base::OnceCallback<void()> end_navigation_callback,
bool browse_mode) { std::move(end_navigation_callback).Run(); }));
ON_CALL(mock_action_delegate_, OnSetGenericUi(_, _, _))
.WillByDefault(
Invoke([&](std::unique_ptr<GenericUserInterfaceProto> generic_ui,
base::OnceCallback<void(const ClientStatus&)>&
end_action_callback,
base::OnceCallback<void(const ClientStatus&)>&
view_inflation_finished_callback) {
std::move(view_inflation_finished_callback)
.Run(ClientStatus(ACTION_APPLIED));
// Also end action when UI is set. At this point, the action should
// have terminated already.
std::move(end_action_callback)
.Run(ClientStatus(OTHER_ACTION_STATUS));
}));
EXPECT_CALL(mock_action_delegate_, CleanUpAfterPrompt()).Times(1);
EXPECT_CALL(
callback_,
Run(Pointee(
AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::show_generic_ui_result,
Property(&ShowGenericUiProto::Result::navigation_ended,
true))))));
proto_.set_end_on_navigation(true);
Run();
}
// TODO(b/161652848): Add test coverage for element checks and interrupts.
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -1325,6 +1325,8 @@ message ShowGenericUiProto { ...@@ -1325,6 +1325,8 @@ message ShowGenericUiProto {
// The model containing the values for all keys specified in // The model containing the values for all keys specified in
// |output_model_identifiers|. // |output_model_identifiers|.
optional ModelProto model = 1; optional ModelProto model = 1;
// Set to true if the action was interrupted by a navigation event.
optional bool navigation_ended = 2;
} }
// The generic user interface to show. // The generic user interface to show.
...@@ -1357,6 +1359,12 @@ message ShowGenericUiProto { ...@@ -1357,6 +1359,12 @@ message ShowGenericUiProto {
repeated ElementCheck element_checks = 1; repeated ElementCheck element_checks = 1;
} }
optional PeriodicElementChecks periodic_element_checks = 6; optional PeriodicElementChecks periodic_element_checks = 6;
// When set to true, end this action on navigation events. The result will
// have |navigation_ended| set to true.
optional bool end_on_navigation = 7;
// If true, run scripts flagged with |interrupt=true| as soon as their
// preconditions match, then go back to the parent action.
optional bool allow_interrupt = 8;
} }
// Allow choosing one or more possibility. If FocusElement was called just // Allow choosing one or more possibility. If FocusElement was called just
......
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