Fix draggable elements are painted at incorrect position

Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158
Committed: https://crrev.com/89317940af3b85bdff2f0e99c35082b5d3d517cf
Cr-Commit-Position: refs/heads/master@{#377852}

[hyunjunekim2, 2016-02-20 08:23:39]

Description was changed from

==========
WIP

BUG=579153
==========

to

==========
WIP

BUG=579153
==========

[hyunjunekim2, 2016-02-20 08:26:27]

Description was changed from

==========
WIP

BUG=579153
==========

to

==========
Fix draggable svg elements are painted at incorrect position



BUG=579153
==========

[hyunjunekim2, 2016-02-20 08:28:05]

Description was changed from

==========
Fix draggable svg elements are painted at incorrect position



BUG=579153
==========

to

==========
Fix draggable svg elements are painted at incorrect position



BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:29:51]

Description was changed from

==========
Fix draggable svg elements are painted at incorrect position



BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position




BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:46:04]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position




BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.



BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:47:52]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.



BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Because don't contain position of them on localToAbsolute function.


BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:50:29]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Because don't contain position of them on localToAbsolute function.


BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use BoundingBox{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants.

BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:51:44]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use BoundingBox{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants.

BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants.

BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-20 08:52:24]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants.

BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

BUG=579153, 579158
==========

[hyunjunekim2, 2016-02-23 14:44:13]

pdr, fs,
Could you check PS2? Thank you :)

[hyunjunekim2, 2016-02-23 14:44:54]

hyunjune.kim@samsung.com changed reviewers:
+ fs@opera.com, pdr@chromium.org

[hyunjunekim2, 2016-02-23 14:44:54]

+ pdr, fs
Could you check PS2? Thank you.

[hyunjunekim2, 2016-02-23 15:18:50]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158
==========

[fs, 2016-02-23 17:27:46]

I'll defer this to pdr, because I know he was poking about this area recently
(and filed both of the bugs! =))

You might want to loop in some input dev. too to make sure they don't mind
having these tests.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:693: if
(dragState().m_dragDataTransfer) {
if (...)
    return ...;
return IntPoint();

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3331: if
(dragState().m_dragType == DragSourceActionDHTML) {
(I wonder why this code isn't in DragController::populateDragDataTransfer like
the code for all the other types...)

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3333: IntRect
boundingIncludingDescendants =
layoutObject->absoluteBoundingBoxRectIncludingDescendants();
I guess this makes sense since it's what LocalFrame::nodeImage uses.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3334: FloatPoint
absPos(boundingIncludingDescendants.x(), boundingIncludingDescendants.y());
No need to do int -> float and then round to int again, so maybe just:

IntSize delta = m_mouseDownPos - boundingIncludingDescendants.location();

?

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:136: IntPoint
dragDataTransferLocation();
...ForTesting()?

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandlerTest.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:178: "[draggable]
{-moz-user-select: none; -khtml-user-select: none; -webkit-user-select: none;"
Including the -moz-* and -khtml-* seems a bit excessive to me.

It might also make sense to set "margin: 0" on body to make the mapping to a
point more obvious?

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:184:
PlatformMouseEvent mouseDownEvent1(
Why the '1' at the end?

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:211: "[draggable]
{-moz-user-select: none; -khtml-user-select: none; -webkit-user-select: none;"
Same as above.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:219:
PlatformMouseEvent mouseDownEvent1(
Same as above.

[hyunjunekim2, 2016-02-24 12:00:07]

fs, I wrote the report.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3331: if
(dragState().m_dragType == DragSourceActionDHTML) {
On 2016/02/23 17:27:45, fs wrote:
> (I wonder why this code isn't in DragController::populateDragDataTransfer like
> the code for all the other types...)

This is the report that is flow to draw dragged image simply.

1) There is |m_dragLoc| on DataTransfer. When call setDragImageElement function,
set |m_dragLoc| that is offset on the image.
2) Call |dataTransfer->createDragImage| on |DragController::startDrag| to get
dragOffset that is
|m_dragLoc| also to create image. And calcultate |dragLocation| with dragOffset.
3) Next call DragController::doSystemDrag with dragLocation and image created.
4) Call DragClientImpl::startDrag with dragLocation and image created.
5) finally call WebViewImpl::startDragging with dragLocation and image created.

> Why this code isn't in DragController::populateDragDataTransfer.
I guess because of 1), 2) processes.

[hyunjunekim2, 2016-02-24 12:04:41]

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3331: if
(dragState().m_dragType == DragSourceActionDHTML) {
Did you say that this codes need to transfer on
|DragController::populateDragDataTransfer|?

[fs, 2016-02-24 12:47:48]

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3331: if
(dragState().m_dragType == DragSourceActionDHTML) {
On 2016/02/24 at 12:04:41, hyunjunekim2 wrote:
> Did you say that this codes need to transfer on
|DragController::populateDragDataTransfer|?

No need to take any action on this - I was mostly thinking out aloud. (If
something were to be done about it, a separate CL would be the way to go
regardless.)

[fs, 2016-02-24 12:51:17]

https://codereview.chromium.org/1716023002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3332: IntRect
absoluteBoundingBox = layoutObject->absoluteBoundingBoxRect();
Why did you switch this to absoluteBoundingBoxRect (from
absoluteBoundingBoxRectIncludingDescendants)?

[hyunjunekim2, 2016-02-24 13:06:37]

fs, Changed them such as PS4.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:693: if
(dragState().m_dragDataTransfer) {
On 2016/02/23 17:27:45, fs wrote:
> if (...)
>     return ...;
> return IntPoint();

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3334: FloatPoint
absPos(boundingIncludingDescendants.x(), boundingIncludingDescendants.y());
On 2016/02/23 17:27:45, fs wrote:
> No need to do int -> float and then round to int again, so maybe just:
> 
> IntSize delta = m_mouseDownPos - boundingIncludingDescendants.location();
> 
> ?

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:136: IntPoint
dragDataTransferLocation();
On 2016/02/23 17:27:45, fs wrote:
> ...ForTesting()?

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandlerTest.cpp (right):

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:178: "[draggable]
{-moz-user-select: none; -khtml-user-select: none; -webkit-user-select: none;"
On 2016/02/23 17:27:45, fs wrote:
> Including the -moz-* and -khtml-* seems a bit excessive to me.
> 
> It might also make sense to set "margin: 0" on body to make the mapping to a
> point more obvious?

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:184:
PlatformMouseEvent mouseDownEvent1(
On 2016/02/23 17:27:45, fs wrote:
> Why the '1' at the end?

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:211: "[draggable]
{-moz-user-select: none; -khtml-user-select: none; -webkit-user-select: none;"
On 2016/02/23 17:27:45, fs wrote:
> Same as above.

Done.

https://codereview.chromium.org/1716023002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandlerTest.cpp:219:
PlatformMouseEvent mouseDownEvent1(
On 2016/02/23 17:27:46, fs wrote:
> Same as above.

Done.

https://codereview.chromium.org/1716023002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1716023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:3332: IntRect
absoluteBoundingBox = layoutObject->absoluteBoundingBoxRect();
On 2016/02/24 12:51:16, fs wrote:
> Why did you switch this to absoluteBoundingBoxRect (from
> absoluteBoundingBoxRectIncludingDescendants)?

|absoluteBoundingBoxRect| function was thought to include both.
But re-change |absoluteBoundingBoxRectIncludingDescendants|.
Because |absoluteBoundingBoxRectIncludingDescendants| function contains
|absoluteBoundingBoxrect| Layer of children.

[hyunjunekim2, 2016-02-24 13:57:40]

fs, pdr
Could you check PS4 and take me advice? Thank you.

[pdr., 2016-02-24 20:51:36]

On 2016/02/24 at 13:57:40, hyunjune.kim wrote:
> fs, pdr
> Could you check PS4 and take me advice? Thank you.

LGTM, this looks great.

Can you remove the TODO in LocalFrame.cpp too?
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[hyunjunekim2, 2016-02-24 22:47:15]

On 2016/02/24 20:51:36, pdr wrote:
> On 2016/02/24 at 13:57:40, hyunjune.kim wrote:
> > fs, pdr
> > Could you check PS4 and take me advice? Thank you.
> 
> LGTM, this looks great.
> 
> Can you remove the TODO in LocalFrame.cpp too?
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Done.

[pdr., 2016-02-25 05:28:08]

The CQ bit was checked by pdr@chromium.org

[pdr., 2016-02-25 05:28:09]

lgtm

[commit-bot: I haz the power, 2016-02-25 05:29:29]

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

[commit-bot: I haz the power, 2016-02-25 08:22:59]

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

[commit-bot: I haz the power, 2016-02-25 08:23:00]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hyunjunekim2, 2016-02-25 12:22:42]

hyunjune.kim@samsung.com changed reviewers:
+ rbyers@chromium.org

[hyunjunekim2, 2016-02-25 14:40:02]

fs, pdr I changed this patch about test. added 'font-family' and modified
font-size that is even.
Rick Byers, Also could you check this patch? 
Thank you.

[Rick Byers, 2016-02-25 16:30:43]

rbyers@chromium.org changed reviewers:
+ dtapuska@chromium.org

[Rick Byers, 2016-02-25 16:30:49]

dtapuska@ knows this code as well as I do (and you already have a core OWNERS
LGTM).  Dave can you please take a look?

[Rick Byers, 2016-02-25 16:31:13]

rbyers@chromium.org changed reviewers:
- rbyers@chromium.org

[dtapuska, 2016-02-25 19:31:38]

On 2016/02/25 16:30:49, Rick Byers wrote:
> dtapuska@ knows this code as well as I do (and you already have a core OWNERS
> LGTM).  Dave can you please take a look?

lgtm

[hyunjunekim2, 2016-02-26 06:47:21]

pdr,
After Issue 1736893002 was landing, i will land this patch.
Thank you.

[pdr., 2016-02-26 06:52:28]

On 2016/02/26 at 06:47:21, hyunjune.kim wrote:
> pdr,
> After Issue 1736893002 was landing, i will land this patch.
> Thank you.

Oh no, you win this race since mine isn't even reviewed yet :)

Please go ahead and land this, and I'll rebase my patch tomorrow. I confirmed
the patches work well together too.

[hyunjunekim2, 2016-02-26 06:56:22]

The CQ bit was checked by hyunjune.kim@samsung.com

[hyunjunekim2, 2016-02-26 06:56:23]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
dtapuska@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1716023002/#ps220001
(title: "Remove comment.")

[commit-bot: I haz the power, 2016-02-26 06:57:19]

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

[hyunjunekim2, 2016-02-26 06:57:32]

On 2016/02/26 06:52:28, pdr wrote:
> On 2016/02/26 at 06:47:21, hyunjune.kim wrote:
> > pdr,
> > After Issue 1736893002 was landing, i will land this patch.
> > Thank you.
> 
> Oh no, you win this race since mine isn't even reviewed yet :)
> 
> Please go ahead and land this, and I'll rebase my patch tomorrow. I confirmed
> the patches work well together too.

Ok, Go ahead. Thank you.

[commit-bot: I haz the power, 2016-02-26 07:24:58]

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

[commit-bot: I haz the power, 2016-02-26 07:24:59]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pdr., 2016-02-26 07:51:31]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-02-26 07:52:23]

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

[commit-bot: I haz the power, 2016-02-26 08:19:45]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158
==========

[commit-bot: I haz the power, 2016-02-26 08:19:46]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-02-26 08:21:03]

Description was changed from

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158
==========

to

==========
Fix draggable {svg, inline} elements are painted at incorrect position

When drag {SVGShape, Inline} elements, It are painted at incorrect position.
Don't contain position of them on localToAbsolute function.
Because use bounding{x, y} of them that return on
absoluteBoundingBoxRectIncludingDescendants as offset.

TEST=EventHandlerTest.cpp

BUG=579153, 579158

Committed: https://crrev.com/89317940af3b85bdff2f0e99c35082b5d3d517cf
Cr-Commit-Position: refs/heads/master@{#377852}
==========

[commit-bot: I haz the power, 2016-02-26 08:21:05]

Patchset 12 (id:??) landed as
https://crrev.com/89317940af3b85bdff2f0e99c35082b5d3d517cf
Cr-Commit-Position: refs/heads/master@{#377852}