[M76 Merge] Fix crash when pressing Assistant suggestion chip.
Previously a crash could occur because the Suggestion associated with
the suggestion chip may get destroyed during the OpenUrl sequence,
leaving us with a reference to a bad GURL.
To address this, we now post the OpenUrl to a task when handling
OnSuggestionChipPressed events. In addition to fixing the crash, this
also prevents any other observers of this event in the future from
receiving notification of the event with a bad pointer.
Note that there is potential for a similar crash when handling
notification clicks in the future, so handling that here as well. This
case does not require posting.
(cherry picked from commit c0f7696202f4172cf4683716b4338e86c5041924)
Bug: b:134773721
Change-Id: I0a178fc849c70b3015631ac2d9f7617395f616ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1646598
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667766}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1653073
Cr-Commit-Position: refs/branch-heads/3809@{#232}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/ash/assistant/assistant_interaction_controller.cc b/ash/assistant/assistant_interaction_controller.cc
index 57c2ba4..5f7ebe1 100644
--- a/ash/assistant/assistant_interaction_controller.cc
+++ b/ash/assistant/assistant_interaction_controller.cc
@@ -478,7 +478,16 @@
// If the suggestion contains a non-empty action url, we will handle the
// suggestion chip pressed event by launching the action url in the browser.
if (!suggestion->action_url.is_empty()) {
- assistant_controller_->OpenUrl(suggestion->action_url);
+ // Note that we post a new task when opening the |action_url| associated
+ // with |sugggestion| as this will potentially cause Assistant UI to close
+ // and destroy |suggestion| in the process. Failure to post in this case
+ // would cause any subsequent observers of this suggestion chip event to
+ // receive a deleted pointer.
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&AssistantController::OpenUrl,
+ assistant_controller_->GetWeakPtr(),
+ suggestion->action_url, /*from_server=*/false));
return;
}
@@ -490,7 +499,7 @@
// user's preceding voice interaction.
StartTextInteraction(suggestion->text, /*allow_tts=*/model_.response() &&
model_.response()->has_tts(),
- /*query_source*/ AssistantQuerySource::kSuggestionChip);
+ /*query_source=*/AssistantQuerySource::kSuggestionChip);
}
void AssistantInteractionController::OnSuggestionsResponse(
diff --git a/ash/assistant/assistant_notification_controller.cc b/ash/assistant/assistant_notification_controller.cc
index 19dcc7a..9552486 100644
--- a/ash/assistant/assistant_notification_controller.cc
+++ b/ash/assistant/assistant_notification_controller.cc
@@ -266,7 +266,10 @@
// Open the action url if it is valid.
if (IsValidActionUrl(action_url)) {
- assistant_controller_->OpenUrl(action_url);
+ // Note that we copy construct a new GURL as our |notification| may be
+ // destroyed during the |OpenUrl| sequence leaving |action_url| in a bad
+ // state.
+ assistant_controller_->OpenUrl(GURL(action_url));
model_.RemoveNotificationById(id, /*from_server=*/false);
return;
}