Move LocalFrame::DragImageForSelection DragController

Move LocalFrame::DragImageForSelection DragController

DragImageForSelection do not use any core functionalities of LocalFrame,
so DragImageForSelection can be moved to DragController.

BUG=727981

[tanvir, 2017-06-01 10:56:44]

tanvir.rizvi@samsung.com changed reviewers:
+ xiaochengh@chromium.org

[tanvir, 2017-06-01 10:56:45]

PTAL!!
Thanks

[tanvir, 2017-06-01 11:05:08]

LocalFrame has class DraggedNodeImageBuilder which is also the core
functionality for Drag, and is currently used by  LocalFrame::NodeImage.
LocalFrame::NodeImage is called from DataTransfer::CreateDragImage, which is
again triggered from DragController. So should we move all the drag
functionality away from LocalFrame().
Please let me know your opinion.
Thanks

[Xiaocheng, 2017-06-01 22:31:28]

On 2017/06/01 at 11:05:08, tanvir.rizvi wrote:
> LocalFrame has class DraggedNodeImageBuilder which is also the core
functionality for Drag, and is currently used by  LocalFrame::NodeImage.
> LocalFrame::NodeImage is called from DataTransfer::CreateDragImage, which is
again triggered from DragController. So should we move all the drag
functionality away from LocalFrame().
> Please let me know your opinion.
> Thanks

Hmm... I haven't check all dependencies yet.

If that is the case, all drag-related code in LocalFrame.cpp should be moved to
DragController if DragController is the only user.

Could you check if that is doable, and if so, upload an all-in-one patch for
discussion?

Thanks!

[tanvir, 2017-06-02 09:36:11]

Updated the Patch by moving Drag related functions and features outside of
LocalFrame.
PTAL!!!

[Xiaocheng, 2017-06-02 17:54:31]

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

[commit-bot: I haz the power, 2017-06-02 17:54:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-06-02 18:26:47]

All right, there doesn't seem to be any dependency issue.

After addressing the comments, could you land it with two patches:
1. Modify the function implementations in-place, and add TODOs for moving
2. Move the functions to correct files

Thanks!

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransfer.cpp (right):

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransfer.cpp:217:
std::unique_ptr<DragImage> DataTransfer::CreateDragImage(
Let's renamed it into |CreateDragImageForFrame| to avoid the name conflict.

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransfer.cpp:411: return
DataTransfer::NodeImage(*frame, *drag_image_element_);
nit: Remove |DataTransfer|

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/clipboard/DataTransfer.h (right):

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/clipboard/DataTransfer.h:106: static
std::unique_ptr<DragImage> NodeImage(const LocalFrame&, Node&);
Please group all the static functions together.

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalFrameTest.cpp (right):

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalFrameTest.cpp:57:
DataTransfer::NodeImage(GetFrame(), *sample);
Let's create a DataTransferTest for these unit tests of NodeImage().

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalFrameTest.cpp:139:
DragController::DragImageForSelection(&GetFrame(), 0.75f));
Let's create a DragControllerTest for these unit tests of
DragImageForSelection().

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:1085: LocalFrame* src,
Can we pass a |const LocalFrame&| parameter?

https://codereview.chromium.org/2916043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/DragController.cpp:1150: drag_image =
DragController::DragImageForSelection(src, kDragImageAlpha);
nit: Remove |DragController|

[commit-bot: I haz the power, 2017-06-02 20:52:04]

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

[commit-bot: I haz the power, 2017-06-02 20:52:06]

Dry run: This issue passed the CQ dry run.