Remove separate SearchResultListView from ContentsView.

Remove separate SearchResultListView from ContentsView.

This CL consolidates the SearchResultListView creation in ContentsView
into a single flow used by the classic and experimental app list. They
now both use SearchResultPageView with a SearchResultListView inside.

This change is being made to facilitate further refactoring.

BUG=455059
Committed: https://crrev.com/07feb4753f0b0459e7c2669054ccb9384062261c
Cr-Commit-Position: refs/heads/master@{#314984}

[calamity, 2015-02-04 05:57:30]

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

[Matt Giuca, 2015-02-05 04:33:18]

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
File ui/app_list/views/contents_view.cc (left):

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:230:
search_results_list_view_->OnContainerSelected(false);
Where is this logic in the SearchResultsPageView version? (I can't find it.)
Also the UpdateAutoLaunchState.

Just want to make sure that the behaviour is equivalent in the classic app list,
despite using code from experimental app list.

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:448: search_results_list_view_ ?
search_results_list_view_->GetPreferredSize()
Similarly, where is the logic to take the max of the container size and the
SRLV? (I'm not sure what this logic was for, but I can't find any equivalent
logic in the experimental version which we are now using.)

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:82: results, new
SearchResultListView(app_list_main_view_, view_delegate));
nit: Newline here.

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

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_page_view.cc:64: views::View* view_to_add =
result_container;
view_to_add = IsExperimentalAppListEnabled ? new SCV(...) : result_container;

(and maybe embed that into the next line so you don't have do name a variable.)

[calamity, 2015-02-05 07:08:41]

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
File ui/app_list/views/contents_view.cc (left):

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:230:
search_results_list_view_->OnContainerSelected(false);
On 2015/02/05 04:33:17, Matt Giuca wrote:
> Where is this logic in the SearchResultsPageView version? (I can't find it.)

SearchResultPageView calls OnContainerSelected when it gets shown.
> Also the UpdateAutoLaunchState.
> 
This is getting called on Update() which is every time a new search result comes
in.
> Just want to make sure that the behaviour is equivalent in the classic app
list,
> despite using code from experimental app list.

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:448: search_results_list_view_ ?
search_results_list_view_->GetPreferredSize()
On 2015/02/05 04:33:17, Matt Giuca wrote:
> Similarly, where is the logic to take the max of the container size and the
> SRLV? (I'm not sure what this logic was for, but I can't find any equivalent
> logic in the experimental version which we are now using.)

The logic was 'if search results bigger, then use that as size, else use apps
container'. We just use the apps container size.

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:82: results, new
SearchResultListView(app_list_main_view_, view_delegate));
On 2015/02/05 04:33:17, Matt Giuca wrote:
> nit: Newline here.

Done.

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

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_page_view.cc:64: views::View* view_to_add =
result_container;
On 2015/02/05 04:33:17, Matt Giuca wrote:
> view_to_add = IsExperimentalAppListEnabled ? new SCV(...) : result_container;
> 
> (and maybe embed that into the next line so you don't have do name a
variable.)

Compiler says no.

[Matt Giuca, 2015-02-06 05:07:14]

lgtm

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
File ui/app_list/views/contents_view.cc (left):

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
ui/app_list/views/contents_view.cc:448: search_results_list_view_ ?
search_results_list_view_->GetPreferredSize()
But that means if the search results are bigger, we no longer use the bigger
size, right? Just I'm not sure when this would happen.

Oh well, seems unnecessary to preserve that behaviour. (Is that your logic?)

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

https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/search_res...
ui/app_list/views/search_result_page_view.cc:64: views::View* view_to_add =
result_container;
On 2015/02/05 07:08:40, calamity wrote:
> On 2015/02/05 04:33:17, Matt Giuca wrote:
> > view_to_add = IsExperimentalAppListEnabled ? new SCV(...) :
result_container;
> > 
> > (and maybe embed that into the next line so you don't have do name a
> variable.)
> 
> Compiler says no.

Acknowledged.


(Compiler is a bastard.)

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

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2015-02-06 05:09:44]

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

[calamity, 2015-02-06 05:10:31]

On 2015/02/06 05:07:14, Matt Giuca wrote:
> lgtm
> 
>
https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
> File ui/app_list/views/contents_view.cc (left):
> 
>
https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/contents_v...
> ui/app_list/views/contents_view.cc:448: search_results_list_view_ ?
> search_results_list_view_->GetPreferredSize()
> But that means if the search results are bigger, we no longer use the bigger
> size, right? Just I'm not sure when this would happen.
> 
> Oh well, seems unnecessary to preserve that behaviour. (Is that your logic?)
> 
Yeah, unnecessary. It's not like we go through and ensure the other contents
views are considered either. AGV is just the canonical sizing view (which it
effectively always has been).
>
https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/search_res...
> File ui/app_list/views/search_result_page_view.cc (right):
> 
>
https://codereview.chromium.org/896633005/diff/1/ui/app_list/views/search_res...
> ui/app_list/views/search_result_page_view.cc:64: views::View* view_to_add =
> result_container;
> On 2015/02/05 07:08:40, calamity wrote:
> > On 2015/02/05 04:33:17, Matt Giuca wrote:
> > > view_to_add = IsExperimentalAppListEnabled ? new SCV(...) :
> result_container;
> > > 
> > > (and maybe embed that into the next line so you don't have do name a
> > variable.)
> > 
> > Compiler says no.
> 
> Acknowledged.
> 
> 
> (Compiler is a bastard.)

[commit-bot: I haz the power, 2015-02-06 06:00:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-06 06:01:32]

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