AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc: https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q/edit?ts=56d0e9f0#heading=h.b2znwtuxd2x2

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
Committed: https://crrev.com/412803081c9d4f4321d267133512f45ce434fc10
Cr-Commit-Position: refs/heads/master@{#386123}

[mfomitchev, 2016-04-06 22:04:21]

mfomitchev@chromium.org changed reviewers:
+ mgiuca@chromium.org, sky@chromium.org

[mfomitchev, 2016-04-06 22:04:22]

@sky, would you be able to review this? @xiyuan is not the owner ash/wm files..

[mfomitchev, 2016-04-07 01:36:27]

Description was changed from

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
==========

to

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
==========

[mfomitchev, 2016-04-07 01:36:28]

mfomitchev@chromium.org changed reviewers:
+ xiyuan@chromium.org

[mfomitchev, 2016-04-07 01:37:24]

I guess it would be best if @xiyuan reviewed this first (and then @sky can
rubberstamp-lgtm)

[xiyuan, 2016-04-07 18:13:22]

lgtm

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate.cc (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.cc:245:
view_->SetAnchorPoint(GetCenterOfDisplayForView(
nit: wrap with {}

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate.h (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.h:11: #include "base/compiler_specific.h"
nit: unused?

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.h:31: namespace test {
nit: insert an empty line before

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate_factory.h (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate_factory.h:28:
scoped_ptr<app_list::AppListShowerDelegate> GetDelegate(
nit: I just learned from my other review that we should use std::unqiue_ptr and
include <memory> instead of "scoped_ptr.h". 

https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/roY78iTblYc

[mfomitchev, 2016-04-07 19:23:28]

@sky, can you PTAL?

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate.cc (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.cc:245:
view_->SetAnchorPoint(GetCenterOfDisplayForView(
On 2016/04/07 18:13:22, xiyuan wrote:
> nit: wrap with {}

Done.

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate.h (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.h:11: #include "base/compiler_specific.h"
On 2016/04/07 18:13:22, xiyuan wrote:
> nit: unused?

Done.

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate.h:31: namespace test {
On 2016/04/07 18:13:22, xiyuan wrote:
> nit: insert an empty line before

Done.

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
File ash/wm/app_list_shower_delegate_factory.h (right):

https://codereview.chromium.org/1856943003/diff/80001/ash/wm/app_list_shower_...
ash/wm/app_list_shower_delegate_factory.h:28:
scoped_ptr<app_list::AppListShowerDelegate> GetDelegate(
Cool, thanks! Done.

[sky, 2016-04-07 20:11:16]

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
File ash/wm/app_list_shower_delegate.h (right):

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
ash/wm/app_list_shower_delegate.h:5: #ifndef ASH_WM_APP_LIST_SHOWER_DELEGATE_H_
Is there a reason you want this in the ash/wm directory? It doesn't seem wm
specific. How about ash/app_list?

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
File ash/wm/app_list_shower_delegate_factory.h (right):

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
ash/wm/app_list_shower_delegate_factory.h:26:
scoped_ptr<AppListViewDelegateFactory> view_delegate_factory);
nit: as you use unique_ptr on 29, use unique_ptr here too. Also, include
#include <utility>.

[mfomitchev, 2016-04-08 15:45:07]

Patchset #7 (id:120001) has been deleted

[mfomitchev, 2016-04-08 15:47:41]

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
File ash/wm/app_list_shower_delegate.h (right):

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
ash/wm/app_list_shower_delegate.h:5: #ifndef ASH_WM_APP_LIST_SHOWER_DELEGATE_H_
On 2016/04/07 20:11:16, sky wrote:
> Is there a reason you want this in the ash/wm directory? It doesn't seem wm
> specific. How about ash/app_list?

Makes sense, done. The only reason I put it in ash/wm was that this is where
app_list_controller lived.

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
File ash/wm/app_list_shower_delegate_factory.h (right):

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
ash/wm/app_list_shower_delegate_factory.h:26:
scoped_ptr<AppListViewDelegateFactory> view_delegate_factory);
On 2016/04/07 20:11:16, sky wrote:
> nit: as you use unique_ptr on 29, use unique_ptr here too. Also, include
> #include <utility>.

Oops, replaced with unique_ptr, thanks.
unique_ptr is defined in <memory> though, I don't think we need to include
<utility>.
http://en.cppreference.com/w/cpp/memory/unique_ptr
If you are looking at the chromium-dev PSA, it listed <utility> in error (which
was later corrected).

[sky, 2016-04-08 16:35:33]

LGTM

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
File ash/wm/app_list_shower_delegate_factory.h (right):

https://codereview.chromium.org/1856943003/diff/100001/ash/wm/app_list_shower...
ash/wm/app_list_shower_delegate_factory.h:26:
scoped_ptr<AppListViewDelegateFactory> view_delegate_factory);
On 2016/04/08 15:47:40, mfomitchev wrote:
> On 2016/04/07 20:11:16, sky wrote:
> > nit: as you use unique_ptr on 29, use unique_ptr here too. Also, include
> > #include <utility>.
> 
> Oops, replaced with unique_ptr, thanks.
> unique_ptr is defined in <memory> though, I don't think we need to include
> <utility>.
> http://en.cppreference.com/w/cpp/memory/unique_ptr
> If you are looking at the chromium-dev PSA, it listed <utility> in error
(which
> was later corrected).

Indeed. I got the <utility> include from an early email from Daniel, I didn't
see the cc's that said use memory. Sorry.

[mfomitchev, 2016-04-08 17:08:46]

The CQ bit was checked by mfomitchev@chromium.org

[mfomitchev, 2016-04-08 17:08:47]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1856943003/#ps160001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-04-08 17:09:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1856943003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1856943003/160001

[commit-bot: I haz the power, 2016-04-08 17:50:15]

Description was changed from

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
==========

to

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
==========

[commit-bot: I haz the power, 2016-04-08 17:50:17]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2016-04-08 17:52:05]

Description was changed from

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408
==========

to

==========
AppListController refactoring part 2: Ash's AppListShowerDelegate imlementation.

Part 1 of the refactoring: https://crrev.com/1830293002
Design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca...

This CL adds ash::AppListShowerDelegate, which is the Ash implementation of
app_list::AppListShowerDelegate. At this point it is unused - Ash is still using
the old ash::AppListController class to control the app list.

BUG=557408

Committed: https://crrev.com/412803081c9d4f4321d267133512f45ce434fc10
Cr-Commit-Position: refs/heads/master@{#386123}
==========

[commit-bot: I haz the power, 2016-04-08 17:52:05]

Patchset 8 (id:??) landed as
https://crrev.com/412803081c9d4f4321d267133512f45ce434fc10
Cr-Commit-Position: refs/heads/master@{#386123}

[beaudoin1, 2016-04-08 19:44:04]

A revert of this CL (patchset #8 id:160001) has been created in
https://codereview.chromium.org/1872893002/ by beaudoin@google.com.

The reason for reverting is: Suspected to be causing that Mac problem:
https://build.chromium.org/p/chromium/builders/Mac/builds/14169.

[beaudoin, 2016-04-08 20:12:18]

A revert of this CL (patchset #8 id:160001) has been created in
https://codereview.chromium.org/1873903003/ by beaudoin@chromium.org.

The reason for reverting is: 
The reason for reverting is: Suspected to be causing that Mac problem:
https://build.chromium.org/p/chromium/builders/Mac/builds/14169.

[beaudoin, 2016-04-08 20:19:20]

A revert of this CL (patchset #8 id:160001) has been created in
https://codereview.chromium.org/1872033002/ by beaudoin@chromium.org.

The reason for reverting is: Trying revert again now that dependent patch was
reverted..