Report user actions when gesture starts and stops in user_model.

components/scheduler/renderer/user_model.cc

Index: components/scheduler/renderer/user_model.cc
diff --git a/components/scheduler/renderer/user_model.cc b/components/scheduler/renderer/user_model.cc
index c71e86c21d8e20f63ab20981964971f4639a08d4..d1e8a6a5002ab828a542625e0cdf702cd804d3b1 100644
--- a/components/scheduler/renderer/user_model.cc
+++ b/components/scheduler/renderer/user_model.cc
@@ -5,6 +5,8 @@
 #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 {
 
@@ -26,8 +28,10 @@ void RecordGesturePrediction(GesturePredictionResult result) {
 
 }  // 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() {}
@@ -42,6 +46,13 @@ void UserModel::DidStartProcessingInputEvent(blink::WebInputEvent::Type type,
     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);
@@ -91,6 +102,11 @@ void UserModel::DidStartProcessingInputEvent(blink::WebInputEvent::Type type,
       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;
   }