Commit d39f59d8 authored by David Black's avatar David Black Committed by Commit Bot

Fixes crash in Assistant.

Root cause of the crash was committing a query without first having
pended a query.

This can occur in a notifications flow or a device actions flow in
which an interaction did not originate from the UI.

We rely on pending/committing the query in the interaction model to
clear the previous query from the UI. The fix, in this case, was just
to pend an empty query so that we have something to commit.

Bug: b:111923411
Change-Id: I53848f856375cf220538738634e7543a455c0dcc
Reviewed-on: https://chromium-review.googlesource.com/1155153Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#579271}
parent db82db1b
......@@ -155,7 +155,18 @@ void AssistantInteractionController::OnInteractionStarted(
assistant_interaction_model_.SetInputModality(InputModality::kVoice);
assistant_interaction_model_.SetMicState(MicState::kOpen);
} else {
// In the case of a non-voice interaction, we commit the pending query.
// TODO(b/112000321): It should not be possible to reach this code without
// having previously pended a query. It does currently happen, however, in
// the case of notifications and device action queries which bypass the
// AssistantInteractionController when beginning an interaction. To address
// this, we temporarily pend an empty text query to commit until we can do
// development to expose something more meaningful.
if (assistant_interaction_model_.pending_query().type() ==
AssistantQueryType::kEmpty) {
assistant_interaction_model_.SetPendingQuery(
std::make_unique<AssistantTextQuery>());
}
assistant_interaction_model_.CommitPendingQuery();
assistant_interaction_model_.SetMicState(MicState::kClosed);
......
......@@ -76,6 +76,8 @@ void AssistantInteractionModel::SetPendingQuery(
}
void AssistantInteractionModel::CommitPendingQuery() {
DCHECK_NE(pending_query_->type(), AssistantQueryType::kEmpty);
committed_query_ = std::move(pending_query_);
pending_query_ = std::make_unique<AssistantEmptyQuery>();
......
......@@ -13,6 +13,7 @@ namespace ash {
// AssistantQueryType ----------------------------------------------------------
// TODO(dmblack): Rename kEmpty to kNull.
// Defines possible types of an Assistant query.
enum class AssistantQueryType {
kEmpty, // See AssistantEmptyQuery.
......@@ -44,6 +45,7 @@ class AssistantQuery {
// AssistantEmptyQuery ---------------------------------------------------------
// TODO(dmblack): Rename to AssistantNullQuery.
// An empty Assistant query used to signify the absence of an Assistant query.
class AssistantEmptyQuery : public AssistantQuery {
public:
......@@ -63,7 +65,7 @@ class AssistantEmptyQuery : public AssistantQuery {
// An Assistant text query.
class AssistantTextQuery : public AssistantQuery {
public:
explicit AssistantTextQuery(const std::string& text)
explicit AssistantTextQuery(const std::string& text = std::string())
: AssistantQuery(AssistantQueryType::kText), text_(text) {}
~AssistantTextQuery() override = default;
......
......@@ -119,16 +119,6 @@ void AssistantMainStage::OnViewVisibilityChanged(views::View* view) {
PreferredSizeChanged();
}
void AssistantMainStage::OnViewIsDeleting(views::View* view) {
if (view == committed_query_view_) {
committed_query_view_ = nullptr;
UpdateCommittedQueryViewSpacer();
} else if (view == pending_query_view_) {
pending_query_view_ = nullptr;
UpdateSuggestionContainer();
}
}
void AssistantMainStage::InitLayout(AssistantController* assistant_controller) {
SetLayoutManager(std::make_unique<views::FillLayout>());
......@@ -209,7 +199,11 @@ void AssistantMainStage::OnCommittedQueryCleared() {
return;
query_layout_container_->RemoveChildView(committed_query_view_);
delete committed_query_view_;
committed_query_view_ = nullptr;
UpdateCommittedQueryViewSpacer();
}
void AssistantMainStage::OnPendingQueryChanged(const AssistantQuery& query) {
......@@ -232,13 +226,12 @@ void AssistantMainStage::OnPendingQueryChanged(const AssistantQuery& query) {
void AssistantMainStage::OnPendingQueryCleared() {
if (pending_query_view_) {
query_layout_container_->RemoveChildView(pending_query_view_);
delete pending_query_view_;
} else {
// We only need to update the suggestion container when we are not deleting
// the pending query view. Deleting the pending query view will trigger an
// update on the suggestion container itself.
UpdateSuggestionContainer();
pending_query_view_ = nullptr;
}
UpdateSuggestionContainer();
}
void AssistantMainStage::UpdateCommittedQueryViewSpacer() {
......
......@@ -35,7 +35,6 @@ class AssistantMainStage : public views::View,
void OnViewBoundsChanged(views::View* view) override;
void OnViewPreferredSizeChanged(views::View* view) override;
void OnViewVisibilityChanged(views::View* view) override;
void OnViewIsDeleting(views::View* view) override;
// AssistantInteractionModelObserver:
void OnCommittedQueryChanged(const AssistantQuery& query) override;
......
......@@ -88,7 +88,7 @@ void AssistantQueryView::SetQuery(const AssistantQuery& query) {
break;
}
case AssistantQueryType::kEmpty:
label_->SetText(base::string16());
SetText(std::string());
break;
}
}
......@@ -123,7 +123,6 @@ void AssistantQueryView::SetText(const std::string& high_confidence_text,
label_->SizeToFit(width());
PreferredSizeChanged();
SetVisible(true);
}
} // namespace ash
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