Move app list search tiles into SearchResultTileItemListView.

Move app list search tiles into SearchResultTileItemListView.

This CL factors a SearchResultTileItemListView out of
SearchResultPageView which is responsible for displaying
tile item search results in a list.

BUG=416756
Committed: https://crrev.com/74ff360d838095dd719dcd3a8a78a4ce07d3b1a5
Cr-Commit-Position: refs/heads/master@{#304009}

[calamity, 2014-11-12 05:37:35]

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

[Matt Giuca, 2014-11-13 00:08:52]

Good change!

Just need some clarification about PreferredSizeChanged and DoUpdate. (Which are
fringely related to this change; the main change I don't have a problem with!)

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_container_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_container_view.cc:30: DoUpdate();
I don't see the relevance of this change. Was this a bug before (it was not
invalidating the weak pointer factory)?

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_list_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_list_view.cc:183: PreferredSizeChanged();
Just to clarify: This has no *real* effect right now, correct? (So it isn't
changing behaviour.) It's just that, on principle, SearchResultContainerViews
should call PreferredSizeChanged() after their update?

Still a bit confused, since it seems like *before* this patch, SRLV::Update()
would not trigger a Layout() in the SRPV, but *after* this patch it will.

Another idea is maybe you should call PreferredSizeChanged() in
SearchResultContainerView::DoUpdate(), rather than manually doing it in all the
child methods?

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_page_view.h (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_page_view.h:23: class APP_LIST_EXPORT
SearchResultPageView : public views::View {
yay!

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_tile_item_list_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.cc:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
No (c).

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.cc:26: new
views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, kTileSpacing));
What happened to this line?

tiles_layout_manager->set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_tile_item_list_view.h (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.h:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
No (c).

[calamity, 2014-11-13 03:44:50]

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_container_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_container_view.cc:30: DoUpdate();
On 2014/11/13 00:08:51, Matt Giuca wrote:
> I don't see the relevance of this change. Was this a bug before (it was not
> invalidating the weak pointer factory)?

Oops. This was a relic of when I was had a PreferredSizeChanged() call in
DoUpdate(). Maybe both Layout() and PreferredSizeChanged() should move into
DoUpdate()?

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_list_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_list_view.cc:183: PreferredSizeChanged();
On 2014/11/13 00:08:52, Matt Giuca wrote:
> Just to clarify: This has no *real* effect right now, correct? (So it isn't
> changing behaviour.) It's just that, on principle, SearchResultContainerViews
> should call PreferredSizeChanged() after their update?
> 
> Still a bit confused, since it seems like *before* this patch, SRLV::Update()
> would not trigger a Layout() in the SRPV, but *after* this patch it will.
It happened to not be needed before because the SRLV always took up all the
space. Since it has to shrink properly now, the SRPV needs to Layout() to use
the new preferred size of the SRLV.

> Another idea is maybe you should call PreferredSizeChanged() in
> SearchResultContainerView::DoUpdate(), rather than manually doing it in all
the
> child methods?

Ah yeah. GMTA.

Done.

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_page_view.h (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_page_view.h:23: class APP_LIST_EXPORT
SearchResultPageView : public views::View {
On 2014/11/13 00:08:52, Matt Giuca wrote:
> yay!

But like, what if we have a SymmetricalAnimation : Animation,
ReversibleAnimation : Animation and ReversedAnimation contains only
SymmetricalAnimation?

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_tile_item_list_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.cc:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
On 2014/11/13 00:08:52, Matt Giuca wrote:
> No (c).

Done.

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.cc:26: new
views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, kTileSpacing));
On 2014/11/13 00:08:52, Matt Giuca wrote:
> What happened to this line?
> 
>
tiles_layout_manager->set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);

We left aligned now.

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_tile_item_list_view.h (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.h:1: // Copyright (c) 2014
The Chromium Authors. All rights reserved.
On 2014/11/13 00:08:52, Matt Giuca wrote:
> No (c).

Done.

[Matt Giuca, 2014-11-13 07:09:18]

lgtm, but consider moving the BoxLayout change into the next CL if it makes more
sense there.

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
File ui/app_list/views/search_result_tile_item_list_view.cc (right):

https://codereview.chromium.org/720663002/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_tile_item_list_view.cc:26: new
views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, kTileSpacing));
That sounds like something that should go into the follow-up patch
(https://codereview.chromium.org/722563002/) ... since I thought this patch was
about refactoring and that patch is about actually changing the appearance.

[calamity, 2014-11-13 07:40:36]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-11-13 07:41:22]

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

[commit-bot: I haz the power, 2014-11-13 09:22:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-13 09:22:36]

Patchset 2 (id:??) landed as
https://crrev.com/74ff360d838095dd719dcd3a8a78a4ce07d3b1a5
Cr-Commit-Position: refs/heads/master@{#304009}