WebPluginContainer: expose and implement localToScreenRect

WebPluginContainer: expose and implement localToScreenRect

A plugin implementation may want to query its position in screen
coordinates.

R=esprehn@chromium.org, japhet@chromium.org
BUG=592031

[Julien Isorce Samsung, 2016-03-04 17:17:28]

Description was changed from

==========
WebPluginContainer: expose and implement localToScreenRect

A plugin implementation may want to query its position in screen
coordinates.

R=

BUG=592031
==========

to

==========
WebPluginContainer: expose and implement localToScreenRect

A plugin implementation may want to query its position in screen
coordinates.

R=esprehn@chromium.org, japhet@chromium.org

BUG=592031
==========

[Julien Isorce Samsung, 2016-03-04 17:17:28]

j.isorce@samsung.com changed reviewers:
+ esprehn@chromium.org, japhet@chromium.org

[Julien Isorce Samsung, 2016-03-10 21:00:22]

ping ? Maybe I can add a unit test somewhere, please advise where. Thx

[esprehn, 2016-03-10 21:09:50]

What plugin? We don't speculatively add plugin APIs. This doesn't seem like
something we should add.

[chrishtr, 2016-03-10 21:11:53]

chrishtr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2016-03-10 21:11:53]

Why is it needed for a plugin to query its screen coordinates? The bug or CL
don't explain
a use case.

[Julien Isorce Samsung, 2016-03-11 18:22:02]

On 2016/03/10 21:09:50, esprehn wrote:
> What plugin? We don't speculatively add plugin APIs. This doesn't seem like
> something we should add.

On 2016/03/10 21:11:53, chrishtr wrote:
> Why is it needed for a plugin to query its screen coordinates? The bug or CL
> don't explain
> a use case.

Sure. So there is a use case where an app just wants to act as a web page that
overlays the TV video broadcast.
The video geometry can only be controlled by IPC and is always on the back, i.e.
any app will be show on top of the video.
In order to properly place the video to an object tag rectangle, it needs to
know the tag object geometry in the screen coordinates.
(There is no way to instruct the video broadcast player to render to a
particular Window or surface)

Problem is that in blink::WebPlugin, none of the rect provided by the following
updateGeometry are in screen coordinates:

void updateGeometry(const blink::WebRect& window_rect,
                      const blink::WebRect& clip_rect,
                      const blink::WebRect& unobscured_rect,
                      const blink::WebVector<blink::WebRect>& cut_outs_rects,
                      bool is_visible) override;

I checked also with
http://www.chromium.org/developers/design-documents/blink-coordinate-spaces.

But maybe I missed something and it is possible to deduce its position in the
screen coordinates somehow from that function above.

Just as a note, the function localToScreenRect that this CL expose, at some
point calls ChromeClientImpl::windowRect() -> m_webView->client()->windowRect();
which then through IPC triggers a call to:
// Retrieve the bounds of the View, in screen coordinates.
virtual gfx::Rect RenderWidgetHostView::GetViewBounds();

which one call aura::Window::GetBoundsInScreen.

Let me know if I missed something. Thx.

[chrishtr, 2016-03-11 18:27:02]

On 2016/03/11 at 18:22:02, j.isorce wrote:
> On 2016/03/10 21:09:50, esprehn wrote:
> > What plugin? We don't speculatively add plugin APIs. This doesn't seem like
> > something we should add.
> 
> On 2016/03/10 21:11:53, chrishtr wrote:
> > Why is it needed for a plugin to query its screen coordinates? The bug or CL
> > don't explain
> > a use case.
> 
> Sure. So there is a use case where an app just wants to act as a web page that
overlays the TV video broadcast.
> The video geometry can only be controlled by IPC and is always on the back,
i.e. any app will be show on top of the video.
> In order to properly place the video to an object tag rectangle,

What do you mean by this? I don't understand the use case of "placing the video
to an object tag rectangle". Didn't you say above that the video is full-screen
behind the app overlays?

 it needs to know the tag object geometry in the screen coordinates.
> (There is no way to instruct the video broadcast player to render to a
particular Window or surface)

[Julien Isorce Samsung, 2016-03-14 21:16:45]

On 2016/03/11 18:27:02, chrishtr wrote:
> What do you mean by this? I don't understand the use case of "placing the
video
> to an object tag rectangle". Didn't you say above that the video is
full-screen
> behind the app overlays?
> 

Hi chrishtr, I do not recall saying the video is fullscreen but maybe I am
mistaken. So the video broadcast is not necessarily full screen.
Internally the plugin should notify the video broadcast plugin of its geometry
on the screen. Indeed the object tag with a specific mimetype represents a video
broadcast, so its position on the web view is where the video broadcast should
show. 

Since the top left corner of the web view does not match the top left corner of
the screen, as far as I know there is currently no way to retrieve the object
tag position in screen coordinates from blink::WebPlugin::updateGeometry. Thx in
advance for your lights.

[esprehn, 2016-03-15 00:07:40]

Description was changed from

==========
WebPluginContainer: expose and implement localToScreenRect

A plugin implementation may want to query its position in screen
coordinates.

R=esprehn@chromium.org, japhet@chromium.org

BUG=592031
==========

to

==========
WebPluginContainer: expose and implement localToScreenRect

A plugin implementation may want to query its position in screen
coordinates.

R=esprehn@chromium.org, japhet@chromium.org
BUG=592031
==========

[esprehn, 2016-03-15 00:09:06]

Without any tests, and without any web exposed consumers, I don't think this
should be added to the public API of blink. Indeed someone will just come along
and delete this as dead code since nothing uses it. The public API of blink is
not considered something plugins should be consuming directly.

[Julien Isorce Samsung, 2016-03-15 22:02:25]

On 2016/03/15 00:09:06, esprehn wrote:
> Without any tests, and without any web exposed consumers, I don't think this
> should be added to the public API of blink. Indeed someone will just come
along
> and delete this as dead code since nothing uses it. 

Thx for your reply. Right I see, I could try to make a unit test if this is the
only blocking thing but I am not sure not sure if it is doable (maybe placing
the window on the screen first like "TEST_F(RenderWidgetHostViewAuraTest,
ParentMovementUpdatesScreenRect)".
I compared with existing API blink::WebPluginContainer::rootFrameToLocalPoint,
it is only used from WebPluginContainerTest.

> The public API of blink is not considered something plugins should be
consuming directly.

Ok, indeed I can see that for example "blink::WebPluginContainer* container =
plugin()->container()" is almost only used from PluginPlaceholder.
And in the first place maybe the screen information should be provided by 
void updateGeometry(const blink::WebRect& window_rect,
                      const blink::WebRect& clip_rect,
                      const blink::WebRect& unobscured_rect,
                      const blink::WebVector<blink::WebRect>& cut_outs_rects,
                      bool is_visible) override;


Also it might be another way to get screen coordinates, by using
RenderFrameObserver::OnMessageReceived and intercepting
ViewMsg_UpdateScreenRects IPC.

[chrishtr, 2016-03-15 23:01:00]

On 2016/03/14 at 21:16:45, j.isorce wrote:
> On 2016/03/11 18:27:02, chrishtr wrote:
> > What do you mean by this? I don't understand the use case of "placing the
video
> > to an object tag rectangle". Didn't you say above that the video is
full-screen
> > behind the app overlays?
> > 
> 
> Hi chrishtr, I do not recall saying the video is fullscreen but maybe I am
mistaken. So the video broadcast is not necessarily full screen.
> Internally the plugin should notify the video broadcast plugin of its geometry
on the screen. Indeed the object tag with a specific mimetype represents a video
broadcast, so its position on the web view is where the video broadcast should
show. 
> 
> Since the top left corner of the web view does not match the top left corner
of the screen, as far as I know there is currently no way to retrieve the object
tag position in screen coordinates from blink::WebPlugin::updateGeometry. Thx in
advance for your lights.

It looks like you want a plugin that paints in a frame that is not the root
frame of the WebView (e.g. an iframe) to be able to paint relative to the root
frame of the WebView.
I don't think that is a good feature to add. It breaks the notion of a
hierarchical web page. If you need the plugin to paint in the root frame's
coordinate space, then the plugin should go in that frame.

[Julien Isorce Samsung, 2016-03-29 10:43:36]

Hi, I realized that localToRootFramePoint is enough since I can compute
rootFrame point to screen coordinates from the app itself. Since the app owns
the web view and it knows its position on the screen. I going to delete this CL
since it is not revelant. Thx for your time.

[chrishtr, 2016-03-29 17:24:16]

On 2016/03/29 at 10:43:36, j.isorce wrote:
> Hi, I realized that localToRootFramePoint is enough since I can compute
rootFrame point to screen coordinates from the app itself. Since the app owns
the web view and it knows its position on the screen. I going to delete this CL
since it is not revelant. Thx for your time.

Cool.