Refactoring media notification

chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java

Index: chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
index 4b1ceb728355042502570dae07625c459d0e2691..4dce275145e12262ab48b4e9f507118a269db701 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
@@ -35,6 +35,7 @@ public class MediaSessionTabHelper {
     private WebContents mWebContents;
     private WebContentsObserver mWebContentsObserver;
     private int mPreviousVolumeControlStream = AudioManager.USE_DEFAULT_STREAM_TYPE;
+    private MediaNotificationInfo.Builder mNotificationInfoBuilder = null;
 
     private MediaNotificationListener mControlsListener = new MediaNotificationListener() {
         @Override
@@ -71,18 +72,14 @@ public class MediaSessionTabHelper {
         if (activity != null) {
             activity.setVolumeControlStream(mPreviousVolumeControlStream);
         }
+        mNotificationInfoBuilder = null;
     }
 
     private WebContentsObserver createWebContentsObserver(WebContents webContents) {
         return new WebContentsObserver(webContents) {
             @Override
             public void destroy() {
-                if (mTab == null) {

[whywhat, 2015/12/09 16:59:27]

Shouldn't we move this logic into hideNotification() ?
Is the question whether mTab can never be null in case we get
WebContentsObserver.destroy() call? It seems to depend on the order of
TabObserver.onTabDestroyed() and WebContentsObserver.destroy().

Because if it is null and we will not hide the notification, the user will be
upset.

[Zhiqiang Zhang (Slow), 2015/12/09 18:02:55]

The reason I removed clear() is that sometimes if a WebContents is destroyed,
even the notification of another Tab will be destroyed, because the clear()
function only takes the notification id as argument. WDYT?

-                    MediaNotificationManager.clear(R.id.media_playback_notification);
-                } else {
-                    hideNotification();
-                }
-
+                hideNotification();
                 super.destroy();
             }
 
@@ -100,25 +97,27 @@ public class MediaSessionTabHelper {
                             + "Showing the full URL instead.");
                 }
 
+                mNotificationInfoBuilder = new MediaNotificationInfo.Builder()
+                        .setTitle(sanitizeMediaTitle(mTab.getTitle()))
+                        .setPaused(isPaused)
+                        .setOrigin(origin)
+                        .setTabId(mTab.getId())
+                        .setPrivate(mTab.isIncognito())
+                        .setIcon(R.drawable.audio_playing)
+                        .setActions(MediaNotificationInfo.ACTION_PLAY_PAUSE
+                                | MediaNotificationInfo.ACTION_SWIPEAWAY)
+                        .setId(R.id.media_playback_notification)
+                        .setListener(mControlsListener);
+
                 MediaNotificationManager.show(ApplicationStatus.getApplicationContext(),
-                        new MediaNotificationInfo.Builder()
-                                .setTitle(sanitizeMediaTitle(mTab.getTitle()))
-                                .setPaused(isPaused)
-                                .setOrigin(origin)
-                                .setTabId(mTab.getId())
-                                .setPrivate(mTab.isIncognito())
-                                .setIcon(R.drawable.audio_playing)
-                                .setActions(MediaNotificationInfo.ACTION_PLAY_PAUSE
-                                        | MediaNotificationInfo.ACTION_SWIPEAWAY)
-                                .setId(R.id.media_playback_notification)
-                                .setListener(mControlsListener));
+                        mNotificationInfoBuilder.build());
+
                 Activity activity = getActivityFromTab(mTab);
                 if (activity != null) {
                     activity.setVolumeControlStream(AudioManager.STREAM_MUSIC);
                 }
             }
         };
-
     }
 
     private void setWebContents(WebContents webContents) {