Commit 521c434b authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Fix crash when autostarting scripts.

Starting with https://chromium-review.googlesource.com/c/1319670
starting a script deletes the BatchElementChecker.

Before this patch, this caused a crash when autostarting, because

- BatchElementChecker calls the try_done callback set by the
  script_tracker
- that callback autostarts a script
- autostarting deletes the calling BatchElementChecker

The crash happens right after calling try_done, when trying to access
this to schedule a delayed task.

With this patch, the callback is called after scheduling the delayed
task. This way, there's no necessary usage of this after try_done is
called. The weak pointer keeps track of dealing with the fact that the
delayed task must not run.

Bug: 806868
Change-Id: I1b907fcc96aecb66fc0b68fe2ea0f50532542100
Reviewed-on: https://chromium-review.googlesource.com/c/1323072
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606054}
parent 8be3348c
...@@ -128,6 +128,8 @@ void BatchElementChecker::Try(base::OnceCallback<void()> try_done_callback) { ...@@ -128,6 +128,8 @@ void BatchElementChecker::Try(base::OnceCallback<void()> try_done_callback) {
void BatchElementChecker::OnTryDone(int64_t remaining_attempts, void BatchElementChecker::OnTryDone(int64_t remaining_attempts,
base::RepeatingCallback<void()> try_done, base::RepeatingCallback<void()> try_done,
base::OnceCallback<void()> all_done) { base::OnceCallback<void()> all_done) {
// Warning: try_done or all_done can indirectly delete this. this must not
// be used after calling either of these.
if (all_found_) { if (all_found_) {
try_done.Run(); try_done.Run();
std::move(all_done).Run(); std::move(all_done).Run();
...@@ -143,7 +145,6 @@ void BatchElementChecker::OnTryDone(int64_t remaining_attempts, ...@@ -143,7 +145,6 @@ void BatchElementChecker::OnTryDone(int64_t remaining_attempts,
std::move(all_done).Run(); std::move(all_done).Run();
return; return;
} }
try_done.Run();
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, FROM_HERE,
...@@ -153,6 +154,10 @@ void BatchElementChecker::OnTryDone(int64_t remaining_attempts, ...@@ -153,6 +154,10 @@ void BatchElementChecker::OnTryDone(int64_t remaining_attempts,
weak_ptr_factory_.GetWeakPtr(), remaining_attempts, weak_ptr_factory_.GetWeakPtr(), remaining_attempts,
try_done, std::move(all_done))), try_done, std::move(all_done))),
kCheckPeriod); kCheckPeriod);
// try_done must be called after creating the delayed task, in case
// try_done.Run() deletes this.
try_done.Run();
} }
void BatchElementChecker::GiveUp() { void BatchElementChecker::GiveUp() {
......
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