Decouple MediaSession messages from WebContents (full patch)

content/public/browser/media_session_observer.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_
+#define CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_
+
+#include "base/macros.h"
+#include "base/optional.h"
+#include "content/common/content_export.h"
+#include "content/public/common/media_metadata.h"
+
+namespace content {
+
+class MediaSession;
+class MediaSessionImpl;
+
+// The observer for observing MediaSession events.
+class CONTENT_EXPORT MediaSessionObserver {
+ public:
+  // Gets the observed MediaSession.
+  MediaSession* media_session() const;

[jam, 2016/10/26 17:30:59]

this seems unnecessary, i.e. the two place that uses this can store the
WebContents or MediaSession directly.

I realize WebContentsObserver does that, but that was to avoid a few hundred
derived classes doing the same thing. but here there's only two users. ditto for
the StopObserving method and the destructor

[Zhiqiang Zhang (Slow), 2016/10/26 18:44:25]

I slightly disagree here. We actually have three places using the observer:
CastMediaBlocker, MediaSessionAndroid (for JNI) and MediaSessionTabHelper
(Java). These functions are what every user should do, so putting them here
should avoid misuses.

It's a bit confusing to remove both media_session() and StopObserving() at the
same time. If we remove StopObserving(), then all users should explicitly
call MediaSession::RemoveObserver() to avoid dangling reference, then every user
have to keep a copy of MediaSession. Then users may think: why not have a
media_session() helper method?

[Zhiqiang Zhang (Slow), 2016/10/28 22:40:07]

Removed all the MediaSession getter and StopObserving()

+
+  // Called when the observed MediaSession is being destroyed. Gives subclass a
+  // chance to cleanup. After the whole loop over MediaSessionObservers has been
+  // finished, media_session() returns nullptr.
+  virtual void MediaSessionDestroyed() {}
+
+  // Called when the observed MediaSession has changed its state.
+  virtual void MediaSessionStateChanged(bool is_controllable,
+                                        bool is_suspended) {}
+
+  // Called when the observed MediaSession has changed metadata.
+  virtual void MediaSessionMetadataChanged(
+      const base::Optional<MediaMetadata>& metadata) {}
+
+ protected:
+  // Create a MediaSessionObserver and start observing |session|.
+  explicit MediaSessionObserver(MediaSession* session);
+
+  // Destructor of MediaSessionObserver. It will remove itself from |session| if
+  // the observer is still observing a MediaSession.
+  virtual ~MediaSessionObserver();
+
+ private:
+ friend class MediaSessionImpl;
+
+ void StopObserving();
+
+  // Weak pointer to MediaSession.
+  MediaSessionImpl* session_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaSessionObserver);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_