[WebUI] ClosureCompile cr.ui.DragWrapper, create a real handler class

[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/50ebc1301bf31a50d5a149dc0d0d363f83d2edf2
Cr-Commit-Position: refs/heads/master@{#407242}

[Devlin, 2016-07-19 17:22:27]

Description was changed from

==========
[WebUI] ClosureCompile cr.ui.DragWrapper, create a real handler class



BUG=
==========

to

==========
[WebUI] ClosureCompile cr.ui.DragWrapper, create a real handler class



BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Devlin, 2016-07-19 17:24:45]

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

[commit-bot: I haz the power, 2016-07-19 17:26:07]

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-07-19 17:26:08]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-19 18:20:26]

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

[commit-bot: I haz the power, 2016-07-19 18:20:27]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-07-19 19:19:16]

Description was changed from

==========
[WebUI] ClosureCompile cr.ui.DragWrapper, create a real handler class



BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Devlin, 2016-07-19 19:19:58]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2016-07-19 19:19:59]

Dan, as the proud(?) owner of ntp4 (and everything else here), this one's going
to you.  Michael, FYI.

[Dan Beam, 2016-07-19 20:59:35]

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
File ui/webui/resources/js/cr/ui/drag_wrapper.js (right):

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
ui/webui/resources/js/cr/ui/drag_wrapper.js:12: var DragWrapperHandler =
function() {};
nit: DragWrapper

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
ui/webui/resources/js/cr/ui/drag_wrapper.js:42: function DragWrapper(target,
handler) {
DragWrapperImpl

[Devlin, 2016-07-19 23:17:58]

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
File ui/webui/resources/js/cr/ui/drag_wrapper.js (right):

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
ui/webui/resources/js/cr/ui/drag_wrapper.js:12: var DragWrapperHandler =
function() {};
On 2016/07/19 20:59:35, Dan Beam wrote:
> nit: DragWrapper

Per offline conversation, changing this to DragWrapperDelegate.

https://codereview.chromium.org/2164693002/diff/1/ui/webui/resources/js/cr/ui...
ui/webui/resources/js/cr/ui/drag_wrapper.js:42: function DragWrapper(target,
handler) {
On 2016/07/19 20:59:35, Dan Beam wrote:
> DragWrapperImpl

And keeping this the same.

[Dan Beam, 2016-07-21 02:29:13]

i think this code is confusing and not clearly separated.

you're basically just asking for more things to be notified of useful drag
start/drop, right?  can we just make everything that doesn't require a response
(requires a response: shouldAcceptDrag, doesn't require a response:
doDragOver/doDragEnter) just a CustomEvent?

I just dislike the code this produces so far.  If you want the handler and its
delegate and the delegate's event target all to know when things happen... just
use EventTarget for all these, eh?

If you really want to go forward with this CL, it definitely does improve
things, I just think the overall organization and separation of this code is
weird.

https://codereview.chromium.org/2164693002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/2164693002/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:116: var dragTarget =
document.documentElement;
why not just pass |this| as the event target instead?

[Devlin, 2016-07-21 18:11:30]

On 2016/07/21 02:29:13, Dan Beam wrote:
> i think this code is confusing and not clearly separated.
> 
> you're basically just asking for more things to be notified of useful drag
> start/drop, right?  can we just make everything that doesn't require a
response
> (requires a response: shouldAcceptDrag, doesn't require a response:
> doDragOver/doDragEnter) just a CustomEvent?
> 
> I just dislike the code this produces so far.  If you want the handler and its
> delegate and the delegate's event target all to know when things happen...
just
> use EventTarget for all these, eh?
> 
> If you really want to go forward with this CL, it definitely does improve
> things, I just think the overall organization and separation of this code is
> weird.

In principal, I agree.  In practice, I don't have the time at the moment tackle
this larger change.  I'd like to go ahead with this for the moment to unblock MD
extensions work, but I've added a TODO to drag_wrapper to make it more sane.

https://codereview.chromium.org/2164693002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/2164693002/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:116: var dragTarget =
document.documentElement;
On 2016/07/21 02:29:12, Dan Beam wrote:
> why not just pass |this| as the event target instead?

Because line 41, declaring that this inherits HTMLDivElement, is a blatant lie. 
This is not a div element.  It's no where in the DOM.  It will never receive
events.  (Weird?  Yes.  But I'm not gonna bother changing this in the old
extensions page.)

So in lieu of this, I've removed line 41.

[Dan Beam, 2016-07-21 23:42:39]

lgtm

https://codereview.chromium.org/2164693002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/drag_and_drop_handler.js (right):

https://codereview.chromium.org/2164693002/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/drag_and_drop_handler.js:49:
this.fireDragEnded_();
i think it's odd to say the drag has ended if this really just means the user
has moved the mouse (while still down) out of the area we consider valid.  but
you're basically just using this like a private event channel, so I suppose it's
fine.

[Devlin, 2016-07-22 18:43:50]

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

[commit-bot: I haz the power, 2016-07-22 18:44:35]

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-07-22 18:44:37]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-22 20:04:34]

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

[commit-bot: I haz the power, 2016-07-22 20:04:35]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-07-22 20:13:08]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-07-22 20:13:39]

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-07-22 20:13:42]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-22 20:18:21]

Description was changed from

==========
[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-22 20:18:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 20:19:37]

Description was changed from

==========
[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[WebUI] ClosureCompile cr.ui.DragWrapper; create a real handler class

Pull out the extensions code drag-and-drop handler in order to reuse it
in the MD extensions page. In the process, closure compiler DragWrapper
and create a real interface for the DragWrapperHandler, which previously
was only specified in comments. Update drag wrapper handler implementors
with @override.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/50ebc1301bf31a50d5a149dc0d0d363f83d2edf2
Cr-Commit-Position: refs/heads/master@{#407242}
==========

[commit-bot: I haz the power, 2016-07-22 20:19:38]

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