Adding UMA logging to touch text selection

ui/touch_selection/touch_selection_controller.cc

Index: ui/touch_selection/touch_selection_controller.cc
diff --git a/ui/touch_selection/touch_selection_controller.cc b/ui/touch_selection/touch_selection_controller.cc
index d53f4095e05195aa24e52ff1f6ea2a45770a8632..e9dc2af203d819175e61e39f57a90becafc688e3 100644
--- a/ui/touch_selection/touch_selection_controller.cc
+++ b/ui/touch_selection/touch_selection_controller.cc
@@ -6,6 +6,7 @@
 
 #include "base/auto_reset.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 
 namespace ui {
 namespace {
@@ -55,12 +56,15 @@ TouchSelectionController::TouchSelectionController(
       activate_selection_automatically_(false),
       selection_empty_(false),
       selection_editable_(false),
-      temporarily_hidden_(false) {
+      temporarily_hidden_(false),
+      selection_handle_dragged_(false) {
   DCHECK(client_);
   HideAndDisallowShowingAutomatically();
 }
 
 TouchSelectionController::~TouchSelectionController() {
+  if (is_selection_active_)
+    LogSelectionEnd();
 }
 
 void TouchSelectionController::OnSelectionBoundsChanged(
@@ -239,6 +243,7 @@ void TouchSelectionController::OnHandleDragBegin(const TouchHandle& handle) {
     base = start_selection_handle_->position() + GetStartLineOffset();
     extent = end_selection_handle_->position() + GetEndLineOffset();
   }
+  selection_handle_dragged_ = true;
 
   // When moving the handle we want to move only the extent point. Before doing
   // so we must make sure that the base point is set correctly.
@@ -385,7 +390,13 @@ void TouchSelectionController::ActivateSelection() {
   // an entirely new selection, notify the client but avoid sending an
   // intervening SELECTION_CLEARED update to avoid unnecessary state changes.
   if (!is_selection_active_ || response_pending_input_event_ == LONG_PRESS) {
+    if (is_selection_active_) {
+      // The active selection session finishes with the start of the new one.
+      LogSelectionEnd();
+    }
     is_selection_active_ = true;
+    selection_handle_dragged_ = false;
+    selection_start_time_ = base::TimeTicks::Now();
     response_pending_input_event_ = INPUT_EVENT_TYPE_NONE;
     client_->OnSelectionEvent(SELECTION_SHOWN, GetStartPosition());
   }
@@ -396,6 +407,7 @@ void TouchSelectionController::DeactivateSelection() {
     return;
   DCHECK(start_selection_handle_);
   DCHECK(end_selection_handle_);
+  LogSelectionEnd();
   start_selection_handle_->SetEnabled(false);
   end_selection_handle_->SetEnabled(false);
   is_selection_active_ = false;
@@ -442,4 +454,22 @@ TouchHandle::AnimationStyle TouchSelectionController::GetAnimationStyle(
              : TouchHandle::ANIMATION_NONE;
 }
 
+void TouchSelectionController::LogSelectionEnd() {
+  // Don't log the duration for the case when the selection handles are not
+  // dragged at all during the selection session - it's not very interesting,
+  // plus this may be an accidental selection.
+  // TODO(mfomitchev): Once we are able to tell between 'successful' and
+  // 'unsuccessful' selections - only log the time stats for successful
+  // selection. Successful selections are the ones ending with an execution of a
+  // copy/cut/paste command.
+  if (selection_handle_dragged_) {

[jdduke (slow), 2015/02/09 19:24:48]

I was thinking we'd only log the cumulative duration of the act of dragging?
This now has different semantics than Aura's "Event.TouchSelectionDuration", so
I'd rather just not make any changes to TouchSelectionController for now.

[mfomitchev, 2015/02/09 19:39:26]

I don't like logging individual drags b/c that can make the UX look better than
it actually is if the user needs to performs a sequence of drags instead of one.
Semantics for Aura vs Android were already different - b/c Aura only logs time
for 'successful' selections.
If we don't make any changes, we won't have any data on how the selection
granularity strategy change affects the UX on Android. There are practical
reasons we'd like to have that information.. Perhaps we can call the Android
metric something else instead if different semantics with Aura is a serious
concern?

+    base::TimeDelta duration = base::TimeTicks::Now() - selection_start_time_;
+    UMA_HISTOGRAM_CUSTOM_TIMES("Event.TouchSelectionDuration",
+                               duration,
+                               base::TimeDelta::FromMilliseconds(1),
+                               base::TimeDelta::FromSeconds(60),
+                               50);
+  }
+}
+
 }  // namespace ui