Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961, 497750, 519603
Committed: https://crrev.com/a31198efcc43869580990328f822d090d2a8a2a8
Cr-Commit-Position: refs/heads/master@{#368607}

[zino, 2016-01-05 10:31:37]

Description was changed from

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.

BUG=328961,497750,519603
==========

to

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.

BUG=328961,497750,519603
==========

[zino, 2016-01-05 10:31:37]

jinho.bang@samsung.com changed reviewers:
+ junov@chromium.org

[zino, 2016-01-05 10:36:14]

Hi Justin,

I'm working on this issue and I'm rewriting exisiting tests for hit region.
I'm not sure but current tests seems incorrect.
(so, current implementation doesn't work well on android and highDPI device.)

Thank you and happy new year!

[Justin Novosad, 2016-01-05 16:36:26]

On 2016/01/05 10:36:14, zino wrote:
> Hi Justin,
> 
> I'm working on this issue and I'm rewriting exisiting tests for hit region.
> I'm not sure but current tests seems incorrect.
> (so, current implementation doesn't work well on android and highDPI device.)
> 
> Thank you and happy new year!

Glad to hear this.  Can't wait to get this shipped!

[Justin Novosad, 2016-01-05 16:48:39]

Regarding the tests, you can use window.internals.setPageScaleFactor(2.0) to
simulate a high dpi device in a layout test.

https://codereview.chromium.org/1553373002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/canvas2d/EventHitRegion.cpp (right):

https://codereview.chromium.org/1553373002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/EventHitRegion.cpp:36: height -=
lbmo->borderAndPaddingHeight();
Isn't there a method that already computes these width and height values for us
already? I would expect LayoutBox::content{Width|Height}() to provide the values
you are looking for.

[zino, 2016-01-05 18:48:51]

On 2016/01/05 16:36:26, Justin Novosad wrote:
> Glad to hear this.  Can't wait to get this shipped!

Hi Justin,

So, would you mind if I follow this up?

It may sound like an excuse, but I couldn't work on Chromium/Blink project last
year.
I had to work on infra related work such as telemetry and buildbot and so on.
(and I got a herniated cervical disc)
But I can enjoy the Chromium project from now (from 2016) again because my team
was changed.

It's late but I want to finish this work.
But if I'm interrupting your work, I can reassign other people at Google.
If not so, I will finish the work (hit region shipping) as soon as possible.
(or If you or other people help me for 517521 issue in parallel, we can ship
this faster.)

[zino, 2016-01-06 17:20:53]

Description was changed from

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.

BUG=328961,497750,519603
==========

to

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961,497750,519603
==========

[zino, 2016-01-06 17:22:31]

Please take a look.

Thank you.

https://codereview.chromium.org/1553373002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/canvas2d/EventHitRegion.cpp (right):

https://codereview.chromium.org/1553373002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/EventHitRegion.cpp:36: height -=
lbmo->borderAndPaddingHeight();
On 2016/01/05 16:48:39, Justin Novosad wrote:
> Isn't there a method that already computes these width and height values for
us
> already? I would expect LayoutBox::content{Width|Height}() to provide the
values
> you are looking for.

Done.

[Justin Novosad, 2016-01-07 17:30:47]

On 2016/01/05 18:48:51, zino wrote:
> On 2016/01/05 16:36:26, Justin Novosad wrote:
> > Glad to hear this.  Can't wait to get this shipped!
> 
> Hi Justin,
> 
> So, would you mind if I follow this up?
> 
> It may sound like an excuse, but I couldn't work on Chromium/Blink project
last
> year.
> I had to work on infra related work such as telemetry and buildbot and so on.
> (and I got a herniated cervical disc)
> But I can enjoy the Chromium project from now (from 2016) again because my
team
> was changed.
> 
> It's late but I want to finish this work.
> But if I'm interrupting your work, I can reassign other people at Google.
> If not so, I will finish the work (hit region shipping) as soon as possible.
> (or If you or other people help me for 517521 issue in parallel, we can ship
> this faster.)

Glad to have you back with us!

[Justin Novosad, 2016-01-07 17:31:04]

https://codereview.chromium.org/1553373002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-hit-regions-css-transform-test.html
(right):

https://codereview.chromium.org/1553373002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-hit-regions-css-transform-test.html:17:
padding : 100px;
It would make sense separate this test into individual  tests (or perhaps one
test with many steps), in order to test these different styles independently
from one another.

[zino, 2016-01-11 13:49:12]

Patchset #5 (id:80001) has been deleted

[zino, 2016-01-11 13:49:18]

Patchset #5 (id:100001) has been deleted

[zino, 2016-01-11 13:53:23]

Thank you for review.

I separated the css test into many steps.

https://codereview.chromium.org/1553373002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-hit-regions-css-transform-test.html
(right):

https://codereview.chromium.org/1553373002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/canvas-hit-regions-css-transform-test.html:17:
padding : 100px;
On 2016/01/07 17:31:04, Justin Novosad wrote:
> It would make sense separate this test into individual  tests (or perhaps one
> test with many steps), in order to test these different styles independently
> from one another.

Done.

[Justin Novosad, 2016-01-11 15:43:23]

lgtm

[zino, 2016-01-11 15:57:49]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2016-01-11 15:57:53]

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

[commit-bot: I haz the power, 2016-01-11 17:16:17]

Description was changed from

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961,497750,519603
==========

to

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961,497750,519603
==========

[commit-bot: I haz the power, 2016-01-11 17:16:18]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-01-11 17:17:04]

Description was changed from

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961,497750,519603
==========

to

==========
Canvas2d: Fix incorrect alignment of hit region when applied css/zoom.

This change is fixing incorrect alignment of hit region on canvas when applied
css transform/width/height/padding/border or device pinch-zoom or high DPI.
Also, this CL adds/rewrites related tests.

BUG=328961,497750,519603

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

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

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