ScreenCapture for Android phase1, part II

content/browser/renderer_host/media/peer_connection_tracker_host.cc

Index: content/browser/renderer_host/media/peer_connection_tracker_host.cc
diff --git a/content/browser/renderer_host/media/peer_connection_tracker_host.cc b/content/browser/renderer_host/media/peer_connection_tracker_host.cc
index 9fceb42a8b22123e8b61af89c9b3b20f9d028b25..2b7aa7ff785bf40bee097386cb56c48b10fcefeb 100644
--- a/content/browser/renderer_host/media/peer_connection_tracker_host.cc
+++ b/content/browser/renderer_host/media/peer_connection_tracker_host.cc
@@ -7,6 +7,7 @@
 #include "content/browser/media/webrtc/webrtc_eventlog_host.h"
 #include "content/browser/media/webrtc/webrtc_internals.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/common/media/media_stream_options.h"
 #include "content/common/media/peer_connection_tracker_messages.h"
 
 namespace content {
@@ -16,6 +17,7 @@ PeerConnectionTrackerHost::PeerConnectionTrackerHost(
     WebRTCEventLogHost* event_log_host)
     : BrowserMessageFilter(PeerConnectionTrackerMsgStart),
       render_process_id_(render_process_id),
+      is_screen_capture_(false),
       event_log_host_(event_log_host) {
   DCHECK(event_log_host);
 }
@@ -101,16 +103,20 @@ void PeerConnectionTrackerHost::OnAddStats(int lid,
 void PeerConnectionTrackerHost::OnGetUserMedia(
     const std::string& origin,
     bool audio,
-    bool video,
+    const VideoInfo& video_info,
     const std::string& audio_constraints,
     const std::string& video_constraints) {
   WebRTCInternals::GetInstance()->OnGetUserMedia(render_process_id_,
                                                  peer_pid(),
                                                  origin,
                                                  audio,
-                                                 video,
+                                                 video_info.video,
                                                  audio_constraints,
                                                  video_constraints);
+#if defined(OS_ANDROID)
+  is_screen_capture_ =

[Sergey Ulanov, 2016/08/01 18:20:36]

There can be multiple media streams, and as far as I can tell the same
PeerConnectionTrackerHost() will be used for all of them, so I don't think you
can store the state of in a single flag like this. Beside that you set the value
on IO thread here, but then use it on UI thread without any synchronization.

[braveyao, 2016/08/03 00:23:47]

Done.

Single flag is necessary on Android. During screen sharing, Chrome may be
minimized to backgound which will trigger the OnSuspend() callback here. Since
MediaProjection doesn't have audio capture, there usually is another
peerconnection for audio/video on the same page, e.g. under same
PeerConnectionTrackerHost. We shouldn't cut screensharing, also shouldn't cut
the audio session. (video session will be mute after Chrome goes to background),
which means we have to keep these two peerconnection running for such a
scenario.

[Sergey Ulanov, 2016/08/19 05:54:15]

Then I still think this code won't work correctly, particularly in the following
state:
 1. GetUserMedia() is called for screen
 2. GetUserMedia() is called for camera/mic

after (2) is_screen_capture_ will be set to false, but you want it to be set to
true in this case.

[braveyao, 2016/08/23 21:18:54]

Yes, this is a problem. Here is a temp solution and add a TODO. I'm taking
crbug/513633 now and will handle both cases together later.

[braveyao, 2016/08/30 23:06:14]

Done.

+      video_info.video_stream_source == kMediaStreamSourceScreen;
+#endif
 }
 
 void PeerConnectionTrackerHost::OnSuspend() {
@@ -122,7 +128,7 @@ void PeerConnectionTrackerHost::SendOnSuspendOnUIThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   content::RenderProcessHost* host =
       content::RenderProcessHost::FromID(render_process_id_);
-  if (host)
+  if (host && !is_screen_capture_)

[Sergey Ulanov, 2016/08/01 18:20:36]

See my comment above. I think you always want to send OnSuspend message, but
handle it differently in the renderer, depending on media stream type.

[braveyao, 2016/08/03 00:23:47]

see comments above.

     host->Send(new PeerConnectionTracker_OnSuspend());
 }