Skip to content

Commit

Permalink
[M110 merge] Assistant: Fix potential crash in opt-in flow
Browse files Browse the repository at this point in the history
Check Assistant enabled status before sent voice match enrollment
requests to avoid potential crash.

(cherry picked from commit 1ac0e0b)

Bug: b/265093297
Test: Manual test
Change-Id: I68a29e3469d4e2406e026dae69ae909894ce07cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4179196
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yue Li <updowndota@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1094141}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4218134
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#897}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Yue Li authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 0a4f9a6 commit f09b271
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,47 @@ IN_PROC_BROWSER_TEST_F(AssistantOptInFlowTest, SpeakerIdEnrollment) {
1);
}

IN_PROC_BROWSER_TEST_F(AssistantOptInFlowTest,
FeatureDisabledDuringSpeakerIdEnrollment) {
auto force_lib_assistant_enabled =
AssistantOptInFlowScreen::ForceLibAssistantEnabledForTesting(true);
assistant_settings_->set_consent_ui_flags(
ScopedAssistantSettings::CONSENT_UI_FLAG_SKIP_ACTIVITY_CONTROL);
assistant_settings_->set_speaker_id_enrollment_mode(
ScopedAssistantSettings::SpeakerIdEnrollmentMode::STEP_BY_STEP);

SetUpAssistantScreensForTest();
AssistantState::Get()->NotifyStatusChanged(assistant::AssistantStatus::READY);

ShowAssistantOptInFlowScreen();

OobeScreenWaiter screen_waiter(AssistantOptInFlowScreenView::kScreenId);
screen_waiter.Wait();

test::OobeJS().CreateVisibilityWaiter(true, kAssistantRelatedInfo)->Wait();
TapWhenEnabled(kRelatedInfoNextButton);

PrefService* const prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
prefs->SetBoolean(assistant::prefs::kAssistantEnabled, false);

test::OobeJS().CreateVisibilityWaiter(true, kAssistantVoiceMatch)->Wait();
TapWhenEnabled(kVoiceMatchAgreeButton);

EXPECT_FALSE(assistant_settings_->IsSpeakerIdEnrollmentActive());

WaitForScreenExit();

ExpectCollectedOptIns({});
EXPECT_EQ(assistant::prefs::ConsentStatus::kActivityControlAccepted,
prefs->GetInteger(assistant::prefs::kAssistantConsentStatus));
EXPECT_FALSE(prefs->GetBoolean(assistant::prefs::kAssistantHotwordEnabled));
EXPECT_TRUE(prefs->GetBoolean(assistant::prefs::kAssistantContextEnabled));
EXPECT_EQ(screen_result_.value(), AssistantOptInFlowScreen::Result::NEXT);
histogram_tester_.ExpectTotalCount(kAssistantOptInScreenExitReason, 1);
histogram_tester_.ExpectTotalCount(kAssistantOptInScreenStepCompletionTime,
1);
}

IN_PROC_BROWSER_TEST_F(AssistantOptInFlowTest,
BailOutDuringSpeakerIdEnrollment) {
auto force_lib_assistant_enabled =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,9 @@ void AssistantOptInFlowScreenHandler::StopSpeakerIdEnrollment() {
DCHECK(voice_match_enrollment_started_);
voice_match_enrollment_started_ = false;
CHECK(assistant::AssistantSettings::Get());
assistant::AssistantSettings::Get()->StopSpeakerIdEnrollment();
if (AssistantState::Get()->settings_enabled().value_or(false)) {
assistant::AssistantSettings::Get()->StopSpeakerIdEnrollment();
}
// Reset the mojom receiver of |SpeakerIdEnrollmentClient|.
ResetReceiver();
}
Expand Down Expand Up @@ -551,6 +553,12 @@ void AssistantOptInFlowScreenHandler::HandleRelatedInfoScreenUserAction(

void AssistantOptInFlowScreenHandler::HandleVoiceMatchScreenUserAction(
const std::string& action) {
// If the Assistant is disabled, discard the action and end the flow instead.
if (!AssistantState::Get()->settings_enabled().value_or(false)) {
HandleFlowFinished();
return;
}

PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();

if (action == kVoiceMatchDone) {
Expand Down

0 comments on commit f09b271

Please sign in to comment.