[Cross-Site Isolation] Migrate entire MediaStream verticals to be per-RenderFrame.

content/browser/media/capture/web_contents_audio_input_stream.cc

Index: content/browser/media/capture/web_contents_audio_input_stream.cc
diff --git a/content/browser/media/capture/web_contents_audio_input_stream.cc b/content/browser/media/capture/web_contents_audio_input_stream.cc
index ad5a33f6923541f7465da500a21f1c752f40b803..f9e48f9919dde39321d3c94375dd8d531abafafd 100644
--- a/content/browser/media/capture/web_contents_audio_input_stream.cc
+++ b/content/browser/media/capture/web_contents_audio_input_stream.cc
@@ -25,7 +25,7 @@ class WebContentsAudioInputStream::Impl
       public AudioMirroringManager::MirroringDestination {
  public:
   // Takes ownership of |mixer_stream|.  The rest outlive this instance.
-  Impl(int render_process_id, int render_view_id,
+  Impl(int render_process_id, int main_render_frame_id,

[ncarter (slow), 2014/07/10 01:17:51]

Interesting, this points to a possible hurdle (which in no way needs to be
addressed now). Do you think it will be feasible, in oopif, to capture audio
from all the (cross-process) frames of a page?

Situations like a youtube player inside of an iframe could be imagined to be
fairly common.

[miu, 2014/07/10 22:16:12]

Yes.  I've prototyped a solution that seems to work well, with some unfortunate
added complexity: The problem at-hand is that AudioMirroringManager is trying to
match audio streams, associated by render frame routing IDs, against
MirroringDestinations that want the streams from multiple render frames (for a
tab).  To accomplish this, I've prototyped an asynchronous "query" API where
AudioMirroringManager will ask a MirroringDestination whether it wants to
consume audio for a specific render frame.  The API has to be asynchronous
because we have to hop from the IO thread to the UI thread to execute such a
query.

Once this change lands, I'll rebase my prototype, determine whether it still
looks like the right solution, and clean it up for review.

[ncarter (slow), 2014/07/11 22:32:24]

Pretty cool. Glad to hear you're on top of it.

        AudioMirroringManager* mirroring_manager,
        const scoped_refptr<WebContentsTracker>& tracker,
        media::VirtualAudioInputStream* mixer_stream);
@@ -84,6 +84,8 @@ class WebContentsAudioInputStream::Impl
   void OnTargetChanged(int render_process_id, int render_view_id);
 
   // Injected dependencies.
+  const int initial_render_process_id_;
+  const int initial_main_render_frame_id_;
   AudioMirroringManager* const mirroring_manager_;
   const scoped_refptr<WebContentsTracker> tracker_;
   // The AudioInputStream implementation that handles the audio conversion and
@@ -105,14 +107,23 @@ class WebContentsAudioInputStream::Impl
 };
 
 WebContentsAudioInputStream::Impl::Impl(
-    int render_process_id, int render_view_id,
+    int render_process_id, int main_render_frame_id,
     AudioMirroringManager* mirroring_manager,
     const scoped_refptr<WebContentsTracker>& tracker,
     media::VirtualAudioInputStream* mixer_stream)
-    : mirroring_manager_(mirroring_manager),
-      tracker_(tracker), mixer_stream_(mixer_stream), state_(CONSTRUCTED),
-      target_render_process_id_(render_process_id),
-      target_render_view_id_(render_view_id),
+    : initial_render_process_id_(render_process_id),
+      initial_main_render_frame_id_(main_render_frame_id),
+      mirroring_manager_(mirroring_manager),
+      tracker_(tracker),
+      mixer_stream_(mixer_stream),
+      state_(CONSTRUCTED),
+      // TODO(miu): This hack for initializing target_render_XXX_id_ is needed
+      // so IsTargetLost() won't prematurely return false when called in Start()
+      // below.  This will be removed in a soon-upcoming change when all this
+      // code is migrated away from using RenderViewHosts (for cross-site
+      // isolation).

[ncarter (slow), 2014/07/10 01:17:51]

So -- it seems like this workaround is needed because this object is created on
the IO thread to target an initial frame routing_id, but some of the code hasn't
been switched over to use frame routing_ids -- and because we're not on the UI
thread, we can't easily get from the frame to the view.

I'd consider using a bool parameter (target_identified_ ?) on the side, instead
of using kintmax (which usually means MSG_ROUTING_CONTROL). With the boolean you
probably wouldn't need quite as long a comment.

[miu, 2014/07/10 22:16:12]

Done.

+      target_render_process_id_(kint32max),
+      target_render_view_id_(kint32max),
       callback_(NULL) {
   DCHECK(mirroring_manager_);
   DCHECK(tracker_.get());
@@ -138,7 +149,7 @@ bool WebContentsAudioInputStream::Impl::Open() {
   state_ = OPENED;
 
   tracker_->Start(
-      target_render_process_id_, target_render_view_id_,
+      initial_render_process_id_, initial_main_render_frame_id_,
       base::Bind(&Impl::OnTargetChanged, this));
 
   return true;
@@ -284,14 +295,14 @@ WebContentsAudioInputStream* WebContentsAudioInputStream::Create(
     const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner,
     AudioMirroringManager* audio_mirroring_manager) {
   int render_process_id;
-  int render_view_id;
+  int main_render_frame_id;
   if (!WebContentsCaptureUtil::ExtractTabCaptureTarget(
-          device_id, &render_process_id, &render_view_id)) {
+          device_id, &render_process_id, &main_render_frame_id)) {
     return NULL;
   }
 
   return new WebContentsAudioInputStream(
-      render_process_id, render_view_id,
+      render_process_id, main_render_frame_id,
       audio_mirroring_manager,
       new WebContentsTracker(),
       new media::VirtualAudioInputStream(
@@ -300,11 +311,11 @@ WebContentsAudioInputStream* WebContentsAudioInputStream::Create(
 }
 
 WebContentsAudioInputStream::WebContentsAudioInputStream(
-    int render_process_id, int render_view_id,
+    int render_process_id, int main_render_frame_id,
     AudioMirroringManager* mirroring_manager,
     const scoped_refptr<WebContentsTracker>& tracker,
     media::VirtualAudioInputStream* mixer_stream)
-    : impl_(new Impl(render_process_id, render_view_id,
+    : impl_(new Impl(render_process_id, main_render_frame_id,
                      mirroring_manager, tracker, mixer_stream)) {}
 
 WebContentsAudioInputStream::~WebContentsAudioInputStream() {}