[UseZoomForDSF] Alwatys use DIP coordinates in pepper plugin

[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
Committed: https://crrev.com/be76caa53f0ca1afbd65fcff6a2486e10895bd64
Cr-Commit-Position: refs/heads/master@{#371769}

[oshima, 2016-01-20 10:16:57]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-20 10:17:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1609193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1609193002/1

[commit-bot: I haz the power, 2016-01-20 11:39:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-20 11:39:07]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-01-20 23:39:31]

Description was changed from

==========
Use regular DSF path for ppapi

BUG=485650
==========

to

==========
[UseZoomForDSF] Use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

BUG=485650
==========

[oshima, 2016-01-20 23:39:31]

oshima@chromium.org changed reviewers:
+ bbudge@chromium.org

[oshima, 2016-01-20 23:42:37]

Description was changed from

==========
[UseZoomForDSF] Use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

BUG=485650
==========

to

==========
[UseZoomForDSF] Use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

[oshima, 2016-01-20 23:43:03]

Description was changed from

==========
[UseZoomForDSF] Use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

to

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

[oshima, 2016-01-20 23:43:21]

Description was changed from

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

to

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

[bbudge, 2016-01-21 00:34:52]

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1086: // If it is never
set by the plugin, use the bottom left corner.
Comment seems misplaced. Should it be before setting rect?

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1090: // TODO(kinaba)
Take CSS transformation into accont.
sp. account

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1091: // TODO(kinaba)
Take bounding_box into account. On some platforms, an
It's not clear to me what 'bounding_box' is referring to here.

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1270:
render_frame()->GetRenderWidget()->convertWindowToViewport(1.0f);
nit: Mixing 1.0f and 1.f.

1.0f seems more consistent with the existing code. Perhaps change 1.f literals
to 1.0f?

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.h (right):

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.h:376: void
ConvertDIPToViewport(gfx::Rect* rect) const;
Is there a reason these are public?

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.h:930: float
viewport_to_dip_scale_;
nit: Maybe it would be better to group this with other viewport related fields
(view_data_).

[oshima, 2016-01-21 00:46:40]

Description was changed from

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

to

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

[oshima, 2016-01-21 01:02:16]

Patchset #3 (id:40001) has been deleted

[oshima, 2016-01-21 01:12:07]

thank you for prompt review! PTAL

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1086: // If it is never
set by the plugin, use the bottom left corner.
On 2016/01/21 00:34:51, bbudge wrote:
> Comment seems misplaced. Should it be before setting rect?

Done. I also removed else (I was going to fix it before sending but forgot to do
so)

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1090: // TODO(kinaba)
Take CSS transformation into accont.
On 2016/01/21 00:34:51, bbudge wrote:
> sp. account

Done.

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1091: // TODO(kinaba)
Take bounding_box into account. On some platforms, an
On 2016/01/21 00:34:51, bbudge wrote:
> It's not clear to me what 'bounding_box' is referring to here.

I believe kinaba@ meant |text_input_caret_bounds_|. I updated the comment.

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1270:
render_frame()->GetRenderWidget()->convertWindowToViewport(1.0f);
On 2016/01/21 00:34:51, bbudge wrote:
> nit: Mixing 1.0f and 1.f.
> 
> 1.0f seems more consistent with the existing code. Perhaps change 1.f literals
> to 1.0f?

Done

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.h (right):

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.h:376: void
ConvertDIPToViewport(gfx::Rect* rect) const;
On 2016/01/21 00:34:52, bbudge wrote:
> Is there a reason these are public?

Nope, thank you for the catch. Moved to private/bottom.

https://codereview.chromium.org/1609193002/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.h:930: float
viewport_to_dip_scale_;
On 2016/01/21 00:34:51, bbudge wrote:
> nit: Maybe it would be better to group this with other viewport related fields
> (view_data_).

Done.

[bbudge, 2016-01-21 17:43:04]

content/renderer/pepper LGTM

[oshima, 2016-01-21 18:03:19]

oshima@chromium.org changed reviewers:
+ tdresser@chromium.org

[oshima, 2016-01-21 18:03:20]

tdresser@ -> ui/events/blink, content/browser/renderer_host/input OWNERS

I moved the utility function from content/browser/renderer_host/input to
ui/events/blink so that it can also be used in renderer/ppapi.

[tdresser, 2016-01-21 21:14:52]

LGTM with nits.

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/input_router_impl.cc:422: // This coversion
is temporary. WebInputEvent should be generated
coversion -> conversion

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/input_router_impl.cc:423: // directly
fromui::Event with the viewport coordinates. See
fromui -> from ui

https://codereview.chromium.org/1609193002/diff/60001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/60001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1148:
ui::ScaleWebInputEvent(event, viewport_to_dip_scale_));
Do we need a scoped_ptr here?

const WebInputEvent event_in_dip = *ui::ScaleWebInputEvent(...)

[oshima, 2016-01-22 01:22:23]

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/input_router_impl.cc:422: // This coversion
is temporary. WebInputEvent should be generated
On 2016/01/21 21:14:51, tdresser wrote:
> coversion -> conversion

Done.

https://codereview.chromium.org/1609193002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/input_router_impl.cc:423: // directly
fromui::Event with the viewport coordinates. See
On 2016/01/21 21:14:51, tdresser wrote:
> fromui -> from ui

Done.

https://codereview.chromium.org/1609193002/diff/60001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/1609193002/diff/60001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1148:
ui::ScaleWebInputEvent(event, viewport_to_dip_scale_));
On 2016/01/21 21:14:51, tdresser wrote:
> Do we need a scoped_ptr here?
> 
> const WebInputEvent event_in_dip = *ui::ScaleWebInputEvent(...)

yes because it needs to be deleted if the event was converted, which creates new
instance because the event is passed as a const.

[oshima, 2016-01-22 01:22:38]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 01:23:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1609193002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1609193002/80001

[oshima, 2016-01-22 01:32:29]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2016-01-22 01:33:06]

On 2016/01/22 01:23:52, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1609193002/80001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1609193002/80001

cancelled because the CL this depends hasn't landed yet. my bad.

[oshima, 2016-01-27 09:30:57]

The CQ bit was checked by oshima@chromium.org

[oshima, 2016-01-27 09:30:58]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org,
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1609193002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-01-27 09:32:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1609193002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1609193002/100001

[commit-bot: I haz the power, 2016-01-27 10:35:49]

Description was changed from

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

to

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

[commit-bot: I haz the power, 2016-01-27 10:35:50]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-01-27 10:37:13]

Description was changed from

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650
==========

to

==========
[UseZoomForDSF] Always use DIP coordinates in pepper plugin

Pepper API is device scale factor aware, so we need to convert the coordinates
between viewport and DIP, when use-zoom-for-dsf mode is enabled.
* Convert events and bounds from the Viewport to DIP.
* Convert caret bounds from DIP to Viewport.
* Moved the utility function to scale WebInputEvent  to ui/events/blink

This CL depends on https://codereview.chromium.org/1603253003/

BUG=485650

Committed: https://crrev.com/be76caa53f0ca1afbd65fcff6a2486e10895bd64
Cr-Commit-Position: refs/heads/master@{#371769}
==========

[commit-bot: I haz the power, 2016-01-27 10:37:14]

Patchset 5 (id:??) landed as
https://crrev.com/be76caa53f0ca1afbd65fcff6a2486e10895bd64
Cr-Commit-Position: refs/heads/master@{#371769}