[Chromecast] Fix media session blocking tests.

content/public/browser/media_session.h

Index: content/public/browser/media_session.h
diff --git a/content/public/browser/media_session.h b/content/public/browser/media_session.h
index 509629f894ad72b8111d6e5a418046463e6d8c5b..a38d8833fa7a53e8d05c7d3090d214a4a565bab9 100644
--- a/content/public/browser/media_session.h
+++ b/content/public/browser/media_session.h
@@ -16,6 +16,7 @@ enum class MediaSessionAction;
 
 namespace content {
 
+class MediaSessionObserver;
 class WebContents;
 
 // MediaSession manages the media session and audio focus for a given
@@ -64,7 +65,13 @@ class MediaSession {
   virtual void StopDucking() = 0;
 
  protected:
+  virtual void AddObserver(MediaSessionObserver* observer) = 0;
+  virtual void RemoveObserver(MediaSessionObserver* observer) = 0;
+
   MediaSession() = default;
+
+ private:
+  friend class MediaSessionObserver;

[mlamouri (slow - plz ping), 2017/01/16 11:13:20]

This is a bit confusing. Why does an interface needs to be friend with the
observer class?

[derekjchow1, 2017/01/18 02:25:33]

https://cs.chromium.org/chromium/src/content/public/browser/media_session_obs...

MediaSessionObserver will call the protected AddObserver/RemoveObserver
functions.

[Zhiqiang Zhang (Slow), 2017/01/18 21:57:51]

I think you can follow the pattern of TestWebContents:

Have a TestMediaSession inheriting from MediaSessionImpl, then you don't need to
change MediaSession/MediaSessionObserver in content/public.

Then you can have MockMediaSession mocking TestMediaSession.

In this way you also don't need to change AddObserver/RemoveObserver.

WDYT?

[derekjchow1, 2017/01/19 01:28:14]

I disagree. Inheriting from MediaSessionImpl means that changes in actual
behavior of MediaSessionImpl will affect MockMediaSession. This can have side
effects in test.

The purpose of this test is to make sure that CastMediaBlocker makes the correct
calls to a MediaSession. We should be using a pure mock that is strictly used
for checking (hence why we should be using the MediaSession interface instead of
MediaSessionImpl).

[Zhiqiang Zhang (Slow), 2017/01/19 11:12:45]

I see, TestWebContents is in content/test instead of content/public/test. So if
you want to use it outside of content/, you have to use
WebContentsTester::CreateTestWebContents, which doesn't fit our situation.

I think you can put AddObserver/RemoveObserver in MediaSession and make it
non-virtual. Then MediaSessionObserver can hold a pointer to MediaSession*
instead of MediaSessionImpl*.

As I remember the reason why AddObserver/RemoveObserver were put in
MediaSessionImpl is that they are only called by MediaSessionObserver and
there's no need to expose them. I think the situation has changed as you are
mocking it from outside of content/.

 };
 
 }  // namespace content