[ash] Cleanup of ScopedOverviewAnimationSettings

[ash] Cleanup of ScopedOverviewAnimationSettings

This cleanup is now possible with WmWindow gone.

BUG=722553
TEST=None (refactoring)
TBR=jamescook@chromium.org (for a trivial removal in ash/shell.cc)

Review-Url: https://codereview.chromium.org/2901903003
Cr-Commit-Position: refs/heads/master@{#474403}
Committed: https://chromium.googlesource.com/chromium/src/+/6af4d8438538098e9c0a06bd17453f0ca32ecece

[varkha, 2017-05-24 16:46:11]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

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

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[varkha, 2017-05-24 16:47:06]

The CQ bit was unchecked by varkha@chromium.org

[varkha, 2017-05-24 16:48:07]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[varkha, 2017-05-24 16:48:15]

Patchset #1 (id:1) has been deleted

[varkha, 2017-05-24 16:48:39]

varkha@chromium.org changed reviewers:
+ msw@chromium.org

[varkha, 2017-05-24 16:48:40]

msw@, thanks for the suggestion, PTAL.

[commit-bot: I haz the power, 2017-05-24 16:48:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-05-24 17:39:16]

Nice cleanup; thanks! lgtm with nits.
Removing the factory would be even nicer.

https://codereview.chromium.org/2901903003/diff/20001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/shell.cc#newcode850
ash/shell.cc:850: new ScopedOverviewAnimationSettingsFactory);
optional nit: base::MakeUnique or avoid unique_ptr altogether.

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings.h (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings.h:25: // settings for a
WmWindow given an OverviewAnimationType.
nit: "an aura::Window,"

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings_factory.cc (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings_factory.cc:10: #include
"ui/aura/window.h"
optional nit: probably not necessary

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings_factory.h (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings_factory.h:21: class
ScopedOverviewAnimationSettingsFactory {
We don't really need a factory at all, afaict. Users can just construct
ScopedOverviewAnimationSettings objects themselves.

[commit-bot: I haz the power, 2017-05-24 17:49:07]

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

[commit-bot: I haz the power, 2017-05-24 17:49:08]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[varkha, 2017-05-24 18:29:51]

Thanks!

https://codereview.chromium.org/2901903003/diff/20001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/shell.cc#newcode850
ash/shell.cc:850: new ScopedOverviewAnimationSettingsFactory);
On 2017/05/24 17:39:16, msw wrote:
> optional nit: base::MakeUnique or avoid unique_ptr altogether.

Done.

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings.h (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings.h:25: // settings for a
WmWindow given an OverviewAnimationType.
On 2017/05/24 17:39:16, msw wrote:
> nit: "an aura::Window,"

Done.

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings_factory.cc (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings_factory.cc:10: #include
"ui/aura/window.h"
On 2017/05/24 17:39:16, msw wrote:
> optional nit: probably not necessary

Acknowledged (class removed).

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
File ash/wm/overview/scoped_overview_animation_settings_factory.h (right):

https://codereview.chromium.org/2901903003/diff/20001/ash/wm/overview/scoped_...
ash/wm/overview/scoped_overview_animation_settings_factory.h:21: class
ScopedOverviewAnimationSettingsFactory {
On 2017/05/24 17:39:16, msw wrote:
> We don't really need a factory at all, afaict. Users can just construct
> ScopedOverviewAnimationSettings objects themselves.

Done.

[varkha, 2017-05-24 18:30:24]

Description was changed from

==========
[ash] Cleanup of ScopedOverviewAnimationSettings

This cleanup is now possible with WmWindow gone.

BUG=722553
TEST=None (refactoring)
==========

to

==========
[ash] Cleanup of ScopedOverviewAnimationSettings

This cleanup is now possible with WmWindow gone.

BUG=722553
TEST=None (refactoring)
TBR=jamescook@chromium.org (for a trivial removal in ash/shell.cc)
==========

[varkha, 2017-05-24 18:30:33]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 18:31:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-05-24 18:46:06]

Extra nice! It's very good to see code get simpler. lgtm

[commit-bot: I haz the power, 2017-05-24 20:10:25]

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

[commit-bot: I haz the power, 2017-05-24 20:10:26]

Dry run: This issue passed the CQ dry run.

[varkha, 2017-05-24 20:30:55]

The CQ bit was checked by varkha@chromium.org

[commit-bot: I haz the power, 2017-05-24 20:31:28]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 20:36:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495657855533520,
"parent_rev": "5bb096e9730d7ffc0fe3352fce677fafc0d7fd0c", "commit_rev":
"6af4d8438538098e9c0a06bd17453f0ca32ecece"}

[commit-bot: I haz the power, 2017-05-24 20:36:39]

Description was changed from

==========
[ash] Cleanup of ScopedOverviewAnimationSettings

This cleanup is now possible with WmWindow gone.

BUG=722553
TEST=None (refactoring)
TBR=jamescook@chromium.org (for a trivial removal in ash/shell.cc)
==========

to

==========
[ash] Cleanup of ScopedOverviewAnimationSettings

This cleanup is now possible with WmWindow gone.

BUG=722553
TEST=None (refactoring)
TBR=jamescook@chromium.org (for a trivial removal in ash/shell.cc)

Review-Url: https://codereview.chromium.org/2901903003
Cr-Commit-Position: refs/heads/master@{#474403}
Committed:
https://chromium.googlesource.com/chromium/src/+/6af4d8438538098e9c0a06bd1745...
==========

[commit-bot: I haz the power, 2017-05-24 20:36:40]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6af4d8438538098e9c0a06bd1745...