Implement Main Thread RAF Aligned Input

content/renderer/input/main_thread_event_queue_unittest.cc

Index: content/renderer/input/main_thread_event_queue_unittest.cc
diff --git a/content/renderer/input/main_thread_event_queue_unittest.cc b/content/renderer/input/main_thread_event_queue_unittest.cc
index d5036cfec1dbd991c21d2ef94518a6ef30f05c72..d92e896f6c29c181e4fa33702b88cc0af68b7b7b 100644
--- a/content/renderer/input/main_thread_event_queue_unittest.cc
+++ b/content/renderer/input/main_thread_event_queue_unittest.cc
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/test/histogram_tester.h"
 #include "base/test/test_simple_task_runner.h"
 #include "build/build_config.h"
 #include "content/common/input/synthetic_web_input_event_builders.h"
@@ -36,9 +37,11 @@ namespace content {
 namespace {
 
 const int kTestRoutingID = 13;
+const char* kCoalescedCountHistogram =
+    "Event.MainThreadEventQueue.CoalescedCount";

[tdresser, 2016/08/24 13:43:00]

Missing space

[dtapuska, 2016/08/24 17:10:06]

you mean a LF? done.

 }
 
-class MainThreadEventQueueTest : public testing::Test,
+class MainThreadEventQueueTest : public testing::TestWithParam<bool>,
                                  public MainThreadEventQueueClient {
  public:
   MainThreadEventQueueTest()
@@ -46,7 +49,9 @@ class MainThreadEventQueueTest : public testing::Test,
         queue_(new MainThreadEventQueue(kTestRoutingID,
                                         this,
                                         main_task_runner_,
-                                        &renderer_scheduler_)) {}
+                                        &renderer_scheduler_,
+                                        GetParam())),
+        needs_main_frame_(false) {}

[tdresser, 2016/08/24 13:43:00]

This indentation looks off.

[dtapuska, 2016/08/24 17:10:07]

how so; this is run through git cl format.

[tdresser, 2016/08/24 17:45:16]

Nevermind.

 
   void HandleEventOnMainThread(int routing_id,
                                const blink::WebInputEvent* event,
@@ -69,6 +74,8 @@ class MainThreadEventQueueTest : public testing::Test,
                         ui::LatencyInfo(), DISPATCH_TYPE_BLOCKING, ack_result);
   }
 
+  void NeedsMainFrame(int routing_id) override { needs_main_frame_ = true; }
+
   WebInputEventQueue<EventWithDispatchType>& event_queue() {
     return queue_->events_;
   }
@@ -85,15 +92,33 @@ class MainThreadEventQueueTest : public testing::Test,
     queue_->enable_fling_passive_listener_flag_ = enable_flag;
   }
 
+  void RunPendingTasksWithSimulatedRaf() {
+    while (needs_main_frame_ || main_task_runner_->HasPendingTask()) {
+      main_task_runner_->RunUntilIdle();
+      needs_main_frame_ = false;
+      queue_->DispatchRafAlignedInput();
+    }
+  }
+
+  void RunSimulatedRafOnce() {
+    if (needs_main_frame_) {
+      needs_main_frame_ = false;
+      queue_->DispatchRafAlignedInput();
+    }
+  }
+
  protected:
   scoped_refptr<base::TestSimpleTaskRunner> main_task_runner_;
   blink::scheduler::MockRendererScheduler renderer_scheduler_;
   scoped_refptr<MainThreadEventQueue> queue_;
   std::vector<ui::ScopedWebInputEvent> handled_events_;
   std::vector<uint32_t> additional_acked_events_;
+  bool needs_main_frame_;
 };
 
-TEST_F(MainThreadEventQueueTest, NonBlockingWheel) {
+TEST_P(MainThreadEventQueueTest, NonBlockingWheel) {
+  base::HistogramTester histogram_tester;
+
   WebMouseWheelEvent kEvents[4] = {
       SyntheticWebMouseWheelEventBuilder::Build(10, 10, 0, 53, 0, false),
       SyntheticWebMouseWheelEventBuilder::Build(20, 20, 0, 53, 0, false),
@@ -108,8 +133,8 @@ TEST_F(MainThreadEventQueueTest, NonBlockingWheel) {
     HandleEvent(event, INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
 
   EXPECT_EQ(2u, event_queue().size());
-  EXPECT_TRUE(main_task_runner_->HasPendingTask());
-  main_task_runner_->RunUntilIdle();
+  EXPECT_EQ(GetParam(), !main_task_runner_->HasPendingTask());
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(2u, handled_events_.size());
@@ -137,9 +162,12 @@ TEST_F(MainThreadEventQueueTest, NonBlockingWheel) {
         WebInputEvent::DispatchType::ListenersNonBlockingPassive;
     EXPECT_EQ(coalesced_event, *last_wheel_event);
   }
+  histogram_tester.ExpectUniqueSample(kCoalescedCountHistogram, 1, 2);
 }
 
-TEST_F(MainThreadEventQueueTest, NonBlockingTouch) {
+TEST_P(MainThreadEventQueueTest, NonBlockingTouch) {
+  base::HistogramTester histogram_tester;
+
   SyntheticWebTouchEvent kEvents[4];
   kEvents[0].PressPoint(10, 10);
   kEvents[1].PressPoint(10, 10);
@@ -155,7 +183,7 @@ TEST_F(MainThreadEventQueueTest, NonBlockingTouch) {
 
   EXPECT_EQ(3u, event_queue().size());
   EXPECT_TRUE(main_task_runner_->HasPendingTask());
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(3u, handled_events_.size());
@@ -185,9 +213,13 @@ TEST_F(MainThreadEventQueueTest, NonBlockingTouch) {
   coalesced_event.dispatchType =
       WebInputEvent::DispatchType::ListenersNonBlockingPassive;
   EXPECT_EQ(coalesced_event, *last_touch_event);
+  histogram_tester.ExpectBucketCount(kCoalescedCountHistogram, 0, 1);
+  histogram_tester.ExpectBucketCount(kCoalescedCountHistogram, 1, 1);
 }
 
-TEST_F(MainThreadEventQueueTest, BlockingTouch) {
+TEST_P(MainThreadEventQueueTest, BlockingTouch) {
+  base::HistogramTester histogram_tester;
+
   SyntheticWebTouchEvent kEvents[4];
   kEvents[0].PressPoint(10, 10);
   kEvents[1].PressPoint(10, 10);
@@ -207,14 +239,17 @@ TEST_F(MainThreadEventQueueTest, BlockingTouch) {
 
   EXPECT_EQ(2u, event_queue().size());
   EXPECT_TRUE(main_task_runner_->HasPendingTask());
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
+
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(2u, additional_acked_events_.size());
   EXPECT_EQ(kEvents[2].uniqueTouchEventId, additional_acked_events_.at(0));
   EXPECT_EQ(kEvents[3].uniqueTouchEventId, additional_acked_events_.at(1));
+
+  histogram_tester.ExpectUniqueSample(kCoalescedCountHistogram, 2, 1);
 }
 
-TEST_F(MainThreadEventQueueTest, InterleavedEvents) {
+TEST_P(MainThreadEventQueueTest, InterleavedEvents) {
   WebMouseWheelEvent kWheelEvents[2] = {
       SyntheticWebMouseWheelEventBuilder::Build(10, 10, 0, 53, 0, false),
       SyntheticWebMouseWheelEventBuilder::Build(20, 20, 0, 53, 0, false),
@@ -234,8 +269,8 @@ TEST_F(MainThreadEventQueueTest, InterleavedEvents) {
   HandleEvent(kTouchEvents[1], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
 
   EXPECT_EQ(2u, event_queue().size());
-  EXPECT_TRUE(main_task_runner_->HasPendingTask());
-  main_task_runner_->RunUntilIdle();
+  EXPECT_EQ(GetParam(), !main_task_runner_->HasPendingTask());

[tdresser, 2016/08/24 13:43:00]

Save the parameter into a well named variable.

I think you can do this in the constructor, and then have a function that
returns the parameter value, which is probably the cleanest way to do it.

[dtapuska, 2016/08/24 17:10:07]

done

+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(2u, handled_events_.size());
@@ -265,7 +300,127 @@ TEST_F(MainThreadEventQueueTest, InterleavedEvents) {
   }
 }
 
-TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
+TEST_P(MainThreadEventQueueTest, RafAlignedMouseInput) {
+  // Don't run the test for when we aren't supporting raf aligned.

[tdresser, 2016/08/24 13:43:00]

Don't run the test when we aren't supporting rAF aligned input.

[dtapuska, 2016/08/24 17:10:06]

done

+  if (!GetParam())
+    return;
+
+  WebMouseEvent kMouseEvents[3] = {
+      SyntheticWebMouseEventBuilder::Build(WebInputEvent::MouseDown, 10, 10, 0),
+      SyntheticWebMouseEventBuilder::Build(WebInputEvent::MouseMove, 10, 10, 0),
+      SyntheticWebMouseEventBuilder::Build(WebInputEvent::MouseUp, 10, 10, 0),
+  };
+
+  WebMouseWheelEvent kWheelEvents[2] = {
+      SyntheticWebMouseWheelEventBuilder::Build(10, 10, 0, 53, 0, false),
+      SyntheticWebMouseWheelEventBuilder::Build(20, 20, 0, 53, 0, false),
+  };
+
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+  EXPECT_EQ(0u, event_queue().size());
+
+  // Simulate enqueing a normal event, followed by "continious" events and then

[tdresser, 2016/08/24 13:43:00]

continuous

[dtapuska, 2016/08/24 17:10:06]

done

+  // a normal event. The "continious" events should not wait for RAF.

[tdresser, 2016/08/24 13:43:00]

nittiest nit: We normally write rAF, as it abbreviates requestAnimationFrame.

[tdresser, 2016/08/24 13:43:00]

Is the above comment correct? Isn't it the discrete events that should not wait
for rAF?

[dtapuska, 2016/08/24 17:10:06]

done

+  HandleEvent(kMouseEvents[0], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  HandleEvent(kMouseEvents[1], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  HandleEvent(kWheelEvents[0], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);

[tdresser, 2016/08/24 13:43:00]

If we aren't iterating through the events, I'd rather we give them logical
names, instead of shoving them in an array.

kMouseDownEvent, kMouseMoveEvent, kMouseUpEvent.

The wheel events are fine.

[dtapuska, 2016/08/24 17:10:07]

done

+  HandleEvent(kWheelEvents[1], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  HandleEvent(kMouseEvents[2], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+
+  EXPECT_EQ(4u, event_queue().size());
+  EXPECT_TRUE(main_task_runner_->HasPendingTask());
+  EXPECT_TRUE(needs_main_frame_);
+  main_task_runner_->RunUntilIdle();
+  EXPECT_EQ(0u, event_queue().size());
+  RunPendingTasksWithSimulatedRaf();
+
+  // Simulate the RAF running before the PostTask occurs. The first RAF
+  // shouldn't do anything.
+  HandleEvent(kMouseEvents[0], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  HandleEvent(kWheelEvents[0], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  EXPECT_EQ(2u, event_queue().size());
+  EXPECT_TRUE(needs_main_frame_);
+  RunSimulatedRafOnce();
+  EXPECT_FALSE(needs_main_frame_);
+  EXPECT_EQ(2u, event_queue().size());
+  main_task_runner_->RunUntilIdle();
+  EXPECT_TRUE(needs_main_frame_);
+  EXPECT_EQ(1u, event_queue().size());
+  RunPendingTasksWithSimulatedRaf();
+  EXPECT_EQ(0u, event_queue().size());
+}
+
+TEST_P(MainThreadEventQueueTest, RafAlignedTouchInput) {
+  // Don't run the test for when we aren't supporting raf aligned.
+  if (!GetParam())
+    return;
+
+  SyntheticWebTouchEvent kEvents[3];
+  kEvents[0].PressPoint(10, 10);
+  kEvents[1].PressPoint(10, 10);
+  kEvents[1].MovePoint(0, 50, 50);
+  kEvents[2].PressPoint(10, 10);
+  kEvents[2].ReleasePoint(0);
+
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+  EXPECT_EQ(0u, event_queue().size());
+
+  // Simulate enqueing a normal event, followed by "continious" events and then
+  // a normal event. The "continious" events should not wait for RAF.
+  for (SyntheticWebTouchEvent& event : kEvents)
+    HandleEvent(event, INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+
+  EXPECT_EQ(3u, event_queue().size());
+  EXPECT_TRUE(main_task_runner_->HasPendingTask());
+  EXPECT_TRUE(needs_main_frame_);
+  main_task_runner_->RunUntilIdle();
+  EXPECT_EQ(0u, event_queue().size());
+  RunPendingTasksWithSimulatedRaf();
+
+  // Simulate the RAF running before the PostTask occurs. The first RAF
+  // shouldn't do anything.
+  HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  HandleEvent(kEvents[1], INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  EXPECT_EQ(2u, event_queue().size());
+  EXPECT_TRUE(needs_main_frame_);
+  RunSimulatedRafOnce();
+  EXPECT_FALSE(needs_main_frame_);
+  EXPECT_EQ(2u, event_queue().size());
+  RunPendingTasksWithSimulatedRaf();
+  EXPECT_EQ(0u, event_queue().size());
+}
+
+TEST_P(MainThreadEventQueueTest, RafAlignedMaxSize) {
+  // Don't run the test for when we aren't supporting raf aligned.
+  if (!GetParam())
+    return;
+
+  const size_t kNumEventsToQueue = 16;
+  WebMouseWheelEvent mouseEvent =
+      SyntheticWebMouseWheelEventBuilder::Build(10, 10, 0, 53, 0, false);
+
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+  EXPECT_EQ(0u, event_queue().size());
+
+  for (size_t i = 0; i < kNumEventsToQueue; ++i) {
+    mouseEvent.modifiers = i;
+    HandleEvent(mouseEvent, INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  }
+
+  // There is a maximum number of events we handle in a RAF. kNumEventsToQueue
+  // exceeds that. Ensure that two RAF calls need to run.
+  EXPECT_EQ(kNumEventsToQueue, event_queue().size());
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+  EXPECT_TRUE(needs_main_frame_);
+  RunSimulatedRafOnce();
+  EXPECT_TRUE(needs_main_frame_);
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+  RunSimulatedRafOnce();
+  EXPECT_EQ(0u, event_queue().size());
+  EXPECT_FALSE(main_task_runner_->HasPendingTask());
+}
+
+TEST_P(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
   SyntheticWebTouchEvent kEvents[1];
   kEvents[0].PressPoint(10, 10);
   kEvents[0].touchStartOrFirstTouchMove = true;
@@ -274,7 +429,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
 
   EXPECT_FALSE(last_touch_start_forced_nonblocking_due_to_fling());
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(1u, handled_events_.size());
@@ -289,7 +444,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
 
   kEvents[0].MovePoint(0, 30, 30);
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(2u, handled_events_.size());
@@ -305,7 +460,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
   kEvents[0].MovePoint(0, 50, 50);
   kEvents[0].touchStartOrFirstTouchMove = false;
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(3u, handled_events_.size());
@@ -319,7 +474,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
 
   kEvents[0].ReleasePoint(0);
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(4u, handled_events_.size());
@@ -332,7 +487,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesDuringFling) {
   EXPECT_EQ(kEvents[0], *last_touch_event);
 }
 
-TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
+TEST_P(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
   SyntheticWebTouchEvent kEvents[1];
   kEvents[0].PressPoint(10, 10);
   kEvents[0].touchStartOrFirstTouchMove = true;
@@ -340,7 +495,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
   set_enable_fling_passive_listener_flag(false);
 
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(1u, handled_events_.size());
@@ -356,7 +511,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
   set_is_flinging(true);
   set_enable_fling_passive_listener_flag(false);
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(2u, handled_events_.size());
@@ -373,7 +528,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
   set_is_flinging(false);
   set_enable_fling_passive_listener_flag(true);
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(3u, handled_events_.size());
@@ -389,7 +544,7 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
 
   kEvents[0].MovePoint(0, 30, 30);
   HandleEvent(kEvents[0], INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  main_task_runner_->RunUntilIdle();
+  RunPendingTasksWithSimulatedRaf();
   EXPECT_FALSE(main_task_runner_->HasPendingTask());
   EXPECT_EQ(0u, event_queue().size());
   EXPECT_EQ(4u, handled_events_.size());
@@ -403,4 +558,8 @@ TEST_F(MainThreadEventQueueTest, BlockingTouchesOutsideFling) {
   EXPECT_EQ(kEvents[0], *last_touch_event);
 }
 
-}  // namespace content
+INSTANTIATE_TEST_CASE_P(MainThreadEventQueueTests,

[tdresser, 2016/08/24 13:43:00]

Add a comment indicating what the bool means.

[dtapuska, 2016/08/24 17:10:07]

done

+                        MainThreadEventQueueTest,
+                        testing::Bool());
+
+}  // namespace content