Report user actions when gesture starts and stops in user_model.

components/scheduler/renderer/user_model.cc

 // Copyright 2015 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 "components/scheduler/renderer/user_model.h"
 
 #include "base/metrics/histogram_macros.h"
+#include "base/metrics/user_metrics.h"
+#include "base/single_thread_task_runner.h"
 
 namespace scheduler {
 
 namespace {
 // This enum is used to back a histogram, and should therefore be treated as
 // append-only.
 enum GesturePredictionResult {
   GESTURE_OCCURED_WAS_PREDICTED = 0,
   GESTURE_OCCURED_BUT_NOT_PREDICTED = 1,
   GESTURE_PREDICTED_BUT_DID_NOT_OCCUR = 2,
   GESTURE_PREDICTION_RESULT_COUNT = 3
 };
 
 void RecordGesturePrediction(GesturePredictionResult result) {
   UMA_HISTOGRAM_ENUMERATION(
       "RendererScheduler.UserModel.GesturePredictedCorrectly", result,
       GESTURE_PREDICTION_RESULT_COUNT);
 }
 
 }  // namespace
 
-UserModel::UserModel()
-    : pending_input_event_count_(0),
+UserModel::UserModel(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
+    : task_runner_(task_runner),
+      pending_input_event_count_(0),
       is_gesture_active_(false),
       is_gesture_expected_(false) {}
 UserModel::~UserModel() {}
 
 void UserModel::DidStartProcessingInputEvent(blink::WebInputEvent::Type type,
                                              const base::TimeTicks now) {
   last_input_signal_time_ = now;
   if (type == blink::WebInputEvent::TouchStart ||
       type == blink::WebInputEvent::GestureScrollBegin ||
       type == blink::WebInputEvent::GesturePinchBegin) {
     // Only update stats once per gesture.
     if (!is_gesture_active_) {
       last_gesture_start_time_ = now;
 
+      // NULL when testing.

[Sami, 2016/02/10 10:50:40]

Could you instead pass in a mock task runner in the test to avoid having to
specialize for tests here?

[beaudoin, 2016/02/11 22:03:27]

Done.

+      if (task_runner_) {
+        task_runner_->PostTask(FROM_HERE, base::Bind(&base::RecordAction,

[Ilya Sherman, 2016/02/09 22:03:25]

This is pretty weirdly wrapped -- is this clang-formatted?

[beaudoin, 2016/02/11 22:03:27]

Forgot to run it, oops.

Done.

+            base::UserMetricsAction(

[Sami, 2016/02/10 10:50:40]

Is there a version of UserMetricsAction that takes an explicit timestamp? I
think the post task round trip might introduce significant error (several
hundred ms) in the results, especially if the main thread is busy. With the
timestamp API we could also update the statistics later on the correct thread
instead of posting a task.

Alternatively we could use the control task runner instead of the default one
since it is always high priority.

[Ilya Sherman, 2016/02/10 22:43:04]

The PostTask delay is only part of the total delay -- there's also the IPC
message.  In general, user actions have not been used to try to measure very
fine timings -- the timestamps are meant to provide a general idea of user event
sequences.  We can definitely think about extending the API; but if you need
fine timings, that might suggest that user actions are not really the right
model for representing these metrics.

[beaudoin, 2016/02/11 15:30:01]

[Answered above]

+                "RendererScheduler.UserModel.GestureStart")));
+      }
+
       RecordGesturePrediction(is_gesture_expected_
                                   ? GESTURE_OCCURED_WAS_PREDICTED
                                   : GESTURE_OCCURED_BUT_NOT_PREDICTED);
 
       if (!last_reset_time_.is_null()) {
         base::TimeDelta time_since_reset = now - last_reset_time_;
         UMA_HISTOGRAM_MEDIUM_TIMES(
             "RendererScheduler.UserModel.GestureStartTimeSinceModelReset",
             time_since_reset);
       }
@@ skipping 29 matching lines @@
   // metric that tracks its duration.
   if (type == blink::WebInputEvent::GestureScrollEnd ||
       type == blink::WebInputEvent::GesturePinchEnd ||
       type == blink::WebInputEvent::GestureFlingStart ||
       type == blink::WebInputEvent::TouchEnd) {
     // Only update stats once per gesture.
     if (is_gesture_active_) {
       base::TimeDelta duration = now - last_gesture_start_time_;
       UMA_HISTOGRAM_TIMES("RendererScheduler.UserModel.GestureDuration",
                           duration);
+      // NULL when testing.

[Ilya Sherman, 2016/02/09 22:03:25]

Are there any tests that do end up exercising this code?  Should there be?

[alex clarke (OOO till 29th), 2016/02/10 10:17:01]

Looks like it would be possible to test this via AddActionCallback.

[beaudoin, 2016/02/11 15:30:01]

From a quick look, many tests seems to be called with Gesture*End but I don't
think any ensures is_gesture_active_ == true before they do. Therefore I'd say
this specific line is not exercised.

Not sure it's worth checking the mock itself.

+      if (task_runner_) {
+        task_runner_->PostTask(FROM_HERE, base::Bind(&base::RecordAction,
+            base::UserMetricsAction("RendererScheduler.UserModel.GestureEnd")));
+      }
     }
     is_gesture_active_ = false;
   }
 
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
                  "is_gesture_active", is_gesture_active_);
 
   pending_input_event_count_++;
 }
 
@@ skipping 109 matching lines @@
                    (last_gesture_expected_start_time_ - base::TimeTicks())
                        .InMillisecondsF());
   state->SetDouble("last_reset_time",
                    (last_reset_time_ - base::TimeTicks()).InMillisecondsF());
   state->SetBoolean("is_gesture_expected", is_gesture_expected_);
   state->SetBoolean("is_gesture_active", is_gesture_active_);
   state->EndDictionary();
 }
 
 }  // namespace scheduler