Hook up TABLET_MODE switch to maximize_mode_controller

ash/wm/maximize_mode/maximize_mode_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/wm/maximize_mode/maximize_mode_controller.h"
 
 #include <utility>
 
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/accelerators/accelerator_table.h"
@@ skipping 85 matching lines @@
 }
 #endif  // OS_CHROMEOS
 
 }  // namespace
 
 MaximizeModeController::MaximizeModeController()
     : have_seen_accelerometer_data_(false),
       lid_open_past_180_(false),
       touchview_usage_interval_start_time_(base::Time::Now()),
       tick_clock_(new base::DefaultTickClock()),
+#if defined(OS_CHROMEOS)
+      tablet_mode_(false),
+#endif
       lid_is_closed_(false) {
   Shell* shell = Shell::GetInstance();
   shell->AddShellObserver(this);
   shell->metrics()->RecordUserMetricsAction(
       ash::UMA_MAXIMIZE_MODE_INITIALLY_DISABLED);
 
 #if defined(OS_CHROMEOS)
   // TODO(jonross): Do not create MaximizeModeController if the flag is
   // unavailable. This will require refactoring
   // IsMaximizeModeWindowManagerEnabled to check for the existance of the
@@ skipping 101 matching lines @@
 }
 
 void MaximizeModeController::LidEventReceived(bool open,
                                               const base::TimeTicks& time) {
   if (open)
     last_lid_open_time_ = time;
   lid_is_closed_ = !open;
   LeaveMaximizeMode();
 }
 
+void MaximizeModeController::TabletModeEventReceived(
+    bool on,
+    const base::TimeTicks& time) {
+  if (on) {
+    tablet_mode_ = true;
+    EnterMaximizeMode();
+  } else {
+    tablet_mode_ = false;
+    // Set lid_open_past_180_ so that we get a chance to decide whether to
+    // leave maximize mode.
+    lid_open_past_180_ = true;

[Daniel Erat, 2016/06/03 15:00:18]

this doesn't seem right. this member feels like it should only be updated by
HandleHingeRotation().

[jonross, 2016/06/03 16:27:24]

Though the lid_open_past_180_ may actually be what the sensor is telling us.

[jcliang, 2016/06/06 04:39:22]

This is removed. I use another tablet_mode_swithc_was_on_ to achieve the same
result in HandleHingeRotation.

+  }
+}
+
 void MaximizeModeController::SuspendImminent() {
   // The system is about to suspend, so record TouchView usage interval metrics
   // based on whether TouchView mode is currently active.
   RecordTouchViewUsageInterval(CurrentTouchViewIntervalType());
 }
 
 void MaximizeModeController::SuspendDone(
     const base::TimeDelta& sleep_duration) {
   // We do not want TouchView usage metrics to include time spent in suspend.
   touchview_usage_interval_start_time_ = base::Time::Now();
@@ skipping 79 matching lines @@
     // status as set by the touch-view-testing keyboard shortcut.
     return;
   }
 
   if (IsMaximizeModeWindowManagerEnabled())
     return;
   EnableMaximizeModeWindowManager(true);
 }
 
 void MaximizeModeController::LeaveMaximizeMode() {
+#if defined(OS_CHROMEOS)
+  if (tablet_mode_)

[Daniel Erat, 2016/06/03 15:00:18]

i don't think i've looked at this code before, but adding this check here,
rather than in the places that call LeaveMaximizeMode(), seems a bit weird. i'd
prefer that the logic to decide whether we should be in maximize mode or not
didn't live inside of this method. (a more common approach might be something
like an UpdateState() method that looks at various factors to decide which mode
it should be in and then enters or leaves the mode.)

[jonross, 2016/06/03 16:27:24]

The 'update state' method is HandleHingeRotation. Which could be a more
appropriate place for this exit. These methods are separated out for use with a
testing flag, and for unittesting.

[Daniel Erat, 2016/06/03 16:59:51]

what if the "decide what to do" code from HandleHingeRotation were separated out
into a new UpdateState or SelectMode or whatever method that
HandleHingeRotation, TabletModeEventReceived, etc. call after updating the
lid_open_past_180_, tablet_mode_, etc. members?

[jcliang, 2016/06/06 04:39:22]

This is removed. Everything is handled in HandleHingeRotation now.

+    return;
+#endif
+
   event_blocker_.reset();
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kAshEnableTouchViewTesting)) {
     // We don't let accelerometer updates interfere with the maximize mode
     // status as set by the touch-view-testing keyboard shortcut.
     return;
   }
 
   if (!IsMaximizeModeWindowManagerEnabled())
@@ skipping 79 matching lines @@
   return elapsed_time.InSeconds() <= kLidRecentlyOpenedDurationSeconds;
 }
 
 void MaximizeModeController::SetTickClockForTest(
     std::unique_ptr<base::TickClock> tick_clock) {
   DCHECK(tick_clock_);
   tick_clock_ = std::move(tick_clock);
 }
 
 }  // namespace ash