Skip to content

Commit

Permalink
[continue-section]: Prevent animating out privacy notice a second time
Browse files Browse the repository at this point in the history
When the privacy notice animation ends (or is aborted), we update the
visibility of the ContinueSection which results in the deletion of the
privacy toast view.
In an event where the continue section tries to animate the privacy
toast a second time, the first animation will be canceled, resulting in
the view getting deleted and the second animation crashing because an
UAF event.

Exit early AnimateDismissToast() if the privacy notice is already
animating out to prevent deleting the view in this case.

(cherry picked from commit a5b3791)

Bug: 1326237
Change-Id: Ie0823d1e54a01b1611e8881de1d9df57f0565027
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3649497
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Ana Salazar <anasalazar@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003985}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3654260
Auto-Submit: Ana Salazar <anasalazar@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#85}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Ana Salazar authored and Chromium LUCI CQ committed May 18, 2022
1 parent bb06976 commit e4723d1
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
8 changes: 8 additions & 0 deletions ash/app_list/views/continue_section_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ void ContinueSectionView::OnPrivacyToastAcknowledged() {
}

void ContinueSectionView::AnimateDismissToast(base::RepeatingClosure callback) {
// Prevents setting up new animation if the toast is already hiding.
// https://crbug.com/1326237.
DCHECK(privacy_toast_);
if (privacy_toast_->layer() &&
privacy_toast_->layer()->GetTargetOpacity() == 0.f) {
return;
}

PrepareForLayerAnimation(privacy_toast_);

views::AnimationBuilder animation_builder;
Expand Down
34 changes: 34 additions & 0 deletions ash/app_list/views/continue_section_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2429,5 +2429,39 @@ TEST_P(ContinueSectionViewTest, AnimatesPrivacyNoticeAccept) {
EXPECT_TRUE(GetContinueSectionView()->GetVisible());
}

// Regression test for https://crbug.com/1326237.
TEST_F(ContinueSectionViewClamshellModeTest,
RemoveSearchResultWhileAnimatingContinueSection) {
ResetPrivacyNoticePref();
InitializeForAnimationTest(/*result_count=*/3);

EXPECT_TRUE(IsPrivacyNoticeVisible());
EXPECT_EQ(GetAppListNudgeController()->current_nudge(),
AppListNudgeController::NudgeType::kPrivacyNotice);

AppListToastView* privacy_notice =
GetContinueSectionView()->GetPrivacyNoticeForTest();

GestureTapOn(privacy_notice->toast_button());

EXPECT_EQ(1.0f, privacy_notice->layer()->opacity());
EXPECT_EQ(0.0f, privacy_notice->layer()->GetTargetOpacity());
EXPECT_TRUE(privacy_notice->layer()->GetAnimator()->is_animating());

RemoveSearchResultAt(0);

EXPECT_EQ(0.0f, privacy_notice->layer()->GetTargetOpacity());
EXPECT_TRUE(privacy_notice->layer()->GetAnimator()->is_animating());

LayerAnimationStoppedWaiter waiter;
waiter.Wait(privacy_notice->layer());

WaitForAllChildrenAnimationsToComplete(
GetContinueSectionView()->suggestions_container());
EXPECT_FALSE(IsPrivacyNoticeVisible());

EXPECT_FALSE(GetContinueSectionView()->GetVisible());
}

} // namespace
} // namespace ash

0 comments on commit e4723d1

Please sign in to comment.