Commit a1057289 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

Fix crash in UiControllerAndroid::UpdateActions

Before this patch, when, during an AA prompt action, Chrome would crash
when users click on a link that opens a new window from within CCT.

This happened, because the Controller would call the UIDelegate during
the shutdown procedure, after calling WillShutdown, which the contract
of UIDelegate explicitly forbids.

This patch guarantees that no UIDelegate is called after WillShutdown
and ignores attempts to switch the controller state away from the
STOPPED state.

This is a workaround. It is meant to be easy and safe to cherry-pick.
The proper fix would be to shutdown immediately as part of b/128300038

Bug: 947403
Change-Id: I828eb7e53a1c789a867052b9df95b90d2b253cc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1546093
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#646256}
parent b200a93e
......@@ -82,6 +82,17 @@ Service* Controller::GetService() {
}
UiController* Controller::GetUiController() {
if (will_shutdown_) {
// Never call the UI controller after having announced a shutdown, no matter
// what happens; this is part of the contract of UIDelegate.
//
// TODO(crbug/925947): Once UIController has become observer, clear list of
// observers instead.
if (!noop_ui_controller_) {
noop_ui_controller_ = std::make_unique<UiController>();
}
return noop_ui_controller_.get();
}
return client_->GetUiController();
}
......@@ -247,11 +258,11 @@ void Controller::StopAndShutdown(Metrics::DropOutReason reason) {
}
void Controller::EnterState(AutofillAssistantState state) {
if (state_ == state)
if (state_ == state || state_ == AutofillAssistantState::STOPPED)
return;
// TODO(b/128300038): Forbid transition out of stopped again instead of
// ignoring it once shutdown sequence is less complex.
DCHECK_NE(state_, AutofillAssistantState::STOPPED)
<< "Unexpected transition from " << state_ << " to " << state;
DVLOG(2) << __func__ << ": " << state_ << " -> " << state;
state_ = state;
......@@ -572,8 +583,9 @@ AutofillAssistantState Controller::GetState() {
bool Controller::Terminate(Metrics::DropOutReason reason) {
StopPeriodicScriptChecks();
if (!will_shutdown_) {
UiController* ui_controller = GetUiController();
will_shutdown_ = true;
GetUiController()->WillShutdown(reason);
ui_controller->WillShutdown(reason);
}
if (script_tracker_ && !script_tracker_->Terminate()) {
terminate_reason_ = reason;
......
......@@ -206,6 +206,8 @@ class Controller : public ScriptExecutorDelegate,
Client* const client_;
const base::TickClock* const tick_clock_;
std::unique_ptr<UiController> noop_ui_controller_;
// Lazily instantiate in GetWebController().
std::unique_ptr<WebController> web_controller_;
......
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