commit d6247b1351f4ed77c14e4236345ca3b425ae1be2
author Becca Hughes <beccahughes@chromium.org> Fri Feb 08 17:50:18 2019
committer Commit Bot <commit-bot@chromium.org> Fri Feb 08 17:50:18 2019
tree 28c2535259ed04b1af25d3d3ec4ffb4143073710
parent 2e77a4e75861ccd6635684adcfc774890df2800f

[Media Notification] Handle null session info

Since ash::MediaNotificationItem is a media controller
we should be able to handle a null media session info.
This fixes a segfault caused by not handling this
scenario.

BUG=897836

Change-Id: I7869e14de9fd3d9d094f21d3daa5ff3260c3e3f8
Reviewed-on: https://chromium-review.googlesource.com/c/1457198
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630373}

ash/media/media_notification_controller_unittest.cc

diff --git a/ash/media/media_notification_controller_unittest.cc b/ash/media/media_notification_controller_unittest.cc
index 4d2d475..1493ff3 100644
--- a/ash/media/media_notification_controller_unittest.cc
+++ b/ash/media/media_notification_controller_unittest.cc
@@ -191,4 +191,24 @@
             notification->custom_view_type());
 }
 
+// Test that if we recieve a null media session info that we hide the
+// notification.
+TEST_F(MediaNotificationControllerTest, HandleNullMediaSessionInfo) {
+  ExpectNotificationCount(0);
+
+  base::UnguessableToken id = base::UnguessableToken::Create();
+
+  Shell::Get()->media_notification_controller()->OnFocusGained(
+      GetRequestStateWithId(id));
+
+  ExpectNotificationCount(1);
+
+  Shell::Get()
+      ->media_notification_controller()
+      ->GetItem(id.ToString())
+      ->MediaSessionInfoChanged(nullptr);
+
+  ExpectNotificationCount(0);
+}
+
 }  // namespace ash

ash/media/media_notification_item.cc

diff --git a/ash/media/media_notification_item.cc b/ash/media/media_notification_item.cc
index f044267b..22e6803e 100644
--- a/ash/media/media_notification_item.cc
+++ b/ash/media/media_notification_item.cc
@@ -85,7 +85,6 @@
   view_ = view;
 
   if (view) {
-    DCHECK(!session_info_.is_null());
     view_->UpdateWithMediaSessionInfo(session_info_);
     view_->UpdateWithMediaMetadata(session_metadata_);
     view_->UpdateWithMediaActions(session_actions_);
@@ -99,7 +98,7 @@
 void MediaNotificationItem::MaybeHideOrShowNotification() {
   // If the |is_controllable| bit is set in MediaSessionInfo then we should show
   // a media notification.
-  if (!session_info_->is_controllable) {
+  if (!session_info_ || !session_info_->is_controllable) {
     HideNotification();
     return;
   }

ash/media/media_notification_view.cc

diff --git a/ash/media/media_notification_view.cc b/ash/media/media_notification_view.cc
index 0c434927..93a1a3b7 100644
--- a/ash/media/media_notification_view.cc
+++ b/ash/media/media_notification_view.cc
@@ -205,8 +205,9 @@
 
 void MediaNotificationView::UpdateWithMediaSessionInfo(
     const media_session::mojom::MediaSessionInfoPtr& session_info) {
-  bool playing = session_info->playback_state ==
-                 media_session::mojom::MediaPlaybackState::kPlaying;
+  bool playing =
+      session_info && session_info->playback_state ==
+                          media_session::mojom::MediaPlaybackState::kPlaying;
   play_pause_button_->SetToggled(playing);
 
   MediaSessionAction action =