Moving default minimize location to the app launcher item.

Moving default minimize location to the app launcher item.

BUG=374833
TEST=visual & unit test

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

[Mr4D (OOO till 08-26), 2014-05-28 16:05:19]

Can you please have a look?
Thanks!

[flackr, 2014-05-28 18:34:00]

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations.cc
File ash/wm/window_animations.cc (right):

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations...
ash/wm/window_animations.cc:499: return gfx::Rect(work_area.x(),
work_area.bottom(), 0, 0);
I believe this is assuming an LTR language but would be wrong in RTL.

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations...
ash/wm/window_animations.cc:506: }
I think it would make more sense if GetScreenBoundsOfItemIconForWindow returned
the overflow icon if the window was overflowed, as this is the place to go to
restore an overflowed window.

[Mr4D (OOO till 08-26), 2014-05-28 21:50:40]

Please have another look after kuscher weighed in.

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations.cc
File ash/wm/window_animations.cc (right):

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations...
ash/wm/window_animations.cc:499: return gfx::Rect(work_area.x(),
work_area.bottom(), 0, 0);
Good catch. Addressed.

https://codereview.chromium.org/302653008/diff/20001/ash/wm/window_animations...
ash/wm/window_animations.cc:506: }
kuscher wanted to have all "unassociateable windows" or "windows associated with
invisible target icons" to minimize to the app launcher instead of some other
place (like the system tray). Since all apps come (more or less) out of the app
launcher this makes sense. Agree?

[flackr, 2014-05-29 14:32:28]

This default position lgtm, as discussed can you look into returning the
overflow button from ShelfView::GetIdealBoundsOfItemIcon for overflowed shelf
icons? Thanks!

[Mr4D (OOO till 08-26), 2014-05-29 16:56:54]

Added the overflow button functionality and a test.

There is actually more wrong: some apps are not properly placed upon minimize
because the ShelfID window property does not get set. Same applies for apps
which got restarted after a system restart. But that problem will be addressed
in a different patch.

Please have another look!

[flackr, 2014-05-29 17:05:00]

Looks good, one question.

https://codereview.chromium.org/302653008/diff/60001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/302653008/diff/60001/ash/shelf/shelf_view.cc#...
ash/shelf/shelf_view.cc:460: if (index > last_visible_index_ && index <
model_->FirstPanelIndex())
Does this currently work for overflowed panels? Or should this be && index <=
last_hidden_index_?

[Mr4D (OOO till 08-26), 2014-05-29 20:09:51]

Please have another look!

https://codereview.chromium.org/302653008/diff/60001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/302653008/diff/60001/ash/shelf/shelf_view.cc#...
ash/shelf/shelf_view.cc:460: if (index > last_visible_index_ && index <
model_->FirstPanelIndex())
Yes - and no. Updated comment.

But in the essence: there is another bug and as such we lose apparently overflow
panel items. Since they do not have any home, they can better point to any other
area on the screen.

[flackr, 2014-05-29 21:51:50]

lgtm

[Mr4D (OOO till 08-26), 2014-05-29 21:53:11]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-05-29 21:55:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/302653008/80001

[commit-bot: I haz the power, 2014-05-30 02:13:39]

Change committed as 273699