GuestView: Move Disable Drag and Drop Out to Chrome

GuestView: Move Disable Drag and Drop Out to Chrome

BUG=364141, 330264
TBR=jam@chromium.org for content/public/browser_plugin_guest_delegate code deletion.

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

[lazyboy, 2014-05-30 00:30:52]

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
File chrome/browser/guest_view/guest_view_base.cc (right):

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
chrome/browser/guest_view/guest_view_base.cc:230: // Disable drag-and-drop by
default. Derived classes that override this method
Don't think this is a good way of doing this: "you need to override
DidStopLoading to disable drag and drop"
Also this changes behavior: Every guest view to have dnd enabled instead of just
WebViewGuest.

I think we should add a GuestViewBase::IsDNDEnabled() and check that before
running this script. It's cleaner to understand what's going on that way.

[Fady Samuel, 2014-05-30 01:53:21]

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
File chrome/browser/guest_view/guest_view_base.cc (right):

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
chrome/browser/guest_view/guest_view_base.cc:230: // Disable drag-and-drop by
default. Derived classes that override this method
On 2014/05/30 00:30:52, lazyboy wrote:
> Don't think this is a good way of doing this: "you need to override
> DidStopLoading to disable drag and drop"
> Also this changes behavior: Every guest view to have dnd enabled instead of
just
> WebViewGuest.
> 
> I think we should add a GuestViewBase::IsDNDEnabled() and check that before
> running this script. It's cleaner to understand what's going on that way.

You got the logic reversed. This disables DND by default. Overriding this method
enables it.

[lazyboy, 2014-05-30 03:11:39]

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
File chrome/browser/guest_view/guest_view_base.cc (right):

https://chromiumcodereview.appspot.com/301303003/diff/1/chrome/browser/guest_...
chrome/browser/guest_view/guest_view_base.cc:230: // Disable drag-and-drop by
default. Derived classes that override this method
On 2014/05/30 01:53:21, Fady Samuel wrote:
> On 2014/05/30 00:30:52, lazyboy wrote:
> > Don't think this is a good way of doing this: "you need to override
> > DidStopLoading to disable drag and drop"
> > Also this changes behavior: Every guest view to have dnd enabled instead of
> just
> > WebViewGuest.
> > 
> > I think we should add a GuestViewBase::IsDNDEnabled() and check that before
> > running this script. It's cleaner to understand what's going on that way.
> 
> You got the logic reversed. This disables DND by default. Overriding this
method
> enables it. 

Yeah I misread.
But my concern is still the same, I'm not concerned about dnd being enabled or
disabled by default; Overriding DidStopLoad magically changing the behavior of
enable/disable dnd would seem very surprising to a Derived class imo.
e.g. XViewGuest gets dnd disabled by default but if they want to do sth on
loadstop and and later override DidStopLoading then dnd gets enabled which
doesn't seem intuitive.

[Fady Samuel, 2014-05-30 16:48:09]

PTAL

[lazyboy, 2014-05-30 17:06:43]

lgtm

[Fady Samuel, 2014-05-30 18:51:57]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2014-05-30 18:52:25]

The CQ bit was unchecked by fsamuel@chromium.org

[Fady Samuel, 2014-05-30 18:52:26]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:53:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/301303003/20001

[commit-bot: I haz the power, 2014-05-31 00:57:20]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-31 03:57:17]

Change committed as 274002