Make start page fill app list bounds.

Make start page fill app list bounds.

This CL makes the start page fully fill the app list bounds. This will
ensure the start page WebContents has maximal space. In order to do this,
this CL decouples code from some layout assumptions and uses more
future-proof calculations instead.

BUG=453205
Committed: https://crrev.com/05c5d5ab671cca266f088f743970604a12ef940b
Cr-Commit-Position: refs/heads/master@{#315273}

[calamity, 2015-02-09 03:32:45]

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

[Matt Giuca, 2015-02-09 03:51:14]

https://codereview.chromium.org/889403005/diff/1/ui/app_list/views/contents_a...
File ui/app_list/views/contents_animator.cc (right):

https://codereview.chromium.org/889403005/diff/1/ui/app_list/views/contents_a...
ui/app_list/views/contents_animator.cc:39: gfx::Rect
ContentsAnimator::GetOnscreenPageBounds(int page_index) const {
I think it's better to simply move GetOnscreenPageBounds to contents_view_.
Then:
a) You don't need to be calling contents_view_->method() on every single call in
this function, and
b) You don't need to expose it to ContentsView as a method of DefaultAnimator.

Also, it paves the way to a proper refactor, where the decision to return
ContentsBounds vs DefaultContentsBounds is made by the ContentsPage (different
subclasses override differently) versus doing it with an if statement.

[calamity, 2015-02-09 05:25:14]

https://codereview.chromium.org/889403005/diff/1/ui/app_list/views/contents_a...
File ui/app_list/views/contents_animator.cc (right):

https://codereview.chromium.org/889403005/diff/1/ui/app_list/views/contents_a...
ui/app_list/views/contents_animator.cc:39: gfx::Rect
ContentsAnimator::GetOnscreenPageBounds(int page_index) const {
On 2015/02/09 03:51:14, Matt Giuca wrote:
> I think it's better to simply move GetOnscreenPageBounds to contents_view_.
> Then:
> a) You don't need to be calling contents_view_->method() on every single call
in
> this function, and
> b) You don't need to expose it to ContentsView as a method of DefaultAnimator.
> 
> Also, it paves the way to a proper refactor, where the decision to return
> ContentsBounds vs DefaultContentsBounds is made by the ContentsPage (different
> subclasses override differently) versus doing it with an if statement.

SGTM. I'm going to move OffscreenBounds too.

[Matt Giuca, 2015-02-09 05:32:17]

lgtm

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

https://codereview.chromium.org/889403005/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:357: return (state ==
AppListModel::STATE_CUSTOM_LAUNCHER_PAGE ||
To match the style of GetOffscreenPageBounds, store the condition here in a bool
variable and then use that in a second statement?

[calamity, 2015-02-09 05:53:20]

New patchsets have been uploaded after l-g-t-m from mgiuca@chromium.org

[calamity, 2015-02-09 06:12:27]

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

https://codereview.chromium.org/889403005/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:357: return (state ==
AppListModel::STATE_CUSTOM_LAUNCHER_PAGE ||
On 2015/02/09 05:32:17, Matt Giuca wrote:
> To match the style of GetOffscreenPageBounds, store the condition here in a
bool
> variable and then use that in a second statement?

Done.

[calamity, 2015-02-09 06:12:32]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2015-02-09 06:12:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/889403005/40001

[commit-bot: I haz the power, 2015-02-09 06:41:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-09 06:41:34]

Patchset 3 (id:??) landed as
https://crrev.com/05c5d5ab671cca266f088f743970604a12ef940b
Cr-Commit-Position: refs/heads/master@{#315273}