Allow TabCapture to capture full screen flash widgets.

Allow the chrome.tabCapture.* extension API to capture full screen flash widgets.

When flash goes fullscreen, it creates a new render widget, so we need to track that in WebContentsVideoCaptureDevice and snapshot that instead of the current render view (which is being covered by the fullscreen render widget).

- Add new WebContentsObserver event to be notified of a fullscreen render widget being shown/destroyed.
- Add a new WebContentsImpl method to query if there is a currently active fullscreen render widget.

BUG=154135
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=178457

[justinlin, 2013-01-19 01:17:52]

Hi jam@, ptal the WebContents{Observer} changes. The proposal is these 2
changes:
- Add a WebContentsObserver::On{Show,Destroy}FullscreenWidget events
- Add a WebContents::GetFullscreenWidgetRoutingId() method

The second one supersedes the first one, so if that's OK, then the events are
not needed. If only the events are OK, then I'll have to find a way to somehow
query the initial state (without the WebContents::GetFullscreenWidgetRoutingId()
method, we won't realize that we're already fullscreen when we start
capturing/mirroring the tab until we get a show/hide fullscreen event).

FYI, capturing/mirroring refers to using the tabCapture extension API to get a
video and audio stream of a specified tab, which can be sent over webRTC.

miu@: PTAL at the web_contents_capture_device.cc changes.

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
File content/browser/renderer_host/media/web_contents_video_capture_device.cc
(right):

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
content/browser/renderer_host/media/web_contents_video_capture_device.cc:341: if
(fullscreen_widget_id_ == MSG_ROUTING_NONE)
These two lines go away if adding a WebContents::GetFullscreenWidgetRoutingID()
method doesn't make sense.

[miu, 2013-01-19 02:14:47]

lgtm

Just one nit...

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
File content/browser/renderer_host/media/web_contents_video_capture_device.cc
(right):

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
content/browser/renderer_host/media/web_contents_video_capture_device.cc:341: if
(fullscreen_widget_id_ == MSG_ROUTING_NONE)
On 2013/01/19 01:17:52, justinlin wrote:
> These two lines go away if adding a
WebContents::GetFullscreenWidgetRoutingID()
> method doesn't make sense.

Put these two lines at the end of LookUpAndObserveWebContents(), since they only
need to be executed once.  The observer callbacks should take care of updating
fullscreen_widget_id_ from then onwards.

[justinlin, 2013-01-19 02:19:09]

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
File content/browser/renderer_host/media/web_contents_video_capture_device.cc
(right):

https://codereview.chromium.org/12026029/diff/1009/content/browser/renderer_h...
content/browser/renderer_host/media/web_contents_video_capture_device.cc:341: if
(fullscreen_widget_id_ == MSG_ROUTING_NONE)
On 2013/01/19 02:14:47, Yuri wrote:
> On 2013/01/19 01:17:52, justinlin wrote:
> > These two lines go away if adding a
> WebContents::GetFullscreenWidgetRoutingID()
> > method doesn't make sense.
> 
> Put these two lines at the end of LookUpAndObserveWebContents(), since they
only
> need to be executed once.  The observer callbacks should take care of updating
> fullscreen_widget_id_ from then onwards.

Yes, good point! Done.

[jam, 2013-01-23 02:26:53]

i'm not familiar with this feature, and the linked bug has no information (what
is chrome.dial?).

can you provide links to what this feature is for? I'd like to understand if
this belongs in content or in a different module. see
http://www.chromium.org/developers/content-module/content-api and
http://www.chromium.org/developers/content-module

https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
content/public/browser/web_contents.h:155: virtual int
GetFullscreenWidgetRoutingID() const = 0;
we only add stuff to the public interface if it's needed by other modules. since
this is only called from within content, add this on to WebContentsImpl (and
feel free to cast to it if necessary)

[justinlin, 2013-01-23 02:42:00]

On 2013/01/23 02:26:53, John Abd-El-Malek wrote:
> i'm not familiar with this feature, and the linked bug has no information
(what
> is chrome.dial?).
> 
> can you provide links to what this feature is for? I'd like to understand if
> this belongs in content or in a different module. see
> http://www.chromium.org/developers/content-module/content-api and
> http://www.chromium.org/developers/content-module
> 
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> File content/public/browser/web_contents.h (right):
> 
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> content/public/browser/web_contents.h:155: virtual int
> GetFullscreenWidgetRoutingID() const = 0;
> we only add stuff to the public interface if it's needed by other modules.
since
> this is only called from within content, add this on to WebContentsImpl (and
> feel free to cast to it if necessary)

Ah, sorry! I linked the wrong bug. Updated.

It's for the tabCapture feature:
http://developer.chrome.com/trunk/extensions/tabCapture.html

This is exposed in Chrome as an extension API, but it's really more of a
WebRTC-ish/content feature since we use mostly the same paths but implement our
own audio/video capture devices instead of using webcam/mic.

You're right, doesn't need to be in the public interface, will update that (for
some reason, I thought I was using this from chrome somewhere).

[justinlin, 2013-01-23 02:53:00]

PTAL, I think this is generic enough to belong in content? It just exposes the
routing ID of fullscreen render widgets (mostly just PPAPI flash, I think).

https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
content/public/browser/web_contents.h:155: virtual int
GetFullscreenWidgetRoutingID() const = 0;
On 2013/01/23 02:26:53, John Abd-El-Malek wrote:
> we only add stuff to the public interface if it's needed by other modules.
since
> this is only called from within content, add this on to WebContentsImpl (and
> feel free to cast to it if necessary)

Done.

[jam, 2013-01-23 05:28:09]

On 2013/01/23 02:42:00, justinlin wrote:
> On 2013/01/23 02:26:53, John Abd-El-Malek wrote:
> > i'm not familiar with this feature, and the linked bug has no information
> (what
> > is chrome.dial?).
> > 
> > can you provide links to what this feature is for? I'd like to understand if
> > this belongs in content or in a different module. see
> > http://www.chromium.org/developers/content-module/content-api and
> > http://www.chromium.org/developers/content-module
> > 
> >
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> > File content/public/browser/web_contents.h (right):
> > 
> >
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> > content/public/browser/web_contents.h:155: virtual int
> > GetFullscreenWidgetRoutingID() const = 0;
> > we only add stuff to the public interface if it's needed by other modules.
> since
> > this is only called from within content, add this on to WebContentsImpl (and
> > feel free to cast to it if necessary)
> 
> Ah, sorry! I linked the wrong bug. Updated.
> 
> It's for the tabCapture feature:
> http://developer.chrome.com/trunk/extensions/tabCapture.html
> 
> This is exposed in Chrome as an extension API, but it's really more of a
> WebRTC-ish/content feature since we use mostly the same paths but implement
our
> own audio/video capture devices instead of using webcam/mic.
> 
> You're right, doesn't need to be in the public interface, will update that
(for
> some reason, I thought I was using this from chrome somewhere).

hmm, usually the code needed to implement extension APIs (a chrome feature) is
located in src\chrome. Why is the code for this in src\content and not
src\chrome?

[justinlin, 2013-01-23 06:14:04]

On 2013/01/23 05:28:09, John Abd-El-Malek wrote:
> On 2013/01/23 02:42:00, justinlin wrote:
> > On 2013/01/23 02:26:53, John Abd-El-Malek wrote:
> > > i'm not familiar with this feature, and the linked bug has no information
> > (what
> > > is chrome.dial?).
> > > 
> > > can you provide links to what this feature is for? I'd like to understand
if
> > > this belongs in content or in a different module. see
> > > http://www.chromium.org/developers/content-module/content-api and
> > > http://www.chromium.org/developers/content-module
> > > 
> > >
> >
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> > > File content/public/browser/web_contents.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/12026029/diff/8001/content/public/browser/web...
> > > content/public/browser/web_contents.h:155: virtual int
> > > GetFullscreenWidgetRoutingID() const = 0;
> > > we only add stuff to the public interface if it's needed by other modules.
> > since
> > > this is only called from within content, add this on to WebContentsImpl
(and
> > > feel free to cast to it if necessary)
> > 
> > Ah, sorry! I linked the wrong bug. Updated.
> > 
> > It's for the tabCapture feature:
> > http://developer.chrome.com/trunk/extensions/tabCapture.html
> > 
> > This is exposed in Chrome as an extension API, but it's really more of a
> > WebRTC-ish/content feature since we use mostly the same paths but implement
> our
> > own audio/video capture devices instead of using webcam/mic.
> > 
> > You're right, doesn't need to be in the public interface, will update that
> (for
> > some reason, I thought I was using this from chrome somewhere).
> 
> hmm, usually the code needed to implement extension APIs (a chrome feature) is
> located in src\chrome. Why is the code for this in src\content and not
> src\chrome?

The reason this is in content is because it's deeply coupled with the WebRTC
feature, which lives in content as well. We just use a
web_contents_capture_device (seen in this cl), instead of the regular
video_capture_device.

The reason the interface for this is exposed as an extension API right now is
mostly for security reasons. We don't want to open this up to regular websites
yet (like WebRTC, which can be invoked using regular js in any page), so we're
exposing it as a Chrome extension API for now. Eventually, when this gets more
stable/safer, perhaps we would look into exposing this directly through JS like
WebRTC.

The chrome side calls into the content side via a js hook. We also have
validation to make sure the tabCapture request came from a chrome API call
instead of directly from js.

This current CL is just an addition to the web_contents_capture_device (which
repeatedly snapshots the current tab and serves that as a video stream), so that
it correctly finds and snapshots any fullscreen flash video when activated
(otherwise it'd be snapshotting the active web_contents, but it's being covered
by the fullscreen flash video render widget).

[jam, 2013-01-23 21:22:50]

thanks for the explanation over vc. i'm ok with this being in content if this
stuff will be in webrtc later

lgtm

https://codereview.chromium.org/12026029/diff/11004/content/public/browser/we...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/12026029/diff/11004/content/public/browser/we...
content/public/browser/web_contents_observer.h:155: // Invoked when a plugin
like PepperFlash creates and shows or destroys a
nit: don't mention PepperFlash directly, but just say pepper plugin

[justinlin, 2013-01-23 22:20:10]

thanks

https://codereview.chromium.org/12026029/diff/11004/content/public/browser/we...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/12026029/diff/11004/content/public/browser/we...
content/public/browser/web_contents_observer.h:155: // Invoked when a plugin
like PepperFlash creates and shows or destroys a
On 2013/01/23 21:22:50, John Abd-El-Malek wrote:
> nit: don't mention PepperFlash directly, but just say pepper plugin

Done.

[commit-bot: I haz the power, 2013-01-23 22:22:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/justinlin@chromium.org/12026029/23003

[commit-bot: I haz the power, 2013-01-24 01:21:55]

Change committed as 178457