aura-mus: add a drag representation image.

aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently
being dragged. The server proxies this to the window manager, which
manages a window toplevel window which displays the representation at an
offset. This offset differs between mouse drags and touch drags, so also
send that data too.

BUG=548443
Review-Url: https://codereview.chromium.org/2764963003
Cr-Commit-Position: refs/heads/master@{#459466}
Committed: https://chromium.googlesource.com/chromium/src/+/b9401c4956d563f88072f370bf033695fb4bbfac

[Elliot Glaysher, 2017-03-22 00:07:06]

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

[commit-bot: I haz the power, 2017-03-22 00:07:26]

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, 2017-03-22 00:24:50]

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

[commit-bot: I haz the power, 2017-03-22 00:24:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-03-22 18:07:16]

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

[commit-bot: I haz the power, 2017-03-22 18:08:08]

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, 2017-03-22 19:04:49]

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

[commit-bot: I haz the power, 2017-03-22 19:04:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-03-22 19:25:29]

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

[commit-bot: I haz the power, 2017-03-22 19:26:04]

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, 2017-03-22 20:34:52]

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

[commit-bot: I haz the power, 2017-03-22 20:34:53]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-03-22 21:27:00]

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

[commit-bot: I haz the power, 2017-03-22 21:27:28]

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, 2017-03-22 22:11:10]

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

[commit-bot: I haz the power, 2017-03-22 22:11:11]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Elliot Glaysher, 2017-03-22 22:50:48]

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

[commit-bot: I haz the power, 2017-03-22 22:51:37]

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

[Elliot Glaysher, 2017-03-22 22:59:49]

Description was changed from

==========
aura-mus: add a drag representation image.

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

to

==========
aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently being
dragged. The server proxies this to the window manager, which manages a window
toplevel window which displays the representation at an offset. This offset
differs between mouse drags and touch drags, so also send that data too.

BUG=548443
==========

[Elliot Glaysher, 2017-03-22 22:59:59]

Description was changed from

==========
aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently being
dragged. The server proxies this to the window manager, which manages a window
toplevel window which displays the representation at an offset. This offset
differs between mouse drags and touch drags, so also send that data too.

BUG=548443
==========

to

==========
aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently
being dragged. The server proxies this to the window manager, which
manages a window toplevel window which displays the representation at an
offset. This offset differs between mouse drags and touch drags, so also
send that data too.

BUG=548443
==========

[Elliot Glaysher, 2017-03-22 23:40:13]

erg@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2017-03-23 00:05:36]

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager...
ash/mus/window_manager.cc:391: drag_image_.reset(new DragImageView(root_window,
ui_source));
MakeUnique.

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager.h
File ash/mus/window_manager.h (right):

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager...
ash/mus/window_manager.h:213: // An image representation of the contents of the
current drag and drop
Please put the drag related state into a struct. That way it's clear what state
is related to a drag.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
File services/ui/public/interfaces/window_manager.mojom (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:194: // which contains a
single image represent
'representation.' ?

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:195:
WmBuildDragImage(gfx.mojom.Point cursor_location,
cursor_location->screen_location (I'm assuming it's in screen coordinates).

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:200: // Moves the drag
representation image to |cursor_location|. Users should
'Users should' The window server queues ...

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
File services/ui/public/interfaces/window_tree_constants.mojom (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_tree_constants.mojom:60: enum
DragEventSource {
Would it make sense to replace this with PointerKind? The enum includes PEN, but
that should be a valid drag source too.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/drag_so...
File services/ui/ws/drag_source.h (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/drag_so...
services/ui/ws/drag_source.h:22: virtual void OnDragContinued(const gfx::Point&
location) = 0;
optional: OnDragMoved?

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:2145: if
(window_server_->GetCurrentDragLoopInitiator() != this)
Remind me when this can ever be false? I'm wondering if it should be a DCHECK?

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:2160: location,
base::Bind(&WindowTree::OnWmMoveDragImageCompleted,
Using 'completed' in the name here makes me think the drag is done. How about
OnWmMoveDragImageAck?

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:2161: weak_factory_.GetWeakPtr()));
Can you reset weak_factory_ once the drag is complete locally? That way we don't
have the possibility of an ack coming out of order.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
File services/ui/ws/window_tree.h (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.h:600: // Whether we're waiting for an ack from a
WmMoveDragImage message.
Similar to WindowManager please put the state related to a drag into it's own
struct.

[commit-bot: I haz the power, 2017-03-23 00:05:49]

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

[commit-bot: I haz the power, 2017-03-23 00:05:50]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-03-23 18:14:07]

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

[commit-bot: I haz the power, 2017-03-23 18:15:04]

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, 2017-03-23 18:35:17]

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

[commit-bot: I haz the power, 2017-03-23 18:35:18]

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_...)

[Elliot Glaysher, 2017-03-23 19:29:16]

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

[commit-bot: I haz the power, 2017-03-23 19:29:54]

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

[Elliot Glaysher, 2017-03-23 19:58:42]

erg@chromium.org changed reviewers:
+ tsepez@chromium.org

[Elliot Glaysher, 2017-03-23 19:58:44]

+tsepez for mojom

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager...
ash/mus/window_manager.cc:391: drag_image_.reset(new DragImageView(root_window,
ui_source));
On 2017/03/23 00:05:35, sky wrote:
> MakeUnique.

Done.

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager.h
File ash/mus/window_manager.h (right):

https://codereview.chromium.org/2764963003/diff/100001/ash/mus/window_manager...
ash/mus/window_manager.h:213: // An image representation of the contents of the
current drag and drop
On 2017/03/23 00:05:36, sky wrote:
> Please put the drag related state into a struct. That way it's clear what
state
> is related to a drag.

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
File services/ui/public/interfaces/window_manager.mojom (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:194: // which contains a
single image represent
On 2017/03/23 00:05:36, sky wrote:
> 'representation.' ?

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:195:
WmBuildDragImage(gfx.mojom.Point cursor_location,
On 2017/03/23 00:05:36, sky wrote:
> cursor_location->screen_location (I'm assuming it's in screen coordinates).

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:200: // Moves the drag
representation image to |cursor_location|. Users should
On 2017/03/23 00:05:36, sky wrote:
> 'Users should' The window server queues ...

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
File services/ui/public/interfaces/window_tree_constants.mojom (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/public/int...
services/ui/public/interfaces/window_tree_constants.mojom:60: enum
DragEventSource {
On 2017/03/23 00:05:36, sky wrote:
> Would it make sense to replace this with PointerKind? The enum includes PEN,
but
> that should be a valid drag source too.

Removed this and moved to PointerKind. I'm collapsing pen and touch into the
same ui::DragTypes::DragEventSource on both sides, but that appears to be the
current behaviour.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/drag_so...
File services/ui/ws/drag_source.h (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/drag_so...
services/ui/ws/drag_source.h:22: virtual void OnDragContinued(const gfx::Point&
location) = 0;
On 2017/03/23 00:05:36, sky wrote:
> optional: OnDragMoved?

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:2160: location,
base::Bind(&WindowTree::OnWmMoveDragImageCompleted,
On 2017/03/23 00:05:36, sky wrote:
> Using 'completed' in the name here makes me think the drag is done. How about
> OnWmMoveDragImageAck?

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:2161: weak_factory_.GetWeakPtr()));
On 2017/03/23 00:05:36, sky wrote:
> Can you reset weak_factory_ once the drag is complete locally? That way we
don't
> have the possibility of an ack coming out of order.

Done.

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
File services/ui/ws/window_tree.h (right):

https://codereview.chromium.org/2764963003/diff/100001/services/ui/ws/window_...
services/ui/ws/window_tree.h:600: // Whether we're waiting for an ack from a
WmMoveDragImage message.
On 2017/03/23 00:05:36, sky wrote:
> Similar to WindowManager please put the state related to a drag into it's own
> struct.

Done. But I left the weak_factory_ in WindowTree (and renamed) as I'm unsure if
its safe to have a factory to an object in another one. I can't think of a
reason this would be so, but...

[Tom Sepez, 2017-03-23 20:05:14]

https://codereview.chromium.org/2764963003/diff/140001/services/ui/public/int...
File services/ui/public/interfaces/window_manager.mojom (right):

https://codereview.chromium.org/2764963003/diff/140001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:200: skia.mojom.Bitmap?
drag_image,
So .. we're passing a skia bitmap up to a more-privileged process.  Is there a
precedent for this?  Thanks,

[Elliot Glaysher, 2017-03-23 20:24:21]

https://codereview.chromium.org/2764963003/diff/140001/services/ui/public/int...
File services/ui/public/interfaces/window_manager.mojom (right):

https://codereview.chromium.org/2764963003/diff/140001/services/ui/public/int...
services/ui/public/interfaces/window_manager.mojom:200: skia.mojom.Bitmap?
drag_image,
On 2017/03/23 20:05:13, Tom Sepez wrote:
> So .. we're passing a skia bitmap up to a more-privileged process.  Is there a
> precedent for this?  Thanks,

IIUC, the image decoder
(services/image_decoder/public/interfaces/image_decoder.mojom) takes a set of
raw bytes and hands back a skia.mojom.Bitmap. This service is run in a tight
sandbox and passes its data back to the browser process.

[Tom Sepez, 2017-03-23 20:41:34]

lgtm

[commit-bot: I haz the power, 2017-03-23 21:08:34]

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

[commit-bot: I haz the power, 2017-03-23 21:08:35]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-03-23 22:58:32]

ping sky

[sky, 2017-03-24 02:30:31]

LGTM

[Elliot Glaysher, 2017-03-24 17:05:55]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2017-03-24 17:06:48]

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, 2017-03-24 17:14:20]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1490375155260910,
"parent_rev": "9b34b21e479145817c93e1e4870ed2e87376eac0", "commit_rev":
"b9401c4956d563f88072f370bf033695fb4bbfac"}

[commit-bot: I haz the power, 2017-03-24 17:15:08]

Description was changed from

==========
aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently
being dragged. The server proxies this to the window manager, which
manages a window toplevel window which displays the representation at an
offset. This offset differs between mouse drags and touch drags, so also
send that data too.

BUG=548443
==========

to

==========
aura-mus: Add a visual drag representation.

This lets the client send an image representation of what's currently
being dragged. The server proxies this to the window manager, which
manages a window toplevel window which displays the representation at an
offset. This offset differs between mouse drags and touch drags, so also
send that data too.

BUG=548443

Review-Url: https://codereview.chromium.org/2764963003
Cr-Commit-Position: refs/heads/master@{#459466}
Committed:
https://chromium.googlesource.com/chromium/src/+/b9401c4956d563f88072f370bf03...
==========

[commit-bot: I haz the power, 2017-03-24 17:15:09]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/b9401c4956d563f88072f370bf03...