Commit de64e7eb authored by Tetsui Ohkubo's avatar Tetsui Ohkubo Committed by Commit Bot

Fix MessagePopupCollection asan failure.

Rather than closing popups immediately, we should close the popups with
animation and let MessagePopupCollection::Update() close them
eventually.

Ideally MarkAllPopupsShown is problematic and this should be cleaned up
along with UiController, but for now the fix should be acceptable as it
just replicates the behavior of old MessagePopupCollection.

Steps to repro:
1. Create a notification
2. Click on a notification

TEST=manual(asan build)
BUG=869593,869716

Change-Id: Ifdfc7c2a362727740401e6e16845d8d770877730
Reviewed-on: https://chromium-review.googlesource.com/1158118Reviewed-by: default avatarYoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580087}
parent ecade6c5
...@@ -209,6 +209,7 @@ void MessageCenterNotificationManager::CancelAll() { ...@@ -209,6 +209,7 @@ void MessageCenterNotificationManager::CancelAll() {
void MessageCenterNotificationManager::StartShutdown() { void MessageCenterNotificationManager::StartShutdown() {
is_shutdown_started_ = true; is_shutdown_started_ = true;
CancelAll(); CancelAll();
popups_only_ui_controller_.reset();
} }
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
......
...@@ -72,17 +72,19 @@ void MessagePopupCollection::Update() { ...@@ -72,17 +72,19 @@ void MessagePopupCollection::Update() {
void MessagePopupCollection::MarkAllPopupsShown() { void MessagePopupCollection::MarkAllPopupsShown() {
if (is_updating_) if (is_updating_)
return; return;
base::AutoReset<bool> reset(&is_updating_, true); {
base::AutoReset<bool> reset(&is_updating_, true);
for (const auto& item : popup_items_) { for (const auto& item : popup_items_)
item.popup->Close(); MessageCenter::Get()->MarkSinglePopupAsShown(item.id, false);
MessageCenter::Get()->MarkSinglePopupAsShown(item.id, false);
ResetHotMode();
state_ = State::IDLE;
animation_->End();
} }
popup_items_.clear();
ResetHotMode(); // Restart animation for FADE_OUT.
state_ = State::IDLE; Update();
animation_->End();
} }
void MessagePopupCollection::ResetBounds() { void MessagePopupCollection::ResetBounds() {
......
...@@ -390,6 +390,7 @@ TEST_F(MessagePopupCollectionTest, MarkAllPopupsShown) { ...@@ -390,6 +390,7 @@ TEST_F(MessagePopupCollectionTest, MarkAllPopupsShown) {
EXPECT_EQ(kMaxVisiblePopupNotifications, GetPopupCounts()); EXPECT_EQ(kMaxVisiblePopupNotifications, GetPopupCounts());
popup_collection()->MarkAllPopupsShown(); popup_collection()->MarkAllPopupsShown();
AnimateUntilIdle();
EXPECT_EQ(0u, GetPopupCounts()); EXPECT_EQ(0u, GetPopupCounts());
EXPECT_EQ(0u, MessageCenter::Get()->GetPopupNotifications().size()); EXPECT_EQ(0u, MessageCenter::Get()->GetPopupNotifications().size());
......
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