Update Android IPC messages to EME WD

content/browser/media/android/browser_media_player_manager.cc

Index: content/browser/media/android/browser_media_player_manager.cc
diff --git a/content/browser/media/android/browser_media_player_manager.cc b/content/browser/media/android/browser_media_player_manager.cc
index 89e6226b719ba5a6f4e7ebfa20b7af716d87d951..c35d3d1ce5e94189ccd22021e29688e7e5c989c1 100644
--- a/content/browser/media/android/browser_media_player_manager.cc
+++ b/content/browser/media/android/browser_media_player_manager.cc
@@ -123,11 +123,9 @@ bool BrowserMediaPlayerManager::OnMessageReceived(const IPC::Message& msg) {
                         DestroyAllMediaPlayers)
     IPC_MESSAGE_HANDLER(MediaKeysHostMsg_InitializeCDM,
                         OnInitializeCDM)
-    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_GenerateKeyRequest,
-                        OnGenerateKeyRequest)
-    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_AddKey, OnAddKey)
-    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_CancelKeyRequest,
-                        OnCancelKeyRequest)
+    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_CreateSession, OnCreateSession)
+    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_UpdateSession, OnUpdateSession)
+    IPC_MESSAGE_HANDLER(MediaKeysHostMsg_ReleaseSession, OnReleaseSession)
 #if defined(GOOGLE_TV)
     IPC_MESSAGE_HANDLER(MediaPlayerHostMsg_NotifyExternalSurface,
                         OnNotifyExternalSurface)
@@ -355,7 +353,7 @@ void BrowserMediaPlayerManager::OnProtectedSurfaceRequested(int player_id) {
   // During the process, DisableFullscreenEncryptedMediaPlayback() may get
   // called before or after OnEnterFullscreen(). If it is called before
   // OnEnterFullscreen(), the player will not enter fullscreen. And it will
-  // retry the process once the GenerateKeyRequest is allowed to proceed
+  // retry the process once CreateSession() is allowed to proceed.
   // TODO(qinmin): make this flag default on android.
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableGestureRequirementForMediaFullscreen)) {
@@ -369,7 +367,7 @@ void BrowserMediaPlayerManager::OnSessionCreated(
     int media_keys_id,
     uint32 session_id,
     const std::string& web_session_id) {
-  Send(new MediaKeysMsg_SetSessionId(
+  Send(new MediaKeysMsg_SessionCreated(
       routing_id(), media_keys_id, session_id, web_session_id));
 }
 
@@ -378,18 +376,18 @@ void BrowserMediaPlayerManager::OnSessionMessage(
     uint32 session_id,
     const std::vector<uint8>& message,
     const std::string& destination_url) {
-  Send(new MediaKeysMsg_KeyMessage(
+  Send(new MediaKeysMsg_SessionMessage(
       routing_id(), media_keys_id, session_id, message, destination_url));
 }
 
 void BrowserMediaPlayerManager::OnSessionReady(int media_keys_id,
                                                uint32 session_id) {
-  Send(new MediaKeysMsg_KeyAdded(routing_id(), media_keys_id, session_id));
+  Send(new MediaKeysMsg_SessionReady(routing_id(), media_keys_id, session_id));
 }
 
 void BrowserMediaPlayerManager::OnSessionClosed(int media_keys_id,
                                                 uint32 session_id) {
-  // TODO(jrummell): Update Android calls and IPC names.
+  Send(new MediaKeysMsg_SessionClosed(routing_id(), media_keys_id, session_id));
 }
 
 void BrowserMediaPlayerManager::OnSessionError(
@@ -397,7 +395,7 @@ void BrowserMediaPlayerManager::OnSessionError(
     uint32 session_id,
     media::MediaKeys::KeyError error_code,
     int system_code) {
-  Send(new MediaKeysMsg_KeyError(
+  Send(new MediaKeysMsg_SessionError(
       routing_id(), media_keys_id, session_id, error_code, system_code));
 }
 
@@ -546,17 +544,39 @@ void BrowserMediaPlayerManager::OnInitializeCDM(
     int media_keys_id,
     const std::vector<uint8>& uuid,
     const GURL& frame_url) {
+  if (uuid.size() != 16) {
+    // This failure will be discovered and reported by OnCreateSession()

[palmer, 2013/12/11 00:18:18]

That's not immediately obvious to me. I think it is most clear to put the
protective code here, right at the IPC boundary, and then "internal" code like
GetDrmBridge can proceed knowing that things are OK. (That is, assuming
OnCreateSession, or this IPC boundary code generally, is the only caller of
GetDrmBridge. If GetDrmBridge has other callers in other places, it will need to
defend itself from them.)

[jrummell, 2013/12/11 02:21:52]

Errors can only be thrown at sessions, so this initialization code has nowhere
to report to. I just copied what AddDrmBridge() does for failures.

We already have the code to check this in every place it is used.

+    // as GetDrmBridge() will return null.
+    DLOG(WARNING) << "Invalid UUID for ID: " << media_keys_id;

[ddorwin, 2013/12/10 23:48:08]

NOTREACHED()?
This is a programming bug, so I think we can just DCHECK.

[palmer, 2013/12/11 00:18:18]

No, because you never know what kind of weirdness a compromised or insane
renderer will send. Keep it as a run-time check.

[ddorwin, 2013/12/11 00:30:04]

Right, keep the runtime check at 547, but NOTREACHED (a DCHECK) instead of DLOG
at 550. Basically, in debug builds, this programming error is fatal.

[jrummell, 2013/12/11 02:21:52]

Done.

+    return;
+  }
+
   AddDrmBridge(media_keys_id, uuid, frame_url);
   // In EME v0.1b MediaKeys lives in the media element. So the |media_keys_id|
   // is the same as the |player_id|.
   OnSetMediaKeys(media_keys_id, media_keys_id);
 }
 
-void BrowserMediaPlayerManager::OnGenerateKeyRequest(
+void BrowserMediaPlayerManager::OnCreateSession(
     int media_keys_id,
     uint32 session_id,
     const std::string& type,
     const std::vector<uint8>& init_data) {
+  if (type.length() > 512) {

[ddorwin, 2013/12/10 23:48:08]

50 is more than enough for type.

[jrummell, 2013/12/11 02:21:52]

Done.

+    DLOG(WARNING) << "'type' for ID: " << media_keys_id

[ddorwin, 2013/12/10 23:48:08]

We probably don't need to log since this should never happen. Same below.

[jrummell, 2013/12/11 02:21:52]

Done.

+                  << " too long: " << type.length();
+    OnSessionError(
+        media_keys_id, session_id, media::MediaKeys::kUnknownError, 0);
+    return;
+  }
+  if (init_data.size() > 10240) {

[ddorwin, 2013/12/10 23:48:08]

Should we note that these are checks for security (and the sizes are arbitrary)?

[palmer, 2013/12/11 00:18:18]

Yes to both. I'd use some kind of named constant in place of the magic numbers.
If we can't appeal to the W3C specification for limits (is that a gap in the
spec?), then yes, note that they are arbitrary.

[ddorwin, 2013/12/11 00:30:04]

initData is container-specific and the messages and response values are key
system-specific, so there's not pre-determined limits. We could say something
like these parameters must be smaller than 50 KB, but that's arbitrary and
someone is likely to complain. Did you have something else in mind?

[jrummell, 2013/12/11 02:21:52]

Done.

+    DLOG(WARNING) << "'init_data' for ID: " << media_keys_id
+                  << " too long:" << init_data.size();
+    OnSessionError(
+        media_keys_id, session_id, media::MediaKeys::kUnknownError, 0);
+    return;
+  }
+
   if (CommandLine::ForCurrentProcess()
       ->HasSwitch(switches::kDisableInfobarForProtectedMediaIdentifier)) {
     GenerateKeyIfAllowed(media_keys_id, session_id, type, init_data, true);
@@ -586,10 +606,10 @@ void BrowserMediaPlayerManager::OnGenerateKeyRequest(
                  init_data));
 }
 
-void BrowserMediaPlayerManager::OnAddKey(int media_keys_id,
-                                         uint32 session_id,
-                                         const std::vector<uint8>& key,
-                                         const std::vector<uint8>& init_data) {
+void BrowserMediaPlayerManager::OnUpdateSession(
+    int media_keys_id,
+    uint32 session_id,
+    const std::vector<uint8>& response) {
   MediaDrmBridge* drm_bridge = GetDrmBridge(media_keys_id);
   if (!drm_bridge) {
     DLOG(WARNING) << "No MediaDrmBridge for ID: " << media_keys_id << " found";
@@ -598,8 +618,15 @@ void BrowserMediaPlayerManager::OnAddKey(int media_keys_id,
     return;
   }
 
-  DCHECK(init_data.empty());
-  drm_bridge->UpdateSession(session_id, &key[0], key.size());
+  if (response.size() > 10240) {

[palmer, 2013/12/11 00:18:18]

Same thing here, and it's another argument in favor of a named constant — we
wouldn't want our arbitrary numbers to get out of sync. :)

[jrummell, 2013/12/11 02:21:52]

Done.

+    DLOG(WARNING) << "Response for ID: " << media_keys_id
+                  << " too long: " << response.size();
+    OnSessionError(
+        media_keys_id, session_id, media::MediaKeys::kUnknownError, 0);
+    return;
+  }
+
+  drm_bridge->UpdateSession(session_id, &response[0], response.size());
   // In EME v0.1b MediaKeys lives in the media element. So the |media_keys_id|
   // is the same as the |player_id|.
   // TODO(xhwang): Separate |media_keys_id| and |player_id|.
@@ -608,8 +635,8 @@ void BrowserMediaPlayerManager::OnAddKey(int media_keys_id,
     player->OnKeyAdded();
 }
 
-void BrowserMediaPlayerManager::OnCancelKeyRequest(int media_keys_id,
-                                                   uint32 session_id) {
+void BrowserMediaPlayerManager::OnReleaseSession(int media_keys_id,
+                                                 uint32 session_id) {
   MediaDrmBridge* drm_bridge = GetDrmBridge(media_keys_id);
   if (!drm_bridge) {
     DLOG(WARNING) << "No MediaDrmBridge for ID: " << media_keys_id << " found";
@@ -681,7 +708,7 @@ void BrowserMediaPlayerManager::AddDrmBridge(int media_keys_id,
   scoped_ptr<MediaDrmBridge> drm_bridge(MediaDrmBridge::Create(
       media_keys_id, uuid, frame_url, security_level, this));
   if (!drm_bridge) {
-    // This failure will be discovered and reported by OnGenerateKeyRequest()
+    // This failure will be discovered and reported by OnCreateSession()
     // as GetDrmBridge() will return null.
     DVLOG(1) << "failed to create drm bridge.";
     return;