Experimental app list: Change transition animations of top-level pages.

Experimental app list: Change transition animations of top-level pages.

Now the start page always slides in from the top, while the other pages
slide in from the bottom. (Previously they were all arranged in order
from top to bottom.)

BUG=406222
Committed: https://crrev.com/38204f53daed8312985a1728b73ac1d39af0047e
Cr-Commit-Position: refs/heads/master@{#294134}

[Matt Giuca, 2014-09-09 08:52:00]

mgiuca@chromium.org changed reviewers:
+ calamity@chromium.org

[calamity, 2014-09-09 09:13:21]

https://codereview.chromium.org/555753002/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:223:
view_model_->view_at(target_page)->SetBoundsRect(target_page_rect);
I think you need to do an IsExperimentalAppListEnabled() somewhere.

[Matt Giuca, 2014-09-10 01:45:36]

PTAL.

https://codereview.chromium.org/555753002/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:223:
view_model_->view_at(target_page)->SetBoundsRect(target_page_rect);
Done.

Whoops.

[calamity, 2014-09-10 01:56:40]

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:193: int ContentsView::PageOrigin(const
gfx::Rect& bounds, int page_index) const {
This is cleaner without |bounds| and I don't think calling GetContentsBounds()
here every time will have a noticeable performance impact.

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.h (right):

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.h:92: bool IndexIsNamedPage(int page_index,
NamedPage named_page) const;
Can we have GetPageIndexForNamedPage() return -1 if the NamedPage isn't found to
avoid bloating the API?

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.h:134: int PageOrigin(const gfx::Rect& bounds,
int page_index) const;
Can this return a gfx::Rect() so that it's actually an origin?

Maybe change the name to (Get)?OffscreenPageBounds().

[Matt Giuca, 2014-09-10 03:15:30]

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:193: int ContentsView::PageOrigin(const
gfx::Rect& bounds, int page_index) const {
On 2014/09/10 01:56:40, calamity wrote:
> This is cleaner without |bounds| and I don't think calling GetContentsBounds()
> here every time will have a noticeable performance impact.

Done.

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.h (right):

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.h:92: bool IndexIsNamedPage(int page_index,
NamedPage named_page) const;
On 2014/09/10 01:56:40, calamity wrote:
> Can we have GetPageIndexForNamedPage() return -1 if the NamedPage isn't found
to
> avoid bloating the API?

Done.

https://codereview.chromium.org/555753002/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_view.h:134: int PageOrigin(const gfx::Rect& bounds,
int page_index) const;
On 2014/09/10 01:56:40, calamity wrote:
> Can this return a gfx::Rect() so that it's actually an origin?
> 
> Maybe change the name to (Get)?OffscreenPageBounds().

Done & Done.

[calamity, 2014-09-10 04:37:21]

lgtm w/ nit.

https://codereview.chromium.org/555753002/diff/100001/ui/app_list/views/conte...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/100001/ui/app_list/views/conte...
ui/app_list/views/contents_view.cc:221: target_page_rect.set_y((1 - progress) *
target_page_rect.y());
Use gfx::RectValueBetween().

[Matt Giuca, 2014-09-10 04:53:33]

https://codereview.chromium.org/555753002/diff/100001/ui/app_list/views/conte...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/555753002/diff/100001/ui/app_list/views/conte...
ui/app_list/views/contents_view.cc:221: target_page_rect.set_y((1 - progress) *
target_page_rect.y());
On 2014/09/10 04:37:21, calamity wrote:
> Use gfx::RectValueBetween().

Done.

(Thanks, I was looking for something like this.)

[Matt Giuca, 2014-09-10 04:53:38]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-09-10 04:55:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mgiuca@chromium.org/555753002/120001

[commit-bot: I haz the power, 2014-09-10 06:51:32]

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

[commit-bot: I haz the power, 2014-09-10 06:51:33]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Matt Giuca, 2014-09-10 07:02:54]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-09-10 07:03:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mgiuca@chromium.org/555753002/120001

[commit-bot: I haz the power, 2014-09-10 07:39:19]

Committed patchset #7 (id:120001) as 5a4ec7c84d6fcdb0946dd5e6c59f5454632831ff

[commit-bot: I haz the power, 2014-09-10 07:47:13]

Patchset 7 (id:??) landed as
https://crrev.com/38204f53daed8312985a1728b73ac1d39af0047e
Cr-Commit-Position: refs/heads/master@{#294134}