Propagate media session id into MediaPlayerAndroid

chrome/browser/media/android/remote/remote_media_player_bridge.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/android/remote/remote_media_player_bridge.h"
 
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 25 matching lines @@
 void DoNothing(int /*i*/) {}
 }
 
 namespace remote_media {
 
 RemoteMediaPlayerBridge::RemoteMediaPlayerBridge(
     int player_id,
     const std::string& user_agent,
     bool hide_url_log,
     RemoteMediaPlayerManager* manager)
-    : MediaPlayerAndroid(player_id,
-                         manager,
-                         base::Bind(&DoNothing),
-                         manager->GetLocalPlayer(player_id)->frame_url()),
+    : MediaPlayerAndroid(
+          player_id,

[davve, 2016/02/25 09:57:32]

Is this reformat ok? Made by git cl format.

[whywhat, 2016/02/29 16:26:29]

Yes. Might cause merge problems but is not a big deal :)

+          manager,
+          base::Bind(&DoNothing),
+          manager->GetLocalPlayer(player_id)->frame_url(),
+          manager->GetLocalPlayer(player_id)->media_session_id()),

[mlamouri (slow - plz ping), 2016/02/25 14:13:54]

What does that mean for a remote player to have the same session id than the
local player?

[philipj_slow, 2016/02/29 09:50:37]

Looks like a problem brewing here, in the interaction between media session and
remote playback. It makes good sense to allow both local and remote playback at
the same time, and it would be silly if two media elements have to belong to the
same session for this to work.

Right now we have the invariant that a WebMediaPlayer belongs to a single
session for its whole lifetime, but the possibility for playback to go remote at
any time makes that invariant kind of useless.

Has anyone thought more about the implications of this? One would presumably
also want some kind of continuity in the notification.

[whywhat, 2016/02/29 16:26:29]

AFAIK, the remote player won't use the media session id at all so it's there
just to satisfy MPA constructor. Perhaps, we could pass an invalid id there?

philipj@ I think the remotely played elements belong to a special remote session
that is defined by the platform / protocol used. E.g. on Android, only one
connection can be made to a remote device at a time so each remotely played
video behaves like it is in its own session (starting one interrupts the other).
On desktop multiple connections can be made to multiple devices, however, only
one element (e.g. last) will get the focus  (e.g. MediaRouter dialog will only
show one connection with metadata and media controls; user has to click on
another connection to switch focus and control it).

[davve, 2016/03/01 05:42:22]

Right now there is no defined invalid id, but we could define one to -1 or so.
Perhaps as a enum friend to the default id.

[philipj_slow, 2016/03/03 11:10:52]

OK, so if remote playback can be modeled as one or multiple other session,
that's good. It effectively means that a media element can be transferred
between sessions, something that isn't possible with the web-exposed API so far.
If the remote session is exposed to the web, and it has to be if one is to
customize the metadata for it, then it also means that there's a new category of
sessions which can be active at the same time as kind=content, or *some* other
change to explain the behavior.

What do you all think about the metadata problem? If a media element can jump
between sessions like this, it begins to make less sense to tie the metadata to
the session. Do we need to revisit this, tying metadata to the HTMLMediaElement
and AudioContext instances instead? Or is there another way to make sense of the
situation?

       width_(0),
       height_(0),
       hide_url_log_(hide_url_log),
       url_(manager->GetLocalPlayer(player_id)->GetUrl()),
       first_party_for_cookies_(
           manager->GetLocalPlayer(player_id)->GetFirstPartyForCookies()),
       user_agent_(user_agent),
       weak_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   JNIEnv* env = base::android::AttachCurrentThread();
@@ skipping 409 matching lines @@
   Java_RemoteMediaPlayerBridge_setCookies(
       env, java_bridge_.obj(), ConvertUTF8ToJavaString(env, cookies).obj());
 }
 
 MediaPlayerAndroid* RemoteMediaPlayerBridge::GetLocalPlayer() {
   return static_cast<RemoteMediaPlayerManager*>(manager())->GetLocalPlayer(
       player_id());
 }
 
 }  // namespace remote_media