Drag-and-drop: Target drag messages to specific RenderWidgets.

Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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/c82b174cd84e9749ec642bfd5bc7805a41d6af0b
Cr-Commit-Position: refs/heads/master@{#432688}

[paulmeyer, 2016-11-16 19:21:53]

Description was changed from

==========
WIP D&D OOPIF changes to WebContentsViewAura.
==========

to

==========
WIP D&D OOPIF changes to WebContentsViewAura.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[paulmeyer, 2016-11-16 19:22:53]

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

[commit-bot: I haz the power, 2016-11-16 19:23:50]

Dry run: 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-16 19:49:24]

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

[commit-bot: I haz the power, 2016-11-16 19:49:25]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[paulmeyer, 2016-11-16 19:54:27]

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

[commit-bot: I haz the power, 2016-11-16 19:55:24]

Dry run: 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-16 20:34:40]

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

[commit-bot: I haz the power, 2016-11-16 20:34:41]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[paulmeyer, 2016-11-16 20:41:19]

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

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

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

[paulmeyer, 2016-11-16 21:09:40]

Description was changed from

==========
WIP D&D OOPIF changes to WebContentsViewAura.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd, etc.) to be
routed to the specific RenderWidgets they affect (rather than always to the main
frame widget). This is part of an ongoing effort to allow D&D to work with
out-of-process iframes. 

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

[paulmeyer, 2016-11-16 21:10:19]

Description was changed from

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd, etc.) to be
routed to the specific RenderWidgets they affect (rather than always to the main
frame widget). This is part of an ongoing effort to allow D&D to work with
out-of-process iframes. 

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

to

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd, etc.) to
be routed to the specific RenderWidgets they affect (rather than always to
the main frame widget). This is part of an ongoing effort to allow D&D to
work with out-of-process iframes.

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

[paulmeyer, 2016-11-16 21:10:50]

Description was changed from

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd, etc.) to
be routed to the specific RenderWidgets they affect (rather than always to
the main frame widget). This is part of an ongoing effort to allow D&D to
work with out-of-process iframes.

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

to

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort to allow D&D to work with out-of-process iframes.

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

[paulmeyer, 2016-11-16 21:11:05]

Patchset #1 (id:1) has been deleted

[paulmeyer, 2016-11-16 21:11:14]

Patchset #1 (id:20001) has been deleted

[paulmeyer, 2016-11-16 21:11:56]

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

[paulmeyer, 2016-11-16 21:14:55]

Description was changed from

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort to allow D&D to work with out-of-process iframes.

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

to

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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-16 22:28:15]

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

[commit-bot: I haz the power, 2016-11-16 22:28:16]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ncarter (slow), 2016-11-16 22:31:24]

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_delegate_view.h (right):

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_view_host_delegate_view.h:31: // use the
default WebContentsView implementations.
Let's delete the second sentence here, it's invalid; embedders don't see this
interface.

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_view_host_delegate_view.h:49:
RenderWidgetHost* source_rwh) {}
This could be a RenderWidgetHostImpl, and I think it'll let us get rid of a cast
inside of the aura version if we do. However, it's also okay as a
RenderWidgetHost if you're in a rush to land this.

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
File content/browser/web_contents/web_contents_view_aura.cc (right):

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_view_aura.cc:1190: void
WebContentsViewAura::OnDragExited() {
Was the old DCHECK(current_rvh_for_drag_) bogus, or did something change
semantically to make it invalid?

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_view_aura.cc:1191: if
(current_rvh_for_drag_ != web_contents_->GetRenderViewHost() ||
Is current_rvh_for_drag_ here to stay, or do later CLs get rid of it? I'm
wondering if 'current_rwh_for_drag_->owner_delegate()' could eventually take its
place; owner_delegate is how the ViewHosts's WidgetHost talks to its ViewHost.
RWHOwnerDelegate to RVH is not a safe cast (RFH also implements
RWHOwnerDelegate), but pointer comparison to an existing RVH ought to work, and
be safer than the 'don't deref me rvh void*' pattern.

Only mentioning this in case you hadn't thought this through. No action needed
in this CL.

[paulmeyer, 2016-11-16 22:43:43]

Patchset #2 (id:60001) has been deleted

[paulmeyer, 2016-11-16 22:44:21]

PTAL

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_delegate_view.h (right):

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_view_host_delegate_view.h:31: // use the
default WebContentsView implementations.
On 2016/11/16 22:31:23, ncarter wrote:
> Let's delete the second sentence here, it's invalid; embedders don't see this
> interface.

Done.

https://codereview.chromium.org/2505113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_view_host_delegate_view.h:49:
RenderWidgetHost* source_rwh) {}
On 2016/11/16 22:31:23, ncarter wrote:
> This could be a RenderWidgetHostImpl, and I think it'll let us get rid of a
cast
> inside of the aura version if we do. However, it's also okay as a
> RenderWidgetHost if you're in a rush to land this.

I wasn't sure if it was okay to be impl here. I will change it.

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
File content/browser/web_contents/web_contents_view_aura.cc (right):

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_view_aura.cc:1190: void
WebContentsViewAura::OnDragExited() {
On 2016/11/16 22:31:23, ncarter wrote:
> Was the old DCHECK(current_rvh_for_drag_) bogus, or did something change
> semantically to make it invalid?

I wasn't sure how to recreate it using the new weak pointer. It essentially was
checking that current_rvh_for_drag_ had been set before. Now it may have been
set, but will fail the DCHECK if the pointer is invalid.

https://codereview.chromium.org/2505113002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_view_aura.cc:1191: if
(current_rvh_for_drag_ != web_contents_->GetRenderViewHost() ||
On 2016/11/16 22:31:23, ncarter wrote:
> Is current_rvh_for_drag_ here to stay, or do later CLs get rid of it? I'm
> wondering if 'current_rwh_for_drag_->owner_delegate()' could eventually take
its
> place; owner_delegate is how the ViewHosts's WidgetHost talks to its ViewHost.
> RWHOwnerDelegate to RVH is not a safe cast (RFH also implements
> RWHOwnerDelegate), but pointer comparison to an existing RVH ought to work,
and
> be safer than the 'don't deref me rvh void*' pattern.
> 
> Only mentioning this in case you hadn't thought this through. No action needed
> in this CL.

I think it may go away (I will try to make it go away). If I can't, then I will
replace it with your suggestion in a later CL. :)

[ncarter (slow), 2016-11-16 22:57:07]

lgtm

[paulmeyer, 2016-11-16 22:57:32]

The CQ bit was checked by paulmeyer@chromium.org

[commit-bot: I haz the power, 2016-11-16 22:58:36]

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-17 01:46:07]

Description was changed from

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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-17 01:46:08]

Committed patchset #3 (id:100001)

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

Description was changed from

==========
Drag-and-drop: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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: Target drag messages to specific RenderWidgets.

This patch allows drag messages (like dragEnter, dragOver, dragEnd,
etc.) to be routed to the specific RenderWidgets they affect (rather
than always to the main frame widget). This is part of an ongoing
effort 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/c82b174cd84e9749ec642bfd5bc7805a41d6af0b
Cr-Commit-Position: refs/heads/master@{#432688}
==========

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

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

[dcheng, 2016-11-20 20:03:45]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-11-20 20:03:46]

(Missed this CL originally, but similar comments to the Mac one)

Also, do we need to do anything for Android?

https://codereview.chromium.org/2505113002/diff/100001/content/browser/web_co...
File content/browser/web_contents/web_contents_view_aura.cc (right):

https://codereview.chromium.org/2505113002/diff/100001/content/browser/web_co...
content/browser/web_contents/web_contents_view_aura.cc:576:
drag_start_rwh_->DragSourceEndedAt(client_loc, screen_loc, ops);
As mentioned on the Mac CL, we should consolidate this to all be handled by
WebContentsImpl.

https://codereview.chromium.org/2505113002/diff/100001/content/browser/web_co...
content/browser/web_contents/web_contents_view_aura.cc:1171:
OnDragEntered(event);
And similarly, we need to figure out how to do this without calling the
delegates multiple times. It might be a simple matter of checking if
current_rwh_for_drag_ is already non-null.