Media Session: passing metadata from renderer/ to browser/.

content/renderer/media/android/renderer_media_session_manager.cc

Index: content/renderer/media/android/renderer_media_session_manager.cc
diff --git a/content/renderer/media/android/renderer_media_session_manager.cc b/content/renderer/media/android/renderer_media_session_manager.cc
index 58b0888f568ab6c26eeec973081f924758c3c264..c5b9fc43cfb77130063e702e26c32db31e76fe09 100644
--- a/content/renderer/media/android/renderer_media_session_manager.cc
+++ b/content/renderer/media/android/renderer_media_session_manager.cc
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "content/common/media/media_session_messages_android.h"
+#include "content/public/common/media_metadata.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/renderer/media/android/webmediasession_android.h"
 
@@ -59,6 +60,27 @@ void RendererMediaSessionManager::Deactivate(
       new MediaSessionHostMsg_Deactivate(routing_id(), session_id, request_id));
 }
 
+void RendererMediaSessionManager::SetMetadata(
+    int session_id,
+    const content::MediaMetadata& metadata) {
+  // Apply some security rules on the MediaMetadata before sending over IPC.

[philipj_slow, 2015/12/14 14:09:24]

Does it help to apply this on the renderer side as well? If the renderer is
compromised, then presumably these checks won't stop it from being naughty?
(Pardon my ignorance if this is how it's always done, for some reason.)

[mlamouri (slow - plz ping), 2016/01/05 16:01:17]

This is mostly sanity check: a string more than 4000 characters long is whether
a bug or an attempt to mess with the browser process. A process sending a string
with 10k characters will not be killed, the string will only be shortened.

[philipj_slow, 2016/01/18 12:31:43]

If it's only a sanity check, maybe s/security rules/sanity check/ in the comment
so that it doesn't look like this code matters for security? Still not sure if
there's any harm in just removing the checks, would like to learn :)

[mlamouri (slow - plz ping), 2016/01/19 14:20:09]

Updated the comment. I was asked to do that a while ago by the security team for
websites provided string sent over IPC. My understanding is that malicious
websites might try to overflow the browser process. Also, long string might help
using exploit. Though, my understanding might be wrong. I'm not part of the
security team for a reason ;)

+  MediaMetadata ipc_metadata = metadata;
+  ipc_metadata.title = base::NullableString16(

[jochen (gone - plz use gerrit), 2015/12/14 12:38:30]

i'm surprised this has to be done manually, and you don't have to just use the
ipc macros to declare the struct fields :-/

[mlamouri (slow - plz ping), 2016/01/05 16:01:17]

AFAICT, it isn't done very often.

+      ipc_metadata.title.string().substr(0, MediaMetadata::kMaxIPCStringLength),
+      ipc_metadata.title.is_null());
+  ipc_metadata.artist = base::NullableString16(
+      ipc_metadata.artist.string().substr(0,
+                                          MediaMetadata::kMaxIPCStringLength),
+      ipc_metadata.artist.is_null());
+  ipc_metadata.album = base::NullableString16(
+      ipc_metadata.album.string().substr(0, MediaMetadata::kMaxIPCStringLength),
+      ipc_metadata.album.is_null());
+
+  Send(new MediaSessionHostMsg_SetMetadata(routing_id(),
+                                           session_id,
+                                           ipc_metadata));
+}
+
 void RendererMediaSessionManager::OnDidActivate(int request_id, bool success) {
   DCHECK(pending_activation_requests_.Lookup(request_id)) << request_id;
   blink::WebMediaSessionActivateCallback* callback =