App list start page is no longer a SearchResultContainerView.

App list start page is no longer a SearchResultContainerView.

This CL makes StartPageView subclass View instead of
SearchResultContainerView. A new SearchResultContainerView has been
added as a child of StartPageView instead. This change will facilitate
further refactoring.

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

[calamity, 2015-02-04 03:50:53]

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

[Matt Giuca, 2015-02-04 06:07:45]

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:66: class StartPageTilesContainer : public
SearchResultContainerView {
I think this can go inside the namespace.

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:69: : all_apps_button_(all_apps_button) {
I'm not a fan of inline methods (inside the class {} block). Please move these
out so they appear just underneath the class.

(That way, if we ever want to move this to its own file, we won't have to
rearrange this at the same time --- something I find myself doing quite a bit.)

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:116: SearchResult* item = NULL;
nullptr

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:152: tiles_container_(nullptr) {
Any reason you can't do new StartPageTilesContainer here? (Like
search_box_spacer_view).

[calamity, 2015-02-04 06:46:55]

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:66: class StartPageTilesContainer : public
SearchResultContainerView {
On 2015/02/04 06:07:45, Matt Giuca wrote:
> I think this can go inside the namespace.

Nope. Moved it into SPV's private instead.

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:69: : all_apps_button_(all_apps_button) {
On 2015/02/04 06:07:45, Matt Giuca wrote:
> I'm not a fan of inline methods (inside the class {} block). Please move these
> out so they appear just underneath the class.
> 
> (That way, if we ever want to move this to its own file, we won't have to
> rearrange this at the same time --- something I find myself doing quite a
bit.)

Done.

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:116: SearchResult* item = NULL;
On 2015/02/04 06:07:45, Matt Giuca wrote:
> nullptr

Done.

https://codereview.chromium.org/896063002/diff/1/ui/app_list/views/start_page...
ui/app_list/views/start_page_view.cc:152: tiles_container_(nullptr) {
On 2015/02/04 06:07:45, Matt Giuca wrote:
> Any reason you can't do new StartPageTilesContainer here? (Like
> search_box_spacer_view).

It's BIG. Done.

[calamity, 2015-02-04 07:46:07]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2015-02-04 07:47:13]

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

[commit-bot: I haz the power, 2015-02-04 07:47:14]

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

[commit-bot: I haz the power, 2015-02-04 07:47:15]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Matt Giuca, 2015-02-04 07:53:28]

lgtm

[calamity, 2015-02-05 01:33:00]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2015-02-05 01:34:07]

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

[commit-bot: I haz the power, 2015-02-05 02:47:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-05 02:48:12]

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