Fix check for updating WebDragDestGtk when creating a view for a RVH.

Fix check for updating WebDragDestGtk when creating a view for a RVH.

CreateViewForWidget can be called for navigations that don't end up
swapping in the render view. An example of this would be dragging and
dropping a file on Chrome that it can't display, such as an executable.
This is problematic because it creates a WebDragDestGtk for a view that
is never actually displayed, thus breaking all drag and drop operations
until that tab is navigated. To fix this, it's not sufficient to just
check that the RVH isn't swapped out. Instead, update WebDragDestGtk
only if the RVH is already the RVH for the web contents; otherwise,
allow RenderViewSwappedIn to update it.

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

[Charlie Reis, 2013-05-17 18:23:31]

One question below.

https://codereview.chromium.org/14918012/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_view_gtk.cc (left):

https://codereview.chromium.org/14918012/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_view_gtk.cc:226: // We don't want to
change any state in this class for swapped out RVHs
Won't removing this break the first RVH?  We don't call RenderViewSwappedIn
until a cross-process navigation commits.

There's another option: instead of checking whether the RVH is swapped out
(which the pending RVH in your case is not), you could check whether the RVH is
equal to the current GetRenderViewHost() of the WebContents.  If not, then we
can wait until RenderViewSwappedIn.

[dcheng, 2013-05-18 00:33:18]

https://codereview.chromium.org/14918012/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_view_gtk.cc (left):

https://codereview.chromium.org/14918012/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_view_gtk.cc:226: // We don't want to
change any state in this class for swapped out RVHs
On 2013/05/17 18:23:31, creis wrote:
> Won't removing this break the first RVH?  We don't call RenderViewSwappedIn
> until a cross-process navigation commits.
> 
> There's another option: instead of checking whether the RVH is swapped out
> (which the pending RVH in your case is not), you could check whether the RVH
is
> equal to the current GetRenderViewHost() of the WebContents.  If not, then we
> can wait until RenderViewSwappedIn.

You're right. I've used your suggestion and updated the comment.

[Charlie Reis, 2013-05-18 01:37:59]

LGTM

https://codereview.chromium.org/14918012/diff/4001/content/browser/web_conten...
File content/browser/web_contents/web_contents_view_gtk.cc (right):

https://codereview.chromium.org/14918012/diff/4001/content/browser/web_conten...
content/browser/web_contents/web_contents_view_gtk.cc:232: // because there are
other navigations that create views that may never be
nit: that create non-swapped-out views that may never...

[dcheng, 2013-05-18 01:49:52]

https://codereview.chromium.org/14918012/diff/4001/content/browser/web_conten...
File content/browser/web_contents/web_contents_view_gtk.cc (right):

https://codereview.chromium.org/14918012/diff/4001/content/browser/web_conten...
content/browser/web_contents/web_contents_view_gtk.cc:232: // because there are
other navigations that create views that may never be
On 2013/05/18 01:37:59, creis wrote:
> nit: that create non-swapped-out views that may never...

Done.

[commit-bot: I haz the power, 2013-05-18 01:50:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/14918012/9001

[commit-bot: I haz the power, 2013-05-18 03:00:37]

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

[commit-bot: I haz the power, 2013-05-21 21:41:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/14918012/21001

[commit-bot: I haz the power, 2013-05-22 00:17:57]

Change committed as 201398