Prevents panels attached to shelf from docking

Removes individual instances of Details structure in WindowResizer subclasses and adds DragDetails to WindowState. It is possible to access the current WindowResizer during the drag via window_state->drag_details()->window_resizer.
Simplifies construction of WindowResizer chain by creating DragDetails once at a point where creation of the resizer chain is imminent (after all checks that would return empty WindowResizer object are made).

Keeps track of whether currently dragged window is a panel attached to the shelf and if so prevents it from indicating that it can be docked (and from getting docked).

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

[varkha, 2014-01-03 20:24:45]

flackr@, what do you think about this? I have tried adjusting panel attachment
to only happen below the top edge of the shelf and I didn't like it - not
intuitive and didn't look visually nice. I think this (disabling docking and not
showing the docking guide) might be the least of the troubles. Please take a
look.

[varkha, 2014-01-06 20:47:22]

oshima@, can you please take a look? I wasn't sure if passing data between the
resizers in the same chain through WindowState is the best but couldn't think of
something just as simple. Thanks!

[oshima, 2014-01-08 01:21:42]

can't we move should_attach to WindowResizer::Details,
add WindowResizer

WindowState->window_resizer()->details.should_attach ?
(could be WindowState->drag_details().should_attach)

(I think it's ok for WindowResizer to have details as most of resizer have the
details)

Also how about should_attach_(to_)shelf? I think it's better name than
panel_should_attach.

[oshima, 2014-01-08 01:25:11]

On 2014/01/08 01:21:42, oshima wrote:
> can't we move should_attach to WindowResizer::Details,
> add WindowResizer
> 
> WindowState->window_resizer()->details.should_attach ?
> (could be WindowState->drag_details().should_attach)

Or simply store DragDetails to WindowState. If we can replace WindowResizer in
WindowState with DragDetails, then that'd be better.
 
> (I think it's ok for WindowResizer to have details as most of resizer have the
> details)
> 
> Also how about should_attach_(to_)shelf? I think it's better name than
> panel_should_attach.

[varkha, 2014-01-09 04:15:51]

>> Or simply store DragDetails to WindowState. If we can replace WindowResizer
in
>> WindowState with DragDetails, then that'd be better.

Done. I think this CL includes https://codereview.chromium.org/127053005/ so if
this has merits and you wait on 127053005 then we could avoid a merge. PTAL.

[oshima, 2014-01-09 19:13:45]

thanks looks good.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_details.h
File ash/wm/drag_details.h (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_details.h#n...
ash/wm/drag_details.h:26: DragDetails();
where is this constructor used?

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_window_resi...
File ash/wm/drag_window_resizer.cc (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_window_resi...
ash/wm/drag_window_resizer.cc:176: }
can you move them to separate drag_details.cc ?

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_resizer.cc
File ash/wm/window_resizer.cc (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_resizer.c...
ash/wm/window_resizer.cc:153: : window_state_(window_state) {
DCHECK to make sure window_state has drag details?

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h
File ash/wm/window_state.h (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:177: bool is_dragged() const { return !!window_resizer();
}
can you instead use drag_details_ ?

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:238: }
It may be better to have

void CreateDragDetails(args...);
void DeleteDragDetails(); // or ClearDragDetails

instead of letting caller create DragDetails and pass. WDYT?

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:245: WindowResizer* window_resizer() const {
Can you try to remove const? (If it causes a lot of error, then nvm)

[varkha, 2014-01-10 01:34:50]

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_details.h
File ash/wm/drag_details.h (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_details.h#n...
ash/wm/drag_details.h:26: DragDetails();
On 2014/01/09 19:13:45, oshima wrote:
> where is this constructor used?

Done. It was not used - removed.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_window_resi...
File ash/wm/drag_window_resizer.cc (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/drag_window_resi...
ash/wm/drag_window_resizer.cc:176: }
On 2014/01/09 19:13:45, oshima wrote:
> can you move them to separate drag_details.cc ?

I assume this comment is about constructor and destructor of DragDetails. Done.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_resizer.cc
File ash/wm/window_resizer.cc (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_resizer.c...
ash/wm/window_resizer.cc:153: : window_state_(window_state) {
On 2014/01/09 19:13:45, oshima wrote:
> DCHECK to make sure window_state has drag details?

Done.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h
File ash/wm/window_state.h (right):

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:177: bool is_dragged() const { return !!window_resizer();
}
On 2014/01/09 19:13:45, oshima wrote:
> can you instead use drag_details_ ?

I could, but I wanted to preserve the semantics during the resizer creation so
that is_dragged still returns false until the complete chain of resizers is
created.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:238: }
On 2014/01/09 19:13:45, oshima wrote:
> It may be better to have
> 
> void CreateDragDetails(args...);
> void DeleteDragDetails(); // or ClearDragDetails
> 
> instead of letting caller create DragDetails and pass. WDYT?

Done. Yes, this allows fewer places to know about DragDetails.

https://codereview.chromium.org/121153003/diff/200001/ash/wm/window_state.h#n...
ash/wm/window_state.h:245: WindowResizer* window_resizer() const {
On 2014/01/09 19:13:45, oshima wrote:
> Can you try to remove const? (If it causes a lot of error, then nvm)

Done. This forced me to use drag_details_->window_resizer in is_dragged() but
that was all.

[oshima, 2014-01-10 02:28:51]

lgtm with nits.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc
File ash/wm/drag_details.cc (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc#...
ash/wm/drag_details.cc:1: // Copyright (c) 2013 The Chromium Authors. All rights
reserved.
ditto

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc#...
ash/wm/drag_details.cc:49: restore_bounds(gfx::Rect()),
I believe you don't need this.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.h
File ash/wm/drag_details.h (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.h#n...
ash/wm/drag_details.h:1: // Copyright (c) 2013 The Chromium Authors. All rights
reserved.
nit: nuke (c)

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc
File ash/wm/window_state.cc (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc#...
ash/wm/window_state.cc:259: window, point_in_parent, window_component, source));
how about:

drag_details_.reset(new ....)
if (drag_details_->is_resiable)
   return true;
drag_details_.reset();
return false;

if you don't like, you may leave it as is.

[oshima, 2014-01-10 02:41:38]

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc
File ash/wm/window_state.cc (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc#...
ash/wm/window_state.cc:262: drag_details_.reset(details.release());
Forgot to mention. If you want to keep this way, please use Pass() instead.

[varkha, 2014-01-10 06:13:07]

Thanks, this was interesting and the end result is less code - always a good
thing!

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc
File ash/wm/drag_details.cc (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc#...
ash/wm/drag_details.cc:1: // Copyright (c) 2013 The Chromium Authors. All rights
reserved.
On 2014/01/10 02:28:52, oshima wrote:
> ditto

Done.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.cc#...
ash/wm/drag_details.cc:49: restore_bounds(gfx::Rect()),
On 2014/01/10 02:28:52, oshima wrote:
> I believe you don't need this.

Done.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.h
File ash/wm/drag_details.h (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/drag_details.h#n...
ash/wm/drag_details.h:1: // Copyright (c) 2013 The Chromium Authors. All rights
reserved.
On 2014/01/10 02:28:52, oshima wrote:
> nit: nuke (c)

Done.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc
File ash/wm/window_state.cc (right):

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc#...
ash/wm/window_state.cc:259: window, point_in_parent, window_component, source));
On 2014/01/10 02:28:52, oshima wrote:
> how about:
> 
> drag_details_.reset(new ....)
> if (drag_details_->is_resiable)
>    return true;
> drag_details_.reset();
> return false;
> 
> if you don't like, you may leave it as is.

I prefer not to expose the details pointer even temporarily through
window_state->drag_details() accessor during DragDetails construction unless the
target is resizable. As it stands this is non-issue since is_resizable is just a
member but it seemed more explicit to me.

https://codereview.chromium.org/121153003/diff/410001/ash/wm/window_state.cc#...
ash/wm/window_state.cc:262: drag_details_.reset(details.release());
On 2014/01/10 02:41:38, oshima wrote:
> Forgot to mention. If you want to keep this way, please use Pass() instead.

Done.

[commit-bot: I haz the power, 2014-01-10 06:14:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varkha@chromium.org/121153003/420002

[commit-bot: I haz the power, 2014-01-10 07:20:51]

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

[commit-bot: I haz the power, 2014-01-10 15:27:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varkha@chromium.org/121153003/880001

[commit-bot: I haz the power, 2014-01-10 17:03:23]

Retried try job too often on win_rel for step(s) browser_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-10 17:17:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varkha@chromium.org/121153003/880001

[commit-bot: I haz the power, 2014-01-10 21:41:28]

Change committed as 244246