Enable maximize mode when keyboard is open past 180 degrees.

Enable maximize mode when keyboard is open past 180 degrees.

BUG=342911
TEST=MaximizeModeControllerTest.*
TEST=Open keyboard past 180 degrees, device should auto-enter maximize mode.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261645

[flackr, 2014-04-01 20:54:00]

I will refactor this later to own the maximize mode controller, however, I'd
like to keep the change small to improve the chances of getting this merged
back. Please take a look. In https://codereview.chromium.org/196413017/ oshima
recommended I do lid rotation elsewhere so I will base the rotation patch on
this and add rotation to maximize_mode_controller and we can later rename
everything to touch_view* instead of maximize_mode*.

[Mr4D (OOO till 08-26), 2014-04-01 22:05:49]

lgtm. Please have a look at my comments though.

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:43:
Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
This was fired periodically and not on state change - right? If not, you would
need to check the initial state.

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:57: gfx::DotProduct(base,
kHingeVector) / base.Length()) * kRadiansToDegrees;
Just checking - What was the frequency for the event? Would it be possible to go
rapidly from one state to another leading to the state where we ignore a state
change?

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:62: // Compute the angle
between the base and the lid.
You might want to pull this out into a separate function which returns the
angle.

[oshima, 2014-04-01 22:29:08]

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:37: const gfx::Vector3dF
kHingeVector(0.0f, 1.0f, 0.0f);
won't this create static initializer?

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:57: gfx::DotProduct(base,
kHingeVector) / base.Length()) * kRadiansToDegrees;
On 2014/04/01 22:05:50, Mr4D wrote:
> Just checking - What was the frequency for the event? Would it be possible to
go
> rapidly from one state to another leading to the state where we ignore a state
> change?

I agree with Stefan's concern. I guess it's already taken care of but
documentation will be helpful.

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.h (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.h:12: namespace internal {
let's kill "internal" namespace. I talked to several people and all agreed that
this
namespace isn't working and we should use other mechanism (DPES/public) if we're
serious about it.

[flackr, 2014-04-02 21:24:30]

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:43:
Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
On 2014/04/01 22:05:50, Mr4D wrote:
> This was fired periodically and not on state change - right? If not, you would
> need to check the initial state.

Yes, this is always fired periodically.

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:57: gfx::DotProduct(base,
kHingeVector) / base.Length()) * kRadiansToDegrees;
On 2014/04/01 22:29:08, oshima wrote:
> On 2014/04/01 22:05:50, Mr4D wrote:
> > Just checking - What was the frequency for the event? Would it be possible
to
> go
> > rapidly from one state to another leading to the state where we ignore a
state
> > change?

Even before I added a separate enter and exit threshold angle, the rapid
toggling seemed to work flawlessly :-).

> 
> I agree with Stefan's concern. I guess it's already taken care of but
> documentation will be helpful.

The update currently happens 10 times per second, but as commented on the
kExitMaximizeModeAngle, there are different thresholds for entering / exiting so
that we don't rapidly toggle. oshima, do you think more documentation is
necessary?

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:62: // Compute the angle
between the base and the lid.
On 2014/04/01 22:05:50, Mr4D wrote:
> You might want to pull this out into a separate function which returns the
> angle.

Good call, made two helpers for shortest angle and "clockwise" angle. Done.

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.h (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.h:12: namespace internal {
On 2014/04/01 22:29:08, oshima wrote:
> let's kill "internal" namespace. I talked to several people and all agreed
that
> this
> namespace isn't working and we should use other mechanism (DPES/public) if
we're
> serious about it.

Done.

[oshima, 2014-04-02 22:11:29]

lgtm

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:57: gfx::DotProduct(base,
kHingeVector) / base.Length()) * kRadiansToDegrees;
On 2014/04/02 21:24:30, flackr wrote:
> On 2014/04/01 22:29:08, oshima wrote:
> > On 2014/04/01 22:05:50, Mr4D wrote:
> > > Just checking - What was the frequency for the event? Would it be possible
> to
> > go
> > > rapidly from one state to another leading to the state where we ignore a
> state
> > > change?
> 
> Even before I added a separate enter and exit threshold angle, the rapid
> toggling seemed to work flawlessly :-).
> 
> > 
> > I agree with Stefan's concern. I guess it's already taken care of but
> > documentation will be helpful.
> 
> The update currently happens 10 times per second, but as commented on the
> kExitMaximizeModeAngle, there are different thresholds for entering / exiting
so
> that we don't rapidly toggle. oshima, do you think more documentation is
> necessary?

I think the mode flag should eventually move here if this class makes the
decision?

[flackr, 2014-04-03 02:52:14]

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
File ash/wm/maximize_mode/maximize_mode_controller.cc (right):

https://codereview.chromium.org/197883011/diff/60001/ash/wm/maximize_mode/max...
ash/wm/maximize_mode/maximize_mode_controller.cc:57: gfx::DotProduct(base,
kHingeVector) / base.Length()) * kRadiansToDegrees;
On 2014/04/02 22:11:30, oshima wrote:
> On 2014/04/02 21:24:30, flackr wrote:
> > On 2014/04/01 22:29:08, oshima wrote:
> > > On 2014/04/01 22:05:50, Mr4D wrote:
> > > > Just checking - What was the frequency for the event? Would it be
possible
> > to
> > > go
> > > > rapidly from one state to another leading to the state where we ignore a
> > state
> > > > change?
> > 
> > Even before I added a separate enter and exit threshold angle, the rapid
> > toggling seemed to work flawlessly :-).
> > 
> > > 
> > > I agree with Stefan's concern. I guess it's already taken care of but
> > > documentation will be helpful.
> > 
> > The update currently happens 10 times per second, but as commented on the
> > kExitMaximizeModeAngle, there are different thresholds for entering /
exiting
> so
> > that we don't rapidly toggle. oshima, do you think more documentation is
> > necessary?
> 
> I think the mode flag should eventually move here if this class makes the
> decision?

I agree, that is what the TODO on line 73 is for. I'm just trying to avoid a
large diff on these patches which we likely want to merge back.

[flackr, 2014-04-03 15:01:05]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-04-03 15:03:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/197883011/120001

[commit-bot: I haz the power, 2014-04-03 15:40:18]

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

[commit-bot: I haz the power, 2014-04-03 15:40:18]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[flackr, 2014-04-03 17:48:52]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-04-03 17:49:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/197883011/120001

[commit-bot: I haz the power, 2014-04-03 20:23:40]

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

[commit-bot: I haz the power, 2014-04-03 20:23:40]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[flackr, 2014-04-03 21:01:39]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-04-03 21:03:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/197883011/120001

[commit-bot: I haz the power, 2014-04-03 21:03:19]

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

[commit-bot: I haz the power, 2014-04-03 21:03:20]

Failed to apply patch for ash/shell.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ash/shell.h
  Hunk #1 FAILED at 85.
  Hunk #2 succeeded at 496 (offset -9 lines).
  Hunk #3 succeeded at 723 (offset -14 lines).
  1 out of 3 hunks FAILED -- saving rejects to file ash/shell.h.rej

Patch:       ash/shell.h
Index: ash/shell.h
diff --git a/ash/shell.h b/ash/shell.h
index
b096e51d974e9a84e364960b552526f684a2486a..455a7453b549469493ee77d570185daf5a5d334b
100644
--- a/ash/shell.h
+++ b/ash/shell.h
@@ -85,6 +85,7 @@ class GPUSupport;
 class HighContrastController;
 class LockStateController;
 class MagnificationController;
+class MaximizeModeController;
 class MediaDelegate;
 class MruWindowTracker;
 class NestedDispatcherController;
@@ -505,6 +506,10 @@ class ASH_EXPORT Shell
     return accelerometer_controller_.get();
   }
 
+  MaximizeModeController* maximize_mode_controller() {
+    return maximize_mode_controller_.get();
+  }
+
 #if defined(OS_CHROMEOS)
   // TODO(oshima): Move these objects to DisplayController.
   ui::OutputConfigurator* output_configurator() {
@@ -733,6 +738,8 @@ class ASH_EXPORT Shell
 #endif  // defined(USE_X11)
 #endif  // defined(OS_CHROMEOS)
 
+  scoped_ptr<MaximizeModeController> maximize_mode_controller_;
+
   // |native_cursor_manager_| is owned by |cursor_manager_|, but we keep a
   // pointer to vend to test code.
   AshNativeCursorManager* native_cursor_manager_;

[flackr, 2014-04-03 21:54:20]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-04-03 21:55:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/197883011/140001

[commit-bot: I haz the power, 2014-04-04 03:28:07]

Change committed as 261645