Implement MediaSession (metadata) per frame [NOT READY, HAS DEPENDENCY]

content/browser/media/android/browser_media_session_manager.cc

Index: content/browser/media/android/browser_media_session_manager.cc
diff --git a/content/browser/media/android/browser_media_session_manager.cc b/content/browser/media/android/browser_media_session_manager.cc
index dffee6cdfd7f3719977e6ae927251828a97bbfa6..1ce090fc2829f23eeafd5be8b0b5bc1823e1c443 100644
--- a/content/browser/media/android/browser_media_session_manager.cc
+++ b/content/browser/media/android/browser_media_session_manager.cc
@@ -11,13 +11,20 @@
 #include "content/common/media/media_session_messages_android.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
-#include "content/public/common/media_metadata.h"
 
 namespace content {
 
 BrowserMediaSessionManager::BrowserMediaSessionManager(
-    RenderFrameHost* render_frame_host)
-    : render_frame_host_(render_frame_host) {}
+    RenderFrameHost* render_frame_host,
+    WebContentsImpl* contents)
+    : render_frame_host_(render_frame_host), contents_(contents) {}
+
+BrowserMediaSessionManager::~BrowserMediaSessionManager() = default;
+
+const base::Optional<MediaMetadata>&
+BrowserMediaSessionManager::media_metadata() const {
+  return media_metadata_;

[whywhat, 2016/09/13 16:23:26]

nit: this is allowed to be inlined in the header IIRC

[Zhiqiang Zhang (Slow), 2016/09/13 18:51:52]

Done.

+}
 
 void BrowserMediaSessionManager::OnActivate(int session_id, int request_id) {
   NOTIMPLEMENTED();
@@ -32,6 +39,12 @@ void BrowserMediaSessionManager::OnDeactivate(int session_id, int request_id) {
 void BrowserMediaSessionManager::OnSetMetadata(
     int session_id,
     const base::Optional<MediaMetadata>& insecure_metadata) {

[whywhat, 2016/09/13 16:23:26]

nit: the declaration of the method has this named as simply |metadata|
why is it |insecure|? update the header file and leave a comment explaining the
insecurity of the metadata.

[Zhiqiang Zhang (Slow), 2016/09/13 18:51:52]

Done. Updated the header file.

+  // Non-top-level frames should not receive metadata.
+  if (render_frame_host_->GetParent()) {
+    render_frame_host_->GetProcess()->ShutdownForBadMessage();

[whywhat, 2016/09/13 16:23:26]

This is a preventative measure against p0wned renderers, right? Maybe leave a
comment.

In general case, we should prevent this from happening in the renderer process,
not kill the renderer whenever an iframe sets metadata.

[Zhiqiang Zhang (Slow), 2016/09/13 17:15:56]

There are sanity check and sanitizing when the renderer sends the message, see
RendererMediaSessionManager::SetMetadata().

I think that is fine. Since we have sanitized the message in the renderer, so if
the browser receives a dirty metadata, it should come from an attacker. It
should be fine to shutdown renderer in this case.

+    return;
+  }
+
   // When receiving a MediaMetadata, the browser process can't trust that it is
   // coming from a known and secure source. It must be processed accordingly.
   if (insecure_metadata.has_value() &&
@@ -40,7 +53,8 @@ void BrowserMediaSessionManager::OnSetMetadata(
     return;
   }
 
-  NOTIMPLEMENTED();
+  media_metadata_ = insecure_metadata;
+  contents_->OnMediaSessionStateChanged();
 }
 
 int BrowserMediaSessionManager::GetRoutingID() const {