Commit dea5a770 authored by John Williams's avatar John Williams Committed by Commit Bot

[Media Router] Fixed crash when dismissing media notification.

This change fixes a crash caused by trying to show the same
supplemental notification twice when dismissing a media notification.
Showing the notification once causes a different problem, where
instead of closing, the dialog stays open with the closed notification
replaced by the previously-hidden supplemental notification.

The fix is to only show supplemental notifications when they are
created, and thereafter only hide them in response to the creation of
a non-supplemental notification.  The original design for supplemental
notifications was based on the idea of supplementatal notifications
being persistent in order to trigger the display of the media control
icon in the toolbar.  In the course of the code review, the design was
changed to create supplemental notifications in a just-in-time manner
to avoid showing the media control icon, so there is no longer any
need to show a supplemental notification that was previously hidden.

Bug: 1107164, b/161612881
Change-Id: I9a9b60b1e1023ef4d8085ca4df949645f999efdc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2453440Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Commit-Queue: John Williams <jrw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814787}
parent b5a365f3
......@@ -873,19 +873,17 @@ void MediaNotificationService::OnNotificationChanged(
observer.OnNotificationListChanged();
// Avoid re-examining the supplemental notifications as a side-effect of
// showing or hiding a supplemental notification.
// hiding a supplemental notification.
if (!changed_notification_id ||
base::Contains(supplemental_notifications_, *changed_notification_id))
return;
// Show or hide supplemental notifications as necessary.
// Hide supplemental notifications if necessary.
for (const auto& pair : supplemental_notifications_) {
// If there is an active session associated with the same web contents as
// this supplemental notification, hide it; if not, show it.
// this supplemental notification, hide it.
if (HasSessionForWebContents(pair.second)) {
HideNotification(pair.first);
} else {
ShowNotification(pair.first);
}
}
}
......
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