Correctly record TouchView metrics on suspend/shutdown

ash/wm/maximize_mode/maximize_mode_controller.h

 // 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.
 
 #ifndef ASH_WM_MAXIMIZE_MODE_MAXIMIZE_MODE_CONTROLLER_H_
 #define ASH_WM_MAXIMIZE_MODE_MAXIMIZE_MODE_CONTROLLER_H_
 
 #include "ash/ash_export.h"
 #include "ash/shell_observer.h"
 #include "base/macros.h"
@@ skipping 79 matching lines @@
   void SuspendImminent() override;
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 #endif  // OS_CHROMEOS
 
  private:
   friend class MaximizeModeControllerTest;
   friend class MaximizeModeWindowManagerTest;
   friend class test::MultiUserWindowManagerChromeOSTest;
   friend class test::VirtualKeyboardControllerTest;
 
+  // Used for recording metrics for intervals of time spent in
+  // and out of TouchView.
+  enum TouchViewIntervalType {

[flackr, 2015/02/12 19:59:06]

Why not a bool if we only have active an inactive? I.e. bool touchview_active
param on the record usage method?

[tdanderson, 2015/02/12 20:30:49]

I prefer enums over bools in cases where it would improve readability at call
sites. RecordTouchViewUsageInterval(TOUCH_VIEW_INTERVAL_ACTIVE) is more
self-documenting than RecordTouchViewUsageInterval(true). But if you prefer that
I use a bool instead, I'm ok with that too.

[flackr, 2015/02/12 20:44:43]

Can also use RecordTouchViewUsageInterval(/* touchview_active */ true) which is
fairly prevalent in codebase:
https://code.google.com/p/chromium/codesearch#search/&q=%5C/%5C*%5C%20%5B%5E*...
of course not strictly enforced. I don't have a strong preference though.

[jonross, 2015/02/12 20:49:26]

using a bool would clean up some of the logic in the *.cc file.

[tdanderson, 2015/02/12 21:38:22]

Rob and I had a chat offline and he's OK leaving this as an enum.

+    TOUCH_VIEW_INTERVAL_INACTIVE,
+    TOUCH_VIEW_INTERVAL_ACTIVE
+  };
+
   // Set the TickClock. This is only to be used by tests that need to
   // artificially and deterministically control the current time.
   void SetTickClockForTest(scoped_ptr<base::TickClock> tick_clock);
 
 #if defined(OS_CHROMEOS)
   // Detect hinge rotation from base and lid accelerometers and automatically
   // start / stop maximize mode.
   void HandleHingeRotation(const chromeos::AccelerometerUpdate& update);
 #endif
 
   // Returns true if the lid was recently opened.
   bool WasLidOpenedRecently() const;
 
   // Enables MaximizeModeWindowManager, and determines the current state of
   // rotation lock.
   void EnterMaximizeMode();
 
   // Removes MaximizeModeWindowManager and resets the display rotation if there
   // is no rotation lock.
   void LeaveMaximizeMode();
 
-  // Record UMA stats tracking touchview usage.
-  void RecordTouchViewStateTransition();
+  // Record UMA stats tracking TouchView usage. If |type| is
+  // TOUCH_VIEW_INTERVAL_INACTIVE, then record that TouchView has been
+  // inactive from |touchview_usage_interval_start_time_| until now.
+  // Similarly, record that TouchView has been active if |type| is
+  // TOUCH_VIEW_INTERVAL_ACTIVE.
+  void RecordTouchViewUsageInterval(TouchViewIntervalType type);
 
   // The maximized window manager (if enabled).
   scoped_ptr<MaximizeModeWindowManager> maximize_mode_window_manager_;
 
   // A helper class which when instantiated will block native events from the
   // internal keyboard and touchpad.
   scoped_ptr<ScopedDisableInternalMouseAndKeyboard> event_blocker_;
 
   // Whether we have ever seen accelerometer data.
   bool have_seen_accelerometer_data_;
 
   // True when the hinge angle has been detected past 180 degrees.
   bool lid_open_past_180_;
 
   // Tracks time spent in (and out of) touchview mode.
-  base::Time last_touchview_transition_time_;
+  base::Time touchview_usage_interval_start_time_;
   base::TimeDelta total_touchview_time_;
   base::TimeDelta total_non_touchview_time_;
 
   // Tracks the last time we received a lid open event. This is used to suppress
   // erroneous accelerometer readings as the lid is opened but the accelerometer
   // reports readings that make the lid to appear near fully open.
   base::TimeTicks last_lid_open_time_;
 
   // Source for the current time in base::TimeTicks.
   scoped_ptr<base::TickClock> tick_clock_;
 
   // Tracks when the lid is closed. Used to prevent entering maximize mode.
   bool lid_is_closed_;
 
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeController);
 };
 
 }  // namespace ash
 
 #endif  // ASH_WM_MAXIMIZE_MODE_MAXIMIZE_MODE_CONTROLLER_H_