Draft CL at https://codereview.chromium.org/1748903003/.

sgabriel@, pkasting@ can you comment on this or in CL review? My preference would be to reset the button pressed or hovered state and not show any distinct button background in its old position during the drag. I think other drag indicators (vertical or horizontal bar and the drag image) are enough. Many other systems go as far as hiding the drag source but this could have a rolling effect on layout and I would prefer avoiding that.
We can do one of the following:
1. Reset button state when drag starts but only for MD
2. Reset button state when drag starts but only in the menus
2. Reset button state when drag starts but only for extension buttons
3. Reset button state when drag starts for all draggable buttons
4. Leave it as is - show HOVERED or PRESSED state on the source while dragging

3 would be my preference. Note that it affects bookmark buttons.

I'm confused by comment 0.  Do you mean PRESSED rather than HOVERED?  Since I assume that clicking to start a drag would move from HOVERED to PRESSED state and thus that's the state we'd be talking about clearing.

I think the weird bit here is leaving the button in the old position while also dragging a "copy" of the button around and having an insertion indicator -- nicer might be to actually move the entire button around live and relayout things as we go.  You say this could have a "rolling effect" on layout but it doesn't seem too bad to me, we just Layout() every time the button position moves.

If we don't do this, and we leave the button where it is while dragging, I don't care very much.  Your first three options (confusingly, numbered 1, 2, and 2) don't make sense, but both 3 and 4 seem about equally OK.

3 seems fine to me.

I agree with what Peter said.

Note that we actually do have this constant relayout for extension buttons on Mac.  It's doable on Views; we just haven't done it.  But it might be out of scope for this change.

In any case, I feel fairly strongly that anything we come up with shouldn't be only for extension buttons.

The behavior pkasting describes is how the apps page works. The code for that is pretty complicated because of animations, sliding between rows, and so forth. This is also how dragging tabs around in the tabstrip works, and that code's pretty complicated too (imo), despite having just a single row. So I think that behavior is desirable, but tricky to get right, and it would be worth carefully weighing the costs (complexity, maintainability, bugs) against the benefits. How often do users drag these things?

Note that the bookmarks bar doesn't do the live re-layout (at least not on Linux) and that's probably a more common drag scenario than extension actions.

> In any case, I feel fairly strongly that anything we come up with shouldn't be only for extension buttons.

There are precious few buttons in Chrome that can be dragged somewhere.

Note that in those cases things are complicated primarily because of animations.  If there's never animation (the button is either in the old position or the new one), it becomes far simpler to do this.

If you want to animate things moving around as e.g. the tabstrip does, life gets much harder.

#3, From what I see in the code the source button is in PRESSED state at the drag initiation when the action is triggered upon mouse press. It is in HOVERED state at drag initiation for the buttons that are triggering action upon mouse release. Extension buttons are programmed to act upon mouse release so they are currently stuck in HOVERED state when dragged.
If I read everybody's comments correctly I can go ahead with the option numbered "3" which would be a simple change in CustomButton::OnCaptureLost to remove the check for InDrag().
The rest are all good suggestions but I think just doing "3" would be an improvement.

#7, agreed. But I think relayout without animations (just icons jumping around) probably looks worse than what we're doing now.

I couldn't answer definitively without seeing it.  My instinct would be that it would look better, but I might be wrong.

I think it would look better, not sure how well it blends with chrome's simple aesthetics. One example is how Ubuntu dock allows reordering launcher app icons. But it definitely looks more complicated to animate it properly.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb50932d80d11040f5637b17173c072a61d15f5a

commit fb50932d80d11040f5637b17173c072a61d15f5a
Author: varkha <varkha@chromium.org>
Date: Wed Mar 02 20:10:13 2016

Resets hover state when ToolbarActionView is dragged

HOVERED state is now reset when drag is started. The fix is similar to
https://codereview.chromium.org/1517003002

BUG= 590941 

Review URL: https://codereview.chromium.org/1748903003

Cr-Commit-Position: refs/heads/master@{#378810}

[modify] https://crrev.com/fb50932d80d11040f5637b17173c072a61d15f5a/ui/views/controls/button/custom_button.cc
[modify] https://crrev.com/fb50932d80d11040f5637b17173c072a61d15f5a/ui/views/controls/button/custom_button_unittest.cc
[modify] https://crrev.com/fb50932d80d11040f5637b17173c072a61d15f5a/ui/views/widget/widget.h

Fixed for MD. I will keep it open until we have clarity if it is necessary to reset the PRESSED / HOVERED state for drag source views for non-MD platforms.
It should still get merged in M-50 since the extension buttons in app menu look bad during drag, so requesting a merge.

Your change meets the bar and is auto-approved for M50 (branch: 2661)

If you think it is a safe merge, please try to merge your change to M50 branch 2661 before 5:00 PM PST today or ASAP.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e0ad74dad20fc4fa14689d3ab62ec70d8840187

commit 5e0ad74dad20fc4fa14689d3ab62ec70d8840187
Author: Valery Arkhangorodsky <varkha@chromium.org>
Date: Tue Mar 08 01:29:11 2016

Resets hover state when ToolbarActionView is dragged

HOVERED state is now reset when drag is started. The fix is similar to
https://codereview.chromium.org/1517003002

BUG= 590941 

Review URL: https://codereview.chromium.org/1748903003

Cr-Commit-Position: refs/heads/master@{#378810}
(cherry picked from commit fb50932d80d11040f5637b17173c072a61d15f5a)

Review URL: https://codereview.chromium.org/1772183002 .

Cr-Commit-Position: refs/branch-heads/2661@{#120}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/5e0ad74dad20fc4fa14689d3ab62ec70d8840187/ui/views/controls/button/custom_button.cc
[modify] https://crrev.com/5e0ad74dad20fc4fa14689d3ab62ec70d8840187/ui/views/controls/button/custom_button_unittest.cc
[modify] https://crrev.com/5e0ad74dad20fc4fa14689d3ab62ec70d8840187/ui/views/widget/widget.h

There aren't going to be non-MD platforms soon.  We aren't going to spend time fixing this sort of thing for them; so closing.