Move MaximizeModeController into ash/common

Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org

Committed: https://crrev.com/e2bde21744c5cefa38133556b2fb21fecea34518
Cr-Commit-Position: refs/heads/master@{#403485}

[James Cook, 2016-07-01 01:02:04]

LGTM. Hooray! It's starting to feel like the boulder we're rolling up the hill
is finally getting close to the top.

https://codereview.chromium.org/2118643002/diff/1/ash/aura/wm_shell_aura.cc
File ash/aura/wm_shell_aura.cc (right):

https://codereview.chromium.org/2118643002/diff/1/ash/aura/wm_shell_aura.cc#n...
ash/aura/wm_shell_aura.cc:149: return base::WrapUnique(new
ScopedDisableInternalMouseAndKeyboardOzone);
ScopedDisableInternalMouseAndKeyboardOzone is my favorite identifier name today.
:-)

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
File ash/common/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
ash/common/wm/maximize_mode/maximize_mode_controller.cc:110: if (IsEnabled())
nit: cache is_enabled so you don't have to do HasSwitch() twice

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
ash/common/wm/maximize_mode/maximize_mode_controller.cc:123: if (IsEnabled())
ditto nit

[sky, 2016-07-01 15:45:37]

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
File ash/common/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
ash/common/wm/maximize_mode/maximize_mode_controller.cc:110: if (IsEnabled())
On 2016/07/01 01:02:04, James Cook wrote:
> nit: cache is_enabled so you don't have to do HasSwitch() twice

Done.

https://codereview.chromium.org/2118643002/diff/1/ash/common/wm/maximize_mode...
ash/common/wm/maximize_mode/maximize_mode_controller.cc:123: if (IsEnabled())
On 2016/07/01 01:02:04, James Cook wrote:
> ditto nit

Done.

[sky, 2016-07-01 15:45:42]

The CQ bit was checked by sky@chromium.org

[sky, 2016-07-01 15:45:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2118643002/#ps40001
(title: "feedback")

[commit-bot: I haz the power, 2016-07-01 15:45:58]

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, 2016-07-01 15:53:29]

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

[commit-bot: I haz the power, 2016-07-01 15:53:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2016-07-01 17:01:08]

Description was changed from

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org
==========

to

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org
==========

[sky, 2016-07-01 17:01:08]

sky@chromium.org changed reviewers:
+ yusukes@chromium.org

[sky, 2016-07-01 17:02:29]

One of the changes I did was look for the switch kAshEnableTouchView everywhere.
It was previously chromeos only. I did this as I couldn't think of a good reason
as to why the switch is chromeos only. Take another look?

[sky, 2016-07-01 17:02:58]

Also, TBR yusukes for include change in
components/arc/window_manager/arc_window_manager_bridge.cc

[oshima, 2016-07-01 17:09:25]

On 2016/07/01 17:02:29, sky wrote:
> One of the changes I did was look for the switch kAshEnableTouchView
everywhere.
> It was previously chromeos only. I did this as I couldn't think of a good
reason
> as to why the switch is chromeos only. Take another look?

Touch view is for chromeos devices that can turn into tablet mode like flip.
What was the problem? (I don't mind moving though

[sky, 2016-07-01 17:18:04]

I would like to be able to enable touch mode on windows for
experimentation. That likely entails a separate flag though, so if you
prefer keeping the flag chromeos for the time being I can do that.

On Fri, Jul 1, 2016 at 10:09 AM,  <oshima@chromium.org> wrote:
> On 2016/07/01 17:02:29, sky wrote:
>> One of the changes I did was look for the switch kAshEnableTouchView
> everywhere.
>> It was previously chromeos only. I did this as I couldn't think of a good
> reason
>> as to why the switch is chromeos only. Take another look?
>
> Touch view is for chromeos devices that can turn into tablet mode like flip.
> What was the problem? (I don't mind moving though
>
> https://codereview.chromium.org/2118643002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[James Cook, 2016-07-01 17:21:22]

I'm OK with the switch existing everywhere. Still LGTM.

https://codereview.chromium.org/2118643002/diff/60001/ash/common/ash_switches.h
File ash/common/ash_switches.h (right):

https://codereview.chromium.org/2118643002/diff/60001/ash/common/ash_switches...
ash/common/ash_switches.h:34: ASH_EXPORT extern const char
kAshEnableTouchView[];
Doesn't this have to move in the .cc file also?

[sky, 2016-07-01 17:30:04]

https://codereview.chromium.org/2118643002/diff/60001/ash/common/ash_switches.h
File ash/common/ash_switches.h (right):

https://codereview.chromium.org/2118643002/diff/60001/ash/common/ash_switches...
ash/common/ash_switches.h:34: ASH_EXPORT extern const char
kAshEnableTouchView[];
On 2016/07/01 17:21:21, James Cook wrote:
> Doesn't this have to move in the .cc file also?

D'OH!

[sky, 2016-07-01 17:30:24]

The CQ bit was checked by sky@chromium.org

[sky, 2016-07-01 17:30:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2118643002/#ps80001
(title: "doh")

[commit-bot: I haz the power, 2016-07-01 17:30:50]

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

[chromium-reviews, 2016-07-01 18:15:26]

No that is fine. It may also touches VK code, which may also be cros
specific now. just FYI.

On Fri, Jul 1, 2016 at 10:18 AM, Scott Violet <sky@chromium.org> wrote:

> I would like to be able to enable touch mode on windows for
> experimentation. That likely entails a separate flag though, so if you
> prefer keeping the flag chromeos for the time being I can do that.
>
> On Fri, Jul 1, 2016 at 10:09 AM,  <oshima@chromium.org> wrote:
> > On 2016/07/01 17:02:29, sky wrote:
> >> One of the changes I did was look for the switch kAshEnableTouchView
> > everywhere.
> >> It was previously chromeos only. I did this as I couldn't think of a
> good
> > reason
> >> as to why the switch is chromeos only. Take another look?
> >
> > Touch view is for chromeos devices that can turn into tablet mode like
> flip.
> > What was the problem? (I don't mind moving though
> >
> > https://codereview.chromium.org/2118643002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-07-01 18:27:19]

Description was changed from

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org
==========

to

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org
==========

[commit-bot: I haz the power, 2016-07-01 18:27:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-01 18:27:30]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 18:29:38]

Description was changed from

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org
==========

to

==========
Move MaximizeModeController into ash/common

BUG=612629
TEST=covered by tests
R=jamescook@chromium.org, yusukes@chromium.org
TBR=yusukes@chromium.org

Committed: https://crrev.com/e2bde21744c5cefa38133556b2fb21fecea34518
Cr-Commit-Position: refs/heads/master@{#403485}
==========

[commit-bot: I haz the power, 2016-07-01 18:29:40]

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