Drag-and-drop: dragSourceEndedAt, dragSourceSystemDragEnded

Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/8fc8ea94ab9b37dc2fab2beeb6bff3180685d2d5
Cr-Commit-Position: refs/heads/master@{#432111}

[paulmeyer, 2016-11-14 22:10:49]

Description was changed from

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
==========

to

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[paulmeyer, 2016-11-14 22:11:41]

Description was changed from

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[paulmeyer, 2016-11-14 22:17:34]

+nick@ for content/ review.

[paulmeyer, 2016-11-14 22:18:34]

paulmeyer@chromium.org changed reviewers:
+ dcheng@chromium.org, nick@chromium.org

[paulmeyer, 2016-11-14 22:18:35]

+dcheng for Blink review.

[ncarter (slow), 2016-11-14 23:21:37]

lgtm with one point of discussion

https://codereview.chromium.org/2501213002/diff/1/content/renderer/render_wid...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/2501213002/diff/1/content/renderer/render_wid...
content/renderer/render_widget.cc:1714: WebDragOperation op) {
RenderWidget::OnDragTargetDrop seems to defend against the !GetWebWidget() case.
Do you know why, and if so, do we need to do that here?

From looking at the code we probably don't need it -- during normal operation,
GetWebWidget() ought to be a valid pointer between Init and Close(). Init is
also where the widget becomes added to the routing map, and the widget is
removed from the routing map before Close() happens.

[paulmeyer, 2016-11-15 01:20:30]

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

[paulmeyer, 2016-11-15 01:20:31]

+rbyers@ for OWNER review of event_sender.cc

https://codereview.chromium.org/2501213002/diff/1/content/renderer/render_wid...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/2501213002/diff/1/content/renderer/render_wid...
content/renderer/render_widget.cc:1714: WebDragOperation op) {
On 2016/11/14 23:21:37, ncarter wrote:
> RenderWidget::OnDragTargetDrop seems to defend against the !GetWebWidget()
case.
> Do you know why, and if so, do we need to do that here?
> 
> From looking at the code we probably don't need it -- during normal operation,
> GetWebWidget() ought to be a valid pointer between Init and Close(). Init is
> also where the widget becomes added to the routing map, and the widget is
> removed from the routing map before Close() happens.

Good catch. I believe it is actually needed. If you look at other functions (not
written by me) in this file, it seems to be possible for these messages to come
through when GetWebWdiget() is null. I will add the check. dcheng@ noticed this
as well and said that something should be fixed about this in the future, but to
leave the checks in for now.

[paulmeyer, 2016-11-15 01:23:13]

paulmeyer@chromium.org changed reviewers:
+ tommycli@chromium.org

[paulmeyer, 2016-11-15 01:23:13]

+tommycli@ for review of webview_plugin.cc

[tommycli, 2016-11-15 01:25:00]

lgtm assuming:

https://codereview.chromium.org/2501213002/diff/20001/components/plugins/rend...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/2501213002/diff/20001/components/plugins/rend...
components/plugins/renderer/webview_plugin.cc:249:
web_view_->mainFrame()->toWebLocalFrame()->frameWidget()->
I assume frameWidget() is never null right?

[paulmeyer, 2016-11-15 01:42:55]

https://codereview.chromium.org/2501213002/diff/20001/components/plugins/rend...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/2501213002/diff/20001/components/plugins/rend...
components/plugins/renderer/webview_plugin.cc:249:
web_view_->mainFrame()->toWebLocalFrame()->frameWidget()->
On 2016/11/15 01:25:00, tommycli wrote:
> I assume frameWidget() is never null right?

Right. It can never be null here.

[Rick Byers, 2016-11-15 01:46:52]

On 2016/11/15 01:20:31, paulmeyer wrote:
> +rbyers@ for OWNER review of event_sender.cc

event_sender LGTM

[dcheng, 2016-11-15 02:40:16]

lgtm

[paulmeyer, 2016-11-15 02:45:55]

The CQ bit was checked by paulmeyer@chromium.org

[paulmeyer, 2016-11-15 02:45:56]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org
Link to the patchset: https://codereview.chromium.org/2501213002/#ps20001
(title: "Rebased. Added check for !GetWebWidget().")

[commit-bot: I haz the power, 2016-11-15 02:47:21]

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

[commit-bot: I haz the power, 2016-11-15 05:12:51]

Description was changed from

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-11-15 05:12:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-15 05:19:43]

Description was changed from

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Drag-and-drop: Move dragSourceEndedAt and dragSourceSystemDragEnded out
of WebView/RenderView.

This patch is part of the effort to transition D&D functions to all be
widget-based instead of view-based. This is necessary in order to allow
D&D to work with out-of-process iframes.

BUG=647249, 655063
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/8fc8ea94ab9b37dc2fab2beeb6bff3180685d2d5
Cr-Commit-Position: refs/heads/master@{#432111}
==========

[commit-bot: I haz the power, 2016-11-15 05:19:44]

Patchset 2 (id:??) landed as
https://crrev.com/8fc8ea94ab9b37dc2fab2beeb6bff3180685d2d5
Cr-Commit-Position: refs/heads/master@{#432111}