Commit 3fb9998d authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

Reland "[Autofill Assistant] Attempt to recover pre-interrupt state."

This is a reland of c19f615e

Original change's description:
> [Autofill Assistant] Attempt to recover pre-interrupt state.
> 
> When an interrupt script is run, it can modify anything, which makes the
> main script with allow_interrupt=true hard to write. This change propose
> to reset some state automatically after an interrupt to make this
> easier.
> 
> It recovers the status message and attempts to set a reasonable scroll
> position. It sets it to either the element WaitForDom was waiting for,
> for a successful WaitForDom. For an unsuccessful WaitForDom, it scrolls
> to the last element focused on (if it still exists.)
> 
> Change-Id: Ie9c0022eebc10e0711c82d83b943e0de152af1fd
> Reviewed-on: https://chromium-review.googlesource.com/c/1346451
> Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
> Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610412}

Change-Id: If45ba47d26f29102388e35d3454c5d8f48911944
Reviewed-on: https://chromium-review.googlesource.com/c/1349212Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610586}
parent 71b5c6a4
...@@ -181,6 +181,11 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat ...@@ -181,6 +181,11 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showStatusMessage(message)); mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showStatusMessage(message));
} }
@CalledByNative
private String onGetStatusMessage() {
return mStatusMessage;
}
@CalledByNative @CalledByNative
private void onShowOverlay() { private void onShowOverlay() {
mUiDelegateHolder.performUiOperation(uiDelegate -> { mUiDelegateHolder.performUiOperation(uiDelegate -> {
......
...@@ -111,6 +111,16 @@ void UiControllerAndroid::ShowStatusMessage(const std::string& message) { ...@@ -111,6 +111,16 @@ void UiControllerAndroid::ShowStatusMessage(const std::string& message) {
base::android::ConvertUTF8ToJavaString(env, message)); base::android::ConvertUTF8ToJavaString(env, message));
} }
std::string UiControllerAndroid::GetStatusMessage() {
JNIEnv* env = AttachCurrentThread();
std::string status;
base::android::ScopedJavaLocalRef<jstring> message =
Java_AutofillAssistantUiController_onGetStatusMessage(
env, java_autofill_assistant_ui_controller_);
base::android::ConvertJavaStringToUTF8(env, message.obj(), &status);
return status;
}
void UiControllerAndroid::ShowOverlay() { void UiControllerAndroid::ShowOverlay() {
Java_AutofillAssistantUiController_onShowOverlay( Java_AutofillAssistantUiController_onShowOverlay(
AttachCurrentThread(), java_autofill_assistant_ui_controller_); AttachCurrentThread(), java_autofill_assistant_ui_controller_);
......
...@@ -38,6 +38,7 @@ class UiControllerAndroid : public UiController, ...@@ -38,6 +38,7 @@ class UiControllerAndroid : public UiController,
// Overrides UiController: // Overrides UiController:
void SetUiDelegate(UiDelegate* ui_delegate) override; void SetUiDelegate(UiDelegate* ui_delegate) override;
void ShowStatusMessage(const std::string& message) override; void ShowStatusMessage(const std::string& message) override;
std::string GetStatusMessage() override;
void ShowOverlay() override; void ShowOverlay() override;
void HideOverlay() override; void HideOverlay() override;
void Shutdown() override; void Shutdown() override;
......
...@@ -23,6 +23,7 @@ class MockUiController : public UiController { ...@@ -23,6 +23,7 @@ class MockUiController : public UiController {
MOCK_METHOD1(SetUiDelegate, void(UiDelegate* ui_delegate)); MOCK_METHOD1(SetUiDelegate, void(UiDelegate* ui_delegate));
MOCK_METHOD1(ShowStatusMessage, void(const std::string& message)); MOCK_METHOD1(ShowStatusMessage, void(const std::string& message));
MOCK_METHOD0(GetStatusMessage, std::string());
MOCK_METHOD0(ShowOverlay, void()); MOCK_METHOD0(ShowOverlay, void());
MOCK_METHOD0(HideOverlay, void()); MOCK_METHOD0(HideOverlay, void());
MOCK_METHOD0(Shutdown, void()); MOCK_METHOD0(Shutdown, void());
......
...@@ -190,12 +190,13 @@ void ScriptExecutor::HighlightElement(const Selector& selector, ...@@ -190,12 +190,13 @@ void ScriptExecutor::HighlightElement(const Selector& selector,
void ScriptExecutor::FocusElement(const Selector& selector, void ScriptExecutor::FocusElement(const Selector& selector,
base::OnceCallback<void(bool)> callback) { base::OnceCallback<void(bool)> callback) {
last_focused_element_selector_ = selector;
delegate_->GetWebController()->FocusElement(selector, std::move(callback)); delegate_->GetWebController()->FocusElement(selector, std::move(callback));
} }
void ScriptExecutor::SetTouchableElements( void ScriptExecutor::SetTouchableElements(
const std::vector<Selector>& element_selectors) { const std::vector<Selector>& element_selector) {
touchable_elements_ = element_selectors; touchable_elements_ = element_selector;
} }
void ScriptExecutor::ShowProgressBar(int progress, const std::string& message) { void ScriptExecutor::ShowProgressBar(int progress, const std::string& message) {
...@@ -515,6 +516,7 @@ void ScriptExecutor::WaitWithInterrupts::OnAllDone() { ...@@ -515,6 +516,7 @@ void ScriptExecutor::WaitWithInterrupts::OnAllDone() {
void ScriptExecutor::WaitWithInterrupts::RunInterrupt(const Script* interrupt) { void ScriptExecutor::WaitWithInterrupts::RunInterrupt(const Script* interrupt) {
batch_element_checker_.reset(); batch_element_checker_.reset();
SavePreInterruptState();
interrupt_executor_ = std::make_unique<ScriptExecutor>( interrupt_executor_ = std::make_unique<ScriptExecutor>(
interrupt->handle.path, main_script_->initial_server_payload_, interrupt->handle.path, main_script_->initial_server_payload_,
/* listener= */ nullptr, main_script_->scripts_state_, &no_interrupts_, /* listener= */ nullptr, main_script_->scripts_state_, &no_interrupts_,
...@@ -532,6 +534,7 @@ void ScriptExecutor::WaitWithInterrupts::OnInterruptDone( ...@@ -532,6 +534,7 @@ void ScriptExecutor::WaitWithInterrupts::OnInterruptDone(
RunCallback(false, &result); RunCallback(false, &result);
return; return;
} }
RestorePreInterruptUiState();
// TODO(crbug.com/806868): Forward global state change from the server_payload // TODO(crbug.com/806868): Forward global state change from the server_payload
// of a successfully run interrupt to the main script. // of a successfully run interrupt to the main script.
...@@ -544,12 +547,46 @@ void ScriptExecutor::WaitWithInterrupts::OnInterruptDone( ...@@ -544,12 +547,46 @@ void ScriptExecutor::WaitWithInterrupts::OnInterruptDone(
void ScriptExecutor::WaitWithInterrupts::RunCallback( void ScriptExecutor::WaitWithInterrupts::RunCallback(
bool found, bool found,
const ScriptExecutor::Result* result) { const ScriptExecutor::Result* result) {
// stop element checking in one is still in progress // stop element checking if one is still in progress
batch_element_checker_.reset(); batch_element_checker_.reset();
if (callback_) if (!callback_)
return;
RestorePreInterruptScroll(found);
std::move(callback_).Run(found, result); std::move(callback_).Run(found, result);
} }
void ScriptExecutor::WaitWithInterrupts::SavePreInterruptState() {
if (saved_pre_interrupt_state_)
return;
pre_interrupt_status_ =
main_script_->delegate_->GetUiController()->GetStatusMessage();
saved_pre_interrupt_state_ = true;
}
void ScriptExecutor::WaitWithInterrupts::RestorePreInterruptUiState() {
if (!saved_pre_interrupt_state_)
return;
main_script_->delegate_->GetUiController()->ShowStatusMessage(
pre_interrupt_status_);
}
void ScriptExecutor::WaitWithInterrupts::RestorePreInterruptScroll(
bool element_found) {
if (!saved_pre_interrupt_state_)
return;
auto* delegate = main_script_->delegate_;
if (element_found) {
delegate->GetWebController()->FocusElement(selector_, base::DoNothing());
} else if (!main_script_->last_focused_element_selector_.empty()) {
delegate->GetWebController()->FocusElement(
main_script_->last_focused_element_selector_, base::DoNothing());
}
}
void ScriptExecutor::OnChosen( void ScriptExecutor::OnChosen(
base::OnceCallback<void(const std::string&)> callback, base::OnceCallback<void(const std::string&)> callback,
const std::string& payload) { const std::string& payload) {
......
...@@ -177,6 +177,16 @@ class ScriptExecutor : public ActionDelegate { ...@@ -177,6 +177,16 @@ class ScriptExecutor : public ActionDelegate {
void OnInterruptDone(const ScriptExecutor::Result& result); void OnInterruptDone(const ScriptExecutor::Result& result);
void RunCallback(bool found, const ScriptExecutor::Result* result); void RunCallback(bool found, const ScriptExecutor::Result* result);
// Saves the current state and sets save_pre_interrupt_state_.
void SavePreInterruptState();
// Restores the UI states as found by SavePreInterruptState.
void RestorePreInterruptUiState();
// if save_pre_interrupt_state_ is set, attempt to scroll the page back to
// the original area.
void RestorePreInterruptScroll(bool element_found);
const ScriptExecutor* main_script_; const ScriptExecutor* main_script_;
const base::TimeDelta max_wait_time_; const base::TimeDelta max_wait_time_;
const ElementCheckType check_type_; const ElementCheckType check_type_;
...@@ -194,6 +204,13 @@ class ScriptExecutor : public ActionDelegate { ...@@ -194,6 +204,13 @@ class ScriptExecutor : public ActionDelegate {
// The interrupt that's currently running. // The interrupt that's currently running.
std::unique_ptr<ScriptExecutor> interrupt_executor_; std::unique_ptr<ScriptExecutor> interrupt_executor_;
// If true, pre-interrupt state was saved already. This happens just before
// the first interrupt.
bool saved_pre_interrupt_state_;
// The status message that was displayed when the interrupt started.
std::string pre_interrupt_status_;
DISALLOW_COPY_AND_ASSIGN(WaitWithInterrupts); DISALLOW_COPY_AND_ASSIGN(WaitWithInterrupts);
}; };
friend class WaitWithInterrupts; friend class WaitWithInterrupts;
...@@ -229,6 +246,7 @@ class ScriptExecutor : public ActionDelegate { ...@@ -229,6 +246,7 @@ class ScriptExecutor : public ActionDelegate {
bool should_stop_script_; bool should_stop_script_;
bool should_clean_contextual_ui_on_finish_; bool should_clean_contextual_ui_on_finish_;
ActionProto::ActionInfoCase previous_action_type_; ActionProto::ActionInfoCase previous_action_type_;
Selector last_focused_element_selector_;
std::vector<Selector> touchable_elements_; std::vector<Selector> touchable_elements_;
std::map<std::string, ScriptStatusProto>* scripts_state_; std::map<std::string, ScriptStatusProto>* scripts_state_;
......
...@@ -39,6 +39,10 @@ class UiController { ...@@ -39,6 +39,10 @@ class UiController {
// Show status message on the bottom bar. // Show status message on the bottom bar.
virtual void ShowStatusMessage(const std::string& message) = 0; virtual void ShowStatusMessage(const std::string& message) = 0;
// Returns the current status message. The purpose of this call is to allow
// restoring a previous status message.
virtual std::string GetStatusMessage() = 0;
// Show the overlay. // Show the overlay.
virtual void ShowOverlay() = 0; virtual void ShowOverlay() = 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