Fixed drag and drop into and out of Browser Plugin.

Fixed drag and drop into and out of Browser Plugin.

Only works when the --enable-browser-plugin-compositing flag is used, which causes web_contents_view_guest to be used.

How the flow works:
When a drag event is initiated on the web page, the BrowserPluginGuest receives an DragHostMsg_StartDragging IPC, and keeps track of that fact that it has initiated a drag event, but doesn't handle the IPC.
The IPC is then passed down the chain (not sure by who) and eventually calls WebContentsViewGuest::StartDragging.
WebContentsViewGuest::StartDragging calls the platform dependent WebContentsView of the embedder and starts the OS drag event.

Now, we get updates on the drag event from 2 places, BrowserPlugin::handleDragStatusUpdate, and RenderViewImpl::OnDragTargetDragOver of the embedder process.

BrowserPlugin::handleDragStatusUpdate goes through the guest to call RenderViewHostImpl::DragTargetDragOver on the guest process (but only when the mouse is hovering over the guest)
RenderViewImpl::OnDragTargetDragOver also calls RenderViewHostImpl::DragTargetDragOver but on the embedder process, and it always calls the function, whether or not the mouse is over the guest. For that reason, we keep track of if the mouse if over the guest, and use the embedder to capture and ignore the DragTargetDragOver message on the embedder process when the mouse is over the guest (or we get a flickering mouse).

Now, when the mouse is released, the native drag_host calls RenderViewImpl::OnDragSourceSystemDragEnded, which in turn calls webview()->dragSourceSystemDragEnded(); on the embedder process. However, the guest process webview() is not informed that the drag has ended, so it still blocks user input. So we send a new IPC, DragHostMsg_DragStopped to the embedder to let it inform the guest, who in turn informs the guest webview, that the drag has ended.

BUG=161112
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=198119

[Fady Samuel, 2013-02-01 14:59:28]

I don't fully understand the control flow here. Could you please put a
high-level description of what's going on and why you did what you did in the
issue description?

https://codereview.chromium.org/12086095/diff/2001/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/2001/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_guest.cc:381: dragging_ = true;
Use IPC_MESSAGE_HANDLER_GENERIC(DragHostMsg_StartDragging,
OnStartDragging(&handled));

That way you can do the work in a handler elsewhere (to avoid cluttering
OnMessageReceived), and set the handled = false.

https://codereview.chromium.org/12086095/diff/2001/content/common/drag_messag...
File content/common/drag_messages.h (right):

https://codereview.chromium.org/12086095/diff/2001/content/common/drag_messag...
content/common/drag_messages.h:74: IPC_MESSAGE_ROUTED0(DragHostMsg_DragStopped)
Why do we need to introduce a new message?

https://codereview.chromium.org/12086095/diff/2001/content/renderer/render_vi...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/12086095/diff/2001/content/renderer/render_vi...
content/renderer/render_view_impl.cc:5278: Send(new
DragHostMsg_DragStopped(routing_id_));
What's going on here? This changes impacts all renderers not just BrowserPlugin.

[Fady Samuel, 2013-02-01 15:01:08]

Also:

1. Does this work on Windows/Mac?
2. Tests please.

[mthiesse, 2013-02-01 15:43:54]

https://codereview.chromium.org/12086095/diff/2001/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/2001/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_guest.cc:381: dragging_ = true;
On 2013/02/01 14:59:28, Fady Samuel wrote:
> Use IPC_MESSAGE_HANDLER_GENERIC(DragHostMsg_StartDragging,
> OnStartDragging(&handled));
> 
> That way you can do the work in a handler elsewhere (to avoid cluttering
> OnMessageReceived), and set the handled = false.

Done.

https://codereview.chromium.org/12086095/diff/2001/content/common/drag_messag...
File content/common/drag_messages.h (right):

https://codereview.chromium.org/12086095/diff/2001/content/common/drag_messag...
content/common/drag_messages.h:74: IPC_MESSAGE_ROUTED0(DragHostMsg_DragStopped)
On 2013/02/01 14:59:28, Fady Samuel wrote:
> Why do we need to introduce a new message?

We need a message that can be sent to the embedder to let it know that the drag
operation is done, so that it can inform the guest of the same, so that the
guest can finish up its drag event and restore user input.

https://codereview.chromium.org/12086095/diff/2001/content/renderer/render_vi...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/12086095/diff/2001/content/renderer/render_vi...
content/renderer/render_view_impl.cc:5278: Send(new
DragHostMsg_DragStopped(routing_id_));
On 2013/02/01 14:59:28, Fady Samuel wrote:
> What's going on here? This changes impacts all renderers not just
BrowserPlugin.

Well that's the point of using a new message; it won't affect any existing
functionality.
The purpose of the message here is to let the embedder know when the native
drag_host_ has ended the drag event (so that the embedder can let the guest know
the drag event is done and the guest can end it's drag event too (and restore
user input)). It's the only platform agnostic place to put this kind of message.

[Fady Samuel, 2013-02-01 15:54:55]

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:204:
it->second)->GetBrowserPluginGuest()->dragging())
If we're effectively only allowing one guest to be dragging/dragged over at any
one time, why is the state stored in BrowserPluginGuest and not
BrowserPluginEmbedder?

[mthiesse1, 2013-02-01 16:02:58]

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:204:
it->second)->GetBrowserPluginGuest()->dragging())
On 2013/02/01 15:54:55, Fady Samuel wrote:
> If we're effectively only allowing one guest to be dragging/dragged over at
any
> one time, why is the state stored in BrowserPluginGuest and not
> BrowserPluginEmbedder?

We need a way to identify which guest is the one that's dragging/being dragged
over, and the embedder doesn't know this based on the messages it receives. The
DragHost messages don't include instance IDs.

[dcheng, 2013-02-01 16:05:29]

Isn't there a browser test for this? How come it didn't catch this failure?

[dcheng, 2013-02-01 16:24:52]

lgtm

[dcheng, 2013-02-01 16:26:49]

Eeps. Please ignore that.

Another question: how come we can't fix this in WebKit by simply forwarding the
drag ended event to the browser plugin as well?

[mthiesse, 2013-02-01 16:51:44]

On 2013/02/01 16:26:49, dcheng wrote:
> Eeps. Please ignore that.
> 
> Another question: how come we can't fix this in WebKit by simply forwarding
the
> drag ended event to the browser plugin as well?

Isn't drag ended started in the drag host, not webkit? If you mean modifying
webview()->dragSourceSystemDragEnded(), that isn't really doable either.. how is
webkit supposed to know there's a guest that needs its drag event ended too?

[mthiesse, 2013-02-01 16:53:43]

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/2003/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:204:
it->second)->GetBrowserPluginGuest()->dragging())
On 2013/02/01 15:54:55, Fady Samuel wrote:
> If we're effectively only allowing one guest to be dragging/dragged over at
any
> one time, why is the state stored in BrowserPluginGuest and not
> BrowserPluginEmbedder?

Done.

[Fady Samuel, 2013-02-01 16:59:53]

https://codereview.chromium.org/12086095/diff/7011/content/browser/browser_pl...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/7011/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:199: void
BrowserPluginEmbedder::DragEnteredGuest(BrowserPluginGuest* guest) {
SetDragEnteredGuest I'm a bit concerned about lifetime issues (what if we
attempt to remove a BrowserPlugin as we're dragging). Maybe we should
have a weak pointer to BrowserPluginGuest?

https://codereview.chromium.org/12086095/diff/7011/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:203: void
BrowserPluginEmbedder::DragLeftGuest(BrowserPluginGuest* guest) {
SetDragLeftGuest. I'm a bit concerned about lifetime issues. Maybe we should
have a weak pointer to BrowserPluginGuest?

https://codereview.chromium.org/12086095/diff/7011/content/browser/browser_pl...
content/browser/browser_plugin/browser_plugin_embedder.cc:249: if
(guest_started_drag)
Please move this and UpdateDragCursor to separate message handlers so as to
avoid cluttering OnMessageReceived.

[dcheng, 2013-02-01 17:02:21]

On 2013/02/01 16:51:44, mthiesse wrote:
> On 2013/02/01 16:26:49, dcheng wrote:
> > Eeps. Please ignore that.
> > 
> > Another question: how come we can't fix this in WebKit by simply forwarding
> the
> > drag ended event to the browser plugin as well?
> 
> Isn't drag ended started in the drag host, not webkit? If you mean modifying
> webview()->dragSourceSystemDragEnded(), that isn't really doable either.. how
is
> webkit supposed to know there's a guest that needs its drag event ended too?

Isn't the guest implemented as a plugin? WebKit itself needs to know when it's
dragging over a plugin, so it seems reasonable for WebKit to forward a "drag
ended" message to the plugin when a stag ends for whatever reason. See
http://trac.webkit.org/changeset/130504 for reference.

[mthiesse, 2013-02-01 17:46:45]

On 2013/02/01 17:02:21, dcheng wrote:
> On 2013/02/01 16:51:44, mthiesse wrote:
> > On 2013/02/01 16:26:49, dcheng wrote:
> > > Eeps. Please ignore that.
> > > 
> > > Another question: how come we can't fix this in WebKit by simply
forwarding
> > the
> > > drag ended event to the browser plugin as well?
> > 
> > Isn't drag ended started in the drag host, not webkit? If you mean modifying
> > webview()->dragSourceSystemDragEnded(), that isn't really doable either..
how
> is
> > webkit supposed to know there's a guest that needs its drag event ended too?
> 
> Isn't the guest implemented as a plugin? WebKit itself needs to know when it's
> dragging over a plugin, so it seems reasonable for WebKit to forward a "drag
> ended" message to the plugin when a stag ends for whatever reason. See
> http://trac.webkit.org/changeset/130504 for reference.

Ah, I see what you're saying, and yes, that would be a much nicer solution. I'll
look into it now.

[sadrul, 2013-02-01 18:09:42]

On 2013/02/01 17:46:45, mthiesse wrote:
> On 2013/02/01 17:02:21, dcheng wrote:
> > On 2013/02/01 16:51:44, mthiesse wrote:
> > > On 2013/02/01 16:26:49, dcheng wrote:
> > > > Eeps. Please ignore that.
> > > > 
> > > > Another question: how come we can't fix this in WebKit by simply
> forwarding
> > > the
> > > > drag ended event to the browser plugin as well?
> > > 
> > > Isn't drag ended started in the drag host, not webkit? If you mean
modifying
> > > webview()->dragSourceSystemDragEnded(), that isn't really doable either..
> how
> > is
> > > webkit supposed to know there's a guest that needs its drag event ended
too?
> > 
> > Isn't the guest implemented as a plugin? WebKit itself needs to know when
it's
> > dragging over a plugin, so it seems reasonable for WebKit to forward a "drag
> > ended" message to the plugin when a stag ends for whatever reason. See
> > http://trac.webkit.org/changeset/130504 for reference.
> 
> Ah, I see what you're saying, and yes, that would be a much nicer solution.
I'll
> look into it now.

I think if the drop happens on the plugin, then the plugin will receive a
message from webkit? However, if the drop happens outside the plugin, then the
plugin probably won't receive that message?

If I understand correctly, the way it works with this patch is:
 * [renderer] Dragging is initiated from the guest.
 * [browser] The guest tells the embedder that dragging started
 * [browser] The embedder receives drag updates, and sends them off to the
embedder-renderer
 * [renderer] The embedder can forward the drag update events to the guest
plugin if the drag update happens over the guest.
 * [browser] Dragging ends. This goes through the embedder. The embedder sends a
message to the renderer.
 * [renderer] The embedder tells WebKit that dragging ended. It sends back a
message to the browser.
 * [browser] The browser then forwards this message back to the guest.
 * [everyone] Happy.

Is my understanding correct?

[mthiesse, 2013-02-01 18:32:42]

On 2013/02/01 17:02:21, dcheng wrote:
> On 2013/02/01 16:51:44, mthiesse wrote:
> > On 2013/02/01 16:26:49, dcheng wrote:
> > > Eeps. Please ignore that.
> > > 
> > > Another question: how come we can't fix this in WebKit by simply
forwarding
> > the
> > > drag ended event to the browser plugin as well?
> > 
> > Isn't drag ended started in the drag host, not webkit? If you mean modifying
> > webview()->dragSourceSystemDragEnded(), that isn't really doable either..
how
> is
> > webkit supposed to know there's a guest that needs its drag event ended too?
> 
> Isn't the guest implemented as a plugin? WebKit itself needs to know when it's
> dragging over a plugin, so it seems reasonable for WebKit to forward a "drag
> ended" message to the plugin when a stag ends for whatever reason. See
> http://trac.webkit.org/changeset/130504 for reference.

Okay, so after looking into this, this looks like a no-go. For starters, even
when the drag is started in the plugin it doesn't look like
WebPluginContainerImpl::handleMouseEvent sees the start and end events, only the
other event types like Over or Drop. Even if it did, if the drag started in the
plugin, and stopped outside of it this message would not be received, and the
plugin would no longer accept input. As far as I can tell the way I did it is
the only way to get it working.

[mthiesse, 2013-02-01 18:42:20]

On 2013/02/01 18:09:42, sadrul wrote:
> I think if the drop happens on the plugin, then the plugin will receive a
> message from webkit? However, if the drop happens outside the plugin, then the
> plugin probably won't receive that message?
> 
> If I understand correctly, the way it works with this patch is:
>  * [renderer] Dragging is initiated from the guest.
>  * [browser] The guest tells the embedder that dragging started
>  * [browser] The embedder receives drag updates, and sends them off to the
> embedder-renderer
>  * [renderer] The embedder can forward the drag update events to the guest
> plugin if the drag update happens over the guest.
>  * [browser] Dragging ends. This goes through the embedder. The embedder sends
a
> message to the renderer.
>  * [renderer] The embedder tells WebKit that dragging ended. It sends back a
> message to the browser.
>  * [browser] The browser then forwards this message back to the guest.
>  * [everyone] Happy.
> 
> Is my understanding correct?

Umm, looks close. I would modify it like this, but I don't really understand
your [browser] [renderer] notation...

* Dragging is initiated from the guest.
* The guest tells the embedder that dragging started
* The embedder-renderView receives drag updates, and sends them off to the
embedder-renderViewHost passing through the embedder on the way
* The Plugin also receives drag up (separately from the embedder-renderView) and
forwards them to the guest.
* Dragging ends. This goes through the embedder. The embedder sends a message to
the renderer.
* The embedder tells WebKit that dragging ended and tells the guest that
dragging ended.
* The embedder tells WebKit that dragging ended.
* Happiness ensues.

[sadrul, 2013-02-01 20:37:15]

On 2013/02/01 18:42:20, mthiesse wrote:
> On 2013/02/01 18:09:42, sadrul wrote:
> > I think if the drop happens on the plugin, then the plugin will receive a
> > message from webkit? However, if the drop happens outside the plugin, then
the
> > plugin probably won't receive that message?
> > 
> > If I understand correctly, the way it works with this patch is:
> >  * [renderer] Dragging is initiated from the guest.
> >  * [browser] The guest tells the embedder that dragging started
> >  * [browser] The embedder receives drag updates, and sends them off to the
> > embedder-renderer
> >  * [renderer] The embedder can forward the drag update events to the guest
> > plugin if the drag update happens over the guest.
> >  * [browser] Dragging ends. This goes through the embedder. The embedder
sends
> a
> > message to the renderer.
> >  * [renderer] The embedder tells WebKit that dragging ended. It sends back a
> > message to the browser.
> >  * [browser] The browser then forwards this message back to the guest.
> >  * [everyone] Happy.
> > 
> > Is my understanding correct?
> 
> Umm, looks close. I would modify it like this, but I don't really understand
> your [browser] [renderer] notation...

I tried to clarify what process was doing what. This was perhaps not clear
enough :)

> 
> * Dragging is initiated from the guest.
> * The guest tells the embedder that dragging started
> * The embedder-renderView receives drag updates, and sends them off to the
> embedder-renderViewHost passing through the embedder on the way

The embedder's RenderView receives the drag-update events from the embedder's
RenderViewHost, right?

> * The Plugin also receives drag up (separately from the embedder-renderView)
and
> forwards them to the guest.

I am curious to know where the plugin receives this update from. Is this
initiated from the embedder's WebKit?

> * Dragging ends. This goes through the embedder. The embedder sends a message
to
> the renderer.
> * The embedder tells WebKit that dragging ended and tells the guest that
> dragging ended.

The embedder tells the guest, through the browser process (i.e. the embedder
sends the new DragStopped message to the browser, the BrowserPluginGuest
intercepts this message, and tells the guest's renderer/WebKit that the dragging
ends), right?

[mthiesse, 2013-02-01 20:53:20]

On 2013/02/01 20:37:15, sadrul wrote:
> On 2013/02/01 18:42:20, mthiesse wrote:
> > On 2013/02/01 18:09:42, sadrul wrote:
> > > I think if the drop happens on the plugin, then the plugin will receive a
> > > message from webkit? However, if the drop happens outside the plugin, then
> the
> > > plugin probably won't receive that message?
> > > 
> > > If I understand correctly, the way it works with this patch is:
> > >  * [renderer] Dragging is initiated from the guest.
> > >  * [browser] The guest tells the embedder that dragging started
> > >  * [browser] The embedder receives drag updates, and sends them off to the
> > > embedder-renderer
> > >  * [renderer] The embedder can forward the drag update events to the guest
> > > plugin if the drag update happens over the guest.
> > >  * [browser] Dragging ends. This goes through the embedder. The embedder
> sends
> > a
> > > message to the renderer.
> > >  * [renderer] The embedder tells WebKit that dragging ended. It sends back
a
> > > message to the browser.
> > >  * [browser] The browser then forwards this message back to the guest.
> > >  * [everyone] Happy.
> > > 
> > > Is my understanding correct?
> > 
> > Umm, looks close. I would modify it like this, but I don't really understand
> > your [browser] [renderer] notation...
> 
> I tried to clarify what process was doing what. This was perhaps not clear
> enough :)
> 
> > 
> > * Dragging is initiated from the guest.
> > * The guest tells the embedder that dragging started
> > * The embedder-renderView receives drag updates, and sends them off to the
> > embedder-renderViewHost passing through the embedder on the way
> 
> The embedder's RenderView receives the drag-update events from the embedder's
> RenderViewHost, right?

No, the other way around. The drag host calls RenderViewImpl, and RenderViewImpl
sends an IPC that's handled by the embedder, then the RenderViewHost.

> 
> > * The Plugin also receives drag up (separately from the embedder-renderView)
> and
> > forwards them to the guest.
> 
> I am curious to know where the plugin receives this update from. Is this
> initiated from the embedder's WebKit?

I don't know, you wrote it :P
http://trac.webkit.org/changeset/130504

But yes, I believe it's the embedder's webkit on mouse event handlers.

> 
> > * Dragging ends. This goes through the embedder. The embedder sends a
message
> to
> > the renderer.
> > * The embedder tells WebKit that dragging ended and tells the guest that
> > dragging ended.
> 
> The embedder tells the guest, through the browser process (i.e. the embedder
> sends the new DragStopped message to the browser, the BrowserPluginGuest
> intercepts this message, and tells the guest's renderer/WebKit that the
dragging
> ends), right?

Umm yes? Though I don't know if it's correct to say the embedder sends the
message to the browser. DragStopped is sent exactly the same way as all of the
other updates started by the drag host (i.e. from the embedder process's
RenderViewImpl), but this time the embedder captures it and calls the
OnMessageReceived of the guest, passing it the message.

[mthiesse, 2013-02-04 17:53:51]

Sorry, when I said OnMessageReceived of the guest I meant the OnMessageReceived
of BrowserPluginGuest, which is in the browser process.

[mthiesse, 2013-02-07 15:28:55]

https://chromiumcodereview.appspot.com/12086095/diff/7011/content/browser/bro...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://chromiumcodereview.appspot.com/12086095/diff/7011/content/browser/bro...
content/browser/browser_plugin/browser_plugin_embedder.cc:199: void
BrowserPluginEmbedder::DragEnteredGuest(BrowserPluginGuest* guest) {
On 2013/02/01 16:59:53, Fady Samuel wrote:
> SetDragEnteredGuest I'm a bit concerned about lifetime issues (what if we
> attempt to remove a BrowserPlugin as we're dragging). Maybe we should
> have a weak pointer to BrowserPluginGuest?

Done.

https://chromiumcodereview.appspot.com/12086095/diff/7011/content/browser/bro...
content/browser/browser_plugin/browser_plugin_embedder.cc:203: void
BrowserPluginEmbedder::DragLeftGuest(BrowserPluginGuest* guest) {
On 2013/02/01 16:59:53, Fady Samuel wrote:
> SetDragLeftGuest. I'm a bit concerned about lifetime issues. Maybe we should
> have a weak pointer to BrowserPluginGuest?

Done.

https://chromiumcodereview.appspot.com/12086095/diff/7011/content/browser/bro...
content/browser/browser_plugin/browser_plugin_embedder.cc:249: if
(guest_started_drag)
On 2013/02/01 16:59:53, Fady Samuel wrote:
> Please move this and UpdateDragCursor to separate message handlers so as to
> avoid cluttering OnMessageReceived.

Done.

[Fady Samuel, 2013-02-07 15:37:18]

https://chromiumcodereview.appspot.com/12086095/diff/17001/content/browser/br...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://chromiumcodereview.appspot.com/12086095/diff/17001/content/browser/br...
content/browser/browser_plugin/browser_plugin_guest.cc:327:
switches::kEnableBrowserPluginCompositing)) {
kEnableBrowserPluginGuestViews. This doesn't need to depend on compositing. We
might want to enable guest views before compositing is on by default.

[mthiesse, 2013-02-07 15:54:05]

https://chromiumcodereview.appspot.com/12086095/diff/17001/content/browser/br...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://chromiumcodereview.appspot.com/12086095/diff/17001/content/browser/br...
content/browser/browser_plugin/browser_plugin_guest.cc:327:
switches::kEnableBrowserPluginCompositing)) {
On 2013/02/07 15:37:18, Fady Samuel wrote:
> kEnableBrowserPluginGuestViews. This doesn't need to depend on compositing. We
> might want to enable guest views before compositing is on by default.

Done.

[mthiesse, 2013-03-28 20:17:35]

Alright Sadrul, Fady, I think we're at the point now where we can enable this
for Linux, and fix the bugs on other platforms later, so PTAL.

[Fady Samuel, 2013-04-09 20:49:11]

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:99: int screen_x, int
screen_y, WebKit::WebDragOperation operation) {
Hmm, is client_x, client_y in the right coordinate system?

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:100: if
(guest_started_drag_)
Put in block:

if (guest_started_drag_) {
  guest_started_drag_->DragSourceEndedAt(client_x, client_y,
      screen_x, screen_y, operation);
}

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:105: void
BrowserPluginEmbedder::DragSourceMovedTo(int client_x, int client_y,
ditto

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:108:
guest_started_drag_->DragSourceMovedTo(client_x, client_y,
{
...
}

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:503:
web_contents()->GetRenderViewHost()->DragSourceSystemDragEnded();
This ought to fix the sticky selection problem:

void BrowserPluginGuest::EndSystemDrag() {
  RenderViewHostImpl* guest_rvh = static_cast<RenderViewHostImpl*>(
      GetWebContents()->GetRenderViewHost());
  guest_rvh->DragSourceSystemDragEnded();
  // Issue a MouseUp event to get out of a selection state.
  WebKit::WebMouseEvent mouse_event;
  mouse_event.type = WebKit::WebInputEvent::MouseUp;
  mouse_event.button = WebKit::WebMouseEvent::ButtonLeft;
  guest_rvh->ForwardMouseEvent(mouse_event);
}

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:586: // the
kEnableBrowserPluginGuestViews flag. So inject some JS to disable
Stale comment.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:774:
host->DragSourceSystemDragEnded();
EndSystemDrag();

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:1089:
embedder_web_contents_->GetBrowserPluginEmbedder()->StartDrag(this);
Why not just move this to WebContentsViewGuest::StartDragging  to avoid this
method all together?

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:1103: // Don't mark as
handled.
This comment on its own is not very useful. Please explain why we do this?

Actually it seems like we either need to call EndSystemDrag() here or
host->DragSourceSystemDragEnded(); in BrowserPluginGuest::OnDragStatusUpdate?

Do we need both? If not, maybe we simply get rid of this one and leave the
other? I'm not sure.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
File content/browser/web_contents/web_contents_view_guest.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_contents_view_guest.cc:198: RenderViewHostImpl*
embedder_render_view_host =
wrong spacing.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
File content/browser/web_contents/web_drag_source_gtk.cc (left):

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:167: if
(GetRenderViewHost()) {
This guard seems weird...if (web_contents_) seems more appropriate.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:303: if
(GetRenderViewHost()) {
This guard seems weird...if (web_contents_) seems more appropriate.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:375: if
(GetRenderViewHost()) {
This guard seems weird...if (web_contents_) seems more appropriate.

[dcheng, 2013-04-09 20:59:12]

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
File content/browser/web_contents/web_contents_view_guest.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_contents_view_guest.cc:205:
view->StartDragging(drop_data, ops, image, image_offset, event_info);
Make sure you call SystemDragEnded() if there's no drag actually taking place.
See WebContentsViewGtk::StartDragging for example.

[mthiesse, 2013-04-18 18:02:03]

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:99: int screen_x, int
screen_y, WebKit::WebDragOperation operation) {
On 2013/04/09 20:49:12, Fady Samuel wrote:
> Hmm, is client_x, client_y in the right coordinate system?

Good catch, doesn't seem to change much (if anything) in linux though.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:100: if
(guest_started_drag_)
On 2013/04/09 20:49:12, Fady Samuel wrote:
> Put in block:
> 
> if (guest_started_drag_) {
>   guest_started_drag_->DragSourceEndedAt(client_x, client_y,
>       screen_x, screen_y, operation);
> }

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:105: void
BrowserPluginEmbedder::DragSourceMovedTo(int client_x, int client_y,
On 2013/04/09 20:49:12, Fady Samuel wrote:
> ditto

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:108:
guest_started_drag_->DragSourceMovedTo(client_x, client_y,
On 2013/04/09 20:49:12, Fady Samuel wrote:
> {
> ...
> }

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:503:
web_contents()->GetRenderViewHost()->DragSourceSystemDragEnded();
On 2013/04/09 20:49:12, Fady Samuel wrote:
> This ought to fix the sticky selection problem:
> 
> void BrowserPluginGuest::EndSystemDrag() {
>   RenderViewHostImpl* guest_rvh = static_cast<RenderViewHostImpl*>(
>       GetWebContents()->GetRenderViewHost());
>   guest_rvh->DragSourceSystemDragEnded();
>   // Issue a MouseUp event to get out of a selection state.
>   WebKit::WebMouseEvent mouse_event;
>   mouse_event.type = WebKit::WebInputEvent::MouseUp;
>   mouse_event.button = WebKit::WebMouseEvent::ButtonLeft;
>   guest_rvh->ForwardMouseEvent(mouse_event);
> }
> 

Done. Thanks.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:586: // the
kEnableBrowserPluginGuestViews flag. So inject some JS to disable
On 2013/04/09 20:49:12, Fady Samuel wrote:
> Stale comment.

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:774:
host->DragSourceSystemDragEnded();
On 2013/04/09 20:49:12, Fady Samuel wrote:
> EndSystemDrag();

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:1089:
embedder_web_contents_->GetBrowserPluginEmbedder()->StartDrag(this);
On 2013/04/09 20:49:12, Fady Samuel wrote:
> Why not just move this to WebContentsViewGuest::StartDragging  to avoid this
> method all together?

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:1103: // Don't mark as
handled.
On 2013/04/09 20:49:12, Fady Samuel wrote:
> This comment on its own is not very useful. Please explain why we do this?
> 
> Actually it seems like we either need to call EndSystemDrag() here or
> host->DragSourceSystemDragEnded(); in BrowserPluginGuest::OnDragStatusUpdate?
> 
> Do we need both? If not, maybe we simply get rid of this one and leave the
> other? I'm not sure.

Ah, meant to clean this up, was part of my experiments to fix drag and drop on
windows.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
File content/browser/web_contents/web_contents_view_guest.cc (right):

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_contents_view_guest.cc:198: RenderViewHostImpl*
embedder_render_view_host =
On 2013/04/09 20:49:12, Fady Samuel wrote:
> wrong spacing.

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_contents_view_guest.cc:205:
view->StartDragging(drop_data, ops, image, image_offset, event_info);
On 2013/04/09 20:59:12, dcheng wrote:
> Make sure you call SystemDragEnded() if there's no drag actually taking place.
> See WebContentsViewGtk::StartDragging for example.

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
File content/browser/web_contents/web_drag_source_gtk.cc (left):

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:167: if
(GetRenderViewHost()) {
On 2013/04/09 20:49:12, Fady Samuel wrote:
> This guard seems weird...if (web_contents_) seems more appropriate.

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:303: if
(GetRenderViewHost()) {
On 2013/04/09 20:49:12, Fady Samuel wrote:
> This guard seems weird...if (web_contents_) seems more appropriate.

Done.

https://codereview.chromium.org/12086095/diff/70001/content/browser/web_conte...
content/browser/web_contents/web_drag_source_gtk.cc:375: if
(GetRenderViewHost()) {
On 2013/04/09 20:49:12, Fady Samuel wrote:
> This guard seems weird...if (web_contents_) seems more appropriate.

Done.

[Fady Samuel, 2013-04-18 18:22:56]

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:349: virtual void DragSourceEndedAt(int
client_x, int client_y, int screen_x,
Is this necessary?

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:354: virtual void DragSourceMovedTo(int
client_x, int client_y,
Is this necessary?

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:358: // WebContentsDelegate, and the
BrowserPluginEmbedder, if present.
Please avoid referencing BrowserPluginEmbedder here as it's an implementation
detail. The notion of an EmbedderWebContents exists though, I think.

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents_observer.h:256: virtual void
SystemDragEnded() {}
Is this necessary?

[mthiesse, 2013-04-18 18:36:32]

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:349: virtual void DragSourceEndedAt(int
client_x, int client_y, int screen_x,
On 2013/04/18 18:22:56, Fady Samuel wrote:
> Is this necessary?

Done.

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:354: virtual void DragSourceMovedTo(int
client_x, int client_y,
On 2013/04/18 18:22:56, Fady Samuel wrote:
> Is this necessary?

Done.

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents.h:358: // WebContentsDelegate, and the
BrowserPluginEmbedder, if present.
On 2013/04/18 18:22:56, Fady Samuel wrote:
> Please avoid referencing BrowserPluginEmbedder here as it's an implementation
> detail. The notion of an EmbedderWebContents exists though, I think.

It's probably fine if I leave this comment as-is. Right?

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/12086095/diff/76001/content/public/browser/we...
content/public/browser/web_contents_observer.h:256: virtual void
SystemDragEnded() {}
On 2013/04/18 18:22:56, Fady Samuel wrote:
> Is this necessary?

Missed cleanup of this, wasn't used.

[dcheng, 2013-04-18 21:10:34]

LGTM with some nits.

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
File chrome/browser/extensions/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:41: mouse_loc_ =
point;
Nit: Generally we prefer to avoid abbreviations, so location_ instead of loc_.

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:125: // In the
drag drop test we add 20px padding to the page body because on
!!!

Shouldn't we just adjust the test instead to not drag to the edge of the window?
Or just add padding in the test page.

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:145: virtual void
SetUpCommandLine(CommandLine* command_line) {
Nit: OVERRIDE.

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:241:
value->GetAsBoolean(&result);
Wrap in ASSERT_TRUE or CHECK or something.

https://codereview.chromium.org/12086095/diff/76001/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:434: //#if
(defined(OS_LINUX))
Is this #if supposed to be commented out?
Nit: Also, no () around defined().

https://codereview.chromium.org/12086095/diff/90001/chrome/test/base/ui_contr...
File chrome/test/base/ui_controls_aurax11.cc (right):

https://codereview.chromium.org/12086095/diff/90001/chrome/test/base/ui_contr...
chrome/test/base/ui_controls_aurax11.cc:169: // According to
http://tronche.com/gui/x/xlib/events/keyboard-pointer/
Nit: Just looking at the diff, it's not 100% clear why this change was needed.
Presumably it was a bug fix of some sort, but for future patches, it'd be nice
to break this part of the patch out and land it separately.

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:635: #if
(defined(OS_LINUX))
Nit: no parens() around defined().

[Fady Samuel, 2013-04-18 21:22:41]

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.h (right):

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.h:110:
base::WeakPtr<BrowserPluginGuest> guest_dragging_over_;
Could you please add comments over these two pointers as to what they're used
for?

Could you also add a TODO(fsamuel) along with a crbug to investigate whether
it's possible avoid guest_dragging_over_.

[mthiesse, 2013-04-19 14:55:49]

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.h (right):

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.h:110:
base::WeakPtr<BrowserPluginGuest> guest_dragging_over_;
On 2013/04/18 21:22:41, Fady Samuel wrote:
> Could you please add comments over these two pointers as to what they're used
> for?
> 
> Could you also add a TODO(fsamuel) along with a crbug to investigate whether
> it's possible avoid guest_dragging_over_.

Done.

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/12086095/diff/90001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest.cc:635: #if
(defined(OS_LINUX))
On 2013/04/18 21:10:34, dcheng wrote:
> Nit: no parens() around defined().

Done.

[Fady Samuel, 2013-04-19 15:15:52]

lgtm

[mthiesse, 2013-04-19 15:17:17]

Sending to OWNERS

miket@ for chrome/browser/extensions
jam@ for content/public

[mthiesse, 2013-04-19 15:25:08]

+sky@ for content/browser/

[miket_OOO, 2013-04-19 17:51:31]

> miket@ for chrome/browser/extensions

OWNERS LGTM

[miket_OOO, 2013-04-19 17:51:42]

https://codereview.chromium.org/12086095/diff/94002/chrome/browser/extensions...
File chrome/browser/extensions/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/12086095/diff/94002/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:434: //#if
(defined(OS_LINUX))
Remove this, I think.

[jam, 2013-04-19 18:26:38]

lgtm

[mthiesse, 2013-04-19 18:40:38]

-sky@, turns out jam's an owner, and the content/browser changes are trivial
anyways.

https://codereview.chromium.org/12086095/diff/94002/chrome/browser/extensions...
File chrome/browser/extensions/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/12086095/diff/94002/chrome/browser/extensions...
chrome/browser/extensions/web_view_interactive_browsertest.cc:434: //#if
(defined(OS_LINUX))
On 2013/04/19 17:51:42, miket wrote:
> Remove this, I think.

Removed. Also updated the stale comment.

[commit-bot: I haz the power, 2013-04-19 18:41:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/101002

[commit-bot: I haz the power, 2013-04-19 19:05:39]

Failed to trigger a try job on win_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-04-19 19:05:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/106002

[commit-bot: I haz the power, 2013-04-19 19:53:48]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-22 15:32:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/95005

[commit-bot: I haz the power, 2013-04-22 16:24:28]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-04-22 17:02:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/125007

[commit-bot: I haz the power, 2013-04-22 18:01:35]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-04-29 19:48:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/144001

[commit-bot: I haz the power, 2013-04-29 19:48:34]

Failed to apply patch for content/browser/web_contents/web_drag_source_win.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/web_contents/web_drag_source_win.cc
  Hunk #1 FAILED at 6.
  Hunk #2 FAILED at 35.
  Hunk #3 succeeded at 57 (offset 2 lines).
  Hunk #4 succeeded at 82 (offset 4 lines).
  Hunk #5 succeeded at 101 (offset 4 lines).
  Hunk #6 succeeded at 118 (offset 4 lines).
  2 out of 6 hunks FAILED -- saving rejects to file
content/browser/web_contents/web_drag_source_win.cc.rej

Patch:       content/browser/web_contents/web_drag_source_win.cc
Index: content/browser/web_contents/web_drag_source_win.cc
diff --git a/content/browser/web_contents/web_drag_source_win.cc
b/content/browser/web_contents/web_drag_source_win.cc
index
18c18819da7ffdd4d85c4a5e9f853005dda425f5..ec4faa207f1f0c0cf655f5359cc21266bfc2f681
100644
--- a/content/browser/web_contents/web_drag_source_win.cc
+++ b/content/browser/web_contents/web_drag_source_win.cc
@@ -6,11 +6,11 @@
 
 #include "base/bind.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/web_contents/web_drag_utils_win.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
-#include "content/public/browser/web_contents.h"
 
 using WebKit::WebDragOperationNone;
 
@@ -35,7 +35,7 @@ WebDragSource::WebDragSource(gfx::NativeWindow source_wnd,
                              WebContents* web_contents)
     : ui::DragSourceWin(),
       source_wnd_(source_wnd),
-      render_view_host_(web_contents->GetRenderViewHost()),
+      web_contents_(static_cast<WebContentsImpl*>(web_contents)),
       effect_(DROPEFFECT_NONE) {
   registrar_.Add(this, NOTIFICATION_WEB_CONTENTS_SWAPPED,
                  Source<WebContents>(web_contents));
@@ -55,15 +55,15 @@ void WebDragSource::OnDragSourceCancel() {
     return;
   }
 
-  if (!render_view_host_)
+  if (!web_contents_)
     return;
 
   gfx::Point client;
   gfx::Point screen;
   GetCursorPositions(source_wnd_, &client, &screen);
-  render_view_host_->DragSourceEndedAt(client.x(), client.y(),
-                                       screen.x(), screen.y(),
-                                       WebDragOperationNone);
+  web_contents_->DragSourceEndedAt(client.x(), client.y(),
+                                   screen.x(), screen.y(),
+                                   WebDragOperationNone);
 }
 
 void WebDragSource::OnDragSourceDrop() {
@@ -78,15 +78,14 @@ void WebDragSource::OnDragSourceDrop() {
 }
 
 void WebDragSource::DelayedOnDragSourceDrop() {
-  if (!render_view_host_)
+  if (!web_contents_)
     return;
 
   gfx::Point client;
   gfx::Point screen;
   GetCursorPositions(source_wnd_, &client, &screen);
-  render_view_host_->DragSourceEndedAt(
-      client.x(), client.y(), screen.x(), screen.y(),
-      WinDragOpToWebDragOp(effect_));
+  web_contents_->DragSourceEndedAt(client.x(), client.y(), screen.x(),
+                                   screen.y(), WinDragOpToWebDragOp(effect_));
 }
 
 void WebDragSource::OnDragSourceMove() {
@@ -98,14 +97,14 @@ void WebDragSource::OnDragSourceMove() {
     return;
   }
 
-  if (!render_view_host_)
+  if (!web_contents_)
     return;
 
   gfx::Point client;
   gfx::Point screen;
   GetCursorPositions(source_wnd_, &client, &screen);
-  render_view_host_->DragSourceMovedTo(client.x(), client.y(),
-                                       screen.x(), screen.y());
+  web_contents_->DragSourceMovedTo(client.x(), client.y(),
+                                   screen.x(), screen.y());
 }
 
 void WebDragSource::Observe(int type,
@@ -115,12 +114,12 @@ void WebDragSource::Observe(int type,
     // When the WebContents get swapped, our render view host goes away.
     // That's OK, we can continue the drag, we just can't send messages back to
     // our drag source.
-    render_view_host_ = NULL;
+    web_contents_ = NULL;
   } else if (type == NOTIFICATION_WEB_CONTENTS_DISCONNECTED) {
     // This could be possible when we close the tab and the source is still
     // being used in DoDragDrop at the time that the virtual file is being
     // downloaded.
-    render_view_host_ = NULL;
+    web_contents_ = NULL;
   }
 }

[commit-bot: I haz the power, 2013-04-29 21:14:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/155001

[commit-bot: I haz the power, 2013-04-29 22:19:55]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-04-30 15:53:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/176001

[commit-bot: I haz the power, 2013-04-30 16:02:24]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_rel_de...

[commit-bot: I haz the power, 2013-05-03 14:00:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/187001

[commit-bot: I haz the power, 2013-05-03 14:28:42]

Retried try job too often on ios_dbg_simulator for step(s) googleurl_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-05-03 14:30:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mthiesse@chromium.org/12086095/187001

[commit-bot: I haz the power, 2013-05-03 16:24:43]

Change committed as 198119