Refactor media out of WebContentsImpl to MediaWebContentsObserver.

content/public/browser/web_contents_observer.h

Index: content/public/browser/web_contents_observer.h
diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h
index 92bf9773edcdeaeb28929fe6197d77393979fb8c..f11d476b43b527af0db4d2cbea32f4f7f56e37a9 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -424,11 +424,13 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
   // Invoked when theme color is changed to |theme_color|.
   virtual void DidChangeThemeColor(SkColor theme_color) {}
 
-  // Invoked when media is playing.
-  virtual void MediaStartedPlaying() {}
-
-  // Invoked when media is paused.
-  virtual void MediaPaused() {}
+  // Invoked when media is playing or paused.  |id| is unique per player and per
+  // RenderFrameHost.  There may be multiple players within a RenderFrameHost
+  // and subsequently within a WebContents.  MediaStartedPlaying() will always
+  // be followed by MediaPaused() after player teardown.
+  using MediaPlayerId = std::pair<RenderFrameHost*, int64_t>;
+  virtual void MediaStartedPlaying(const MediaPlayerId& id) {}

[ncarter (slow), 2015/12/02 17:49:31]

If a frame is detached while media is playing, what are the semantics around
these RenderFrameHost* pointers getting invalidated? We should not tolerate
these pointers outlasting the RFH, even if they're only used as IDs.

[DaleCurtis, 2015/12/02 19:55:46]

We could use uintptr_t if we want too; I don't think anyone needs it as a
RenderFrameHost, I just changed it to avoid extraneous casting. Also, a
MediaPaused() event will be delivered during OnRenderFrameDeleted, during which
I would expect callers to release any ids they are holding. Are you okay with
upping the documentation here or would you prefer a more technical solution?

[ncarter (slow), 2015/12/05 00:43:33]

The uintptr_t vs. RFH* distinction wasn't what I was getting at.

The problem with using pointers as ids is that memory addresses get reused in a
way something like a process_id/routing_id pair doesn't. So if you use a pointer
(even opaquely via something like uintptr_t) as an ID, you need to make sure
that all references to that ID are cleaned up before the free() happens.

Your existing implementations of these observer interfaces are fine, so that
might be good enough. When I first saw this I didn't realize that "Paused" meant
that observers should forget about the |id| -- StoppedPlaying might be a better
name for this method?

[DaleCurtis, 2015/12/05 01:18:02]

Sounds like a good idea. It doesn't technically mean that, but from the POV of a
WCO it should. I've also added a comment that observers must release all copies
of |id| when MediaStoppedPlaying is received.

+  virtual void MediaPaused(const MediaPlayerId& id) {}
 
   // Invoked when media session has changed its state.
   virtual void MediaSessionStateChanged(bool is_controllable,