Make SearchResultPageView animation clip to its onscreen bounds.

Make SearchResultPageView animation clip to its onscreen bounds.

This CL makes the search page clip to its onscreen bounds during animations.
This prevents the search results from animating in behind the search box when
performing a search from the apps page.

BUG=439853
Committed: https://crrev.com/9ac9541476f4ea1ea6493331b377c4732c79da4a
Cr-Commit-Position: refs/heads/master@{#307872}

[calamity, 2014-12-08 05:11:29]

Patchset #1 (id:1) has been deleted

[calamity, 2014-12-09 07:08:32]

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

[Matt Giuca, 2014-12-09 07:34:18]

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

https://codereview.chromium.org/781833005/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_animator.cc:130:
contents_view()->GetPageIndexForState(AppListModel::STATE_SEARCH_RESULTS);
This doesn't do what the CL description says, which is that *all* pages clip to
their target bounds. Why is this only the search view? Is there a reason not to
do it everywhere?

(I don't really like having this search view specific code here. If nothing
else, probably should add another method like the below two, for view-specific
logic.)

Also, the comment here and CL description is wrong: it isn't clipping to
*target* bounds (on the animations where search results are exiting, that would
mean it's clipped to an off-screen box). It is clipping to its *on-screen*
bounds. Please update.

[calamity, 2014-12-10 05:23:10]

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

https://codereview.chromium.org/781833005/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_animator.cc:130:
contents_view()->GetPageIndexForState(AppListModel::STATE_SEARCH_RESULTS);
On 2014/12/09 07:34:18, Matt Giuca wrote:
> This doesn't do what the CL description says, which is that *all* pages clip
to
> their target bounds. Why is this only the search view? Is there a reason not
to
> do it everywhere?
> 
Oopz. Yeah, I changed it. It looked bad when the start page was animating off
the screen. Basically, the apps would vanish _before_ they reached the top of
the screen. It's probably easier to leave it like this and re-evaluate once the
animation plans are finalized.
> (I don't really like having this search view specific code here. If nothing
> else, probably should add another method like the below two, for view-specific
> logic.)
> 
Ehhhhh. Ok. Seems like unnecessary indirection though.
> Also, the comment here and CL description is wrong: it isn't clipping to
> *target* bounds (on the animations where search results are exiting, that
would
> mean it's clipped to an off-screen box). It is clipping to its *on-screen*
> bounds. Please update.
Roger roger.

[Matt Giuca, 2014-12-10 06:53:39]

lgtm

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

https://codereview.chromium.org/781833005/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_animator.h:66: void ClipPageToOnscreenBounds(int
page_index,
Can you rename this to ClipSearchResultsPageToOnscreenBounds and update the
comment?

[calamity, 2014-12-11 04:41:08]

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

https://codereview.chromium.org/781833005/diff/40001/ui/app_list/views/conten...
ui/app_list/views/contents_animator.h:66: void ClipPageToOnscreenBounds(int
page_index,
On 2014/12/10 06:53:39, Matt Giuca wrote:
> Can you rename this to ClipSearchResultsPageToOnscreenBounds and update the
> comment?

Done.

[calamity, 2014-12-11 04:41:12]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-12-11 04:42:35]

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

[commit-bot: I haz the power, 2014-12-11 04:52:22]

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

[commit-bot: I haz the power, 2014-12-11 04:52:23]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[calamity, 2014-12-11 05:53:53]

Patchset #3 (id:60001) has been deleted

[calamity, 2014-12-11 05:53:57]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-12-11 05:54:49]

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

[commit-bot: I haz the power, 2014-12-11 06:40:42]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2014-12-11 06:41:31]

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