Route input-related IPCs through compositor thread filter to preserve order

The compositor thread input filter redirects some IPC messages in the render
process to the compositor thread for handling in order to accelerate scrolling,
pinch zooming, and related gestures. IPC messages that the compositor thread
cannot handle are redirected to the main thread for normal processing. This
thread hop can result in the IPC messages being received on the render process
main thread in a different order than they were sent from the browser. This can
break things for input-related messages, for instance changing the order of a
MouseCaptureLost and MouseUp event can result in the mouse up not being
delivered to the capturing element.

This forces a compositor thread hop to preserve order for a set of IPC types
that appear (mostly by inspection and guessing) to need their order preserved
relative to input events.

BUG=233365,231532

Review URL: https://chromiumcodereview.appspot.com/14328027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195329 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/gpu/input_event_filter.cc

@@ -74,8 +74,35 @@ void InputEventFilter::OnChannelClosing() {
   sender_ = NULL;
 }
 
+// This function returns true if the IPC message is one that the compositor
+// thread can handle *or* one that needs to preserve relative order with
+// messages that the compositor thread can handle. Returning true for a message
+// type means that the message will go through an extra copy and thread hop, so
+// use with care.
+static bool RequiresThreadBounce(const IPC::Message& message) {
+  return message.type() == ViewMsg_HandleInputEvent::ID ||
+         message.type() == ViewMsg_MouseCaptureLost::ID ||
+         message.type() == ViewMsg_SetFocus::ID ||
+         message.type() == ViewMsg_SetInputMethodActive::ID ||
+         message.type() == ViewMsg_Undo::ID ||
+         message.type() == ViewMsg_Redo::ID ||
+         message.type() == ViewMsg_Cut::ID ||
+         message.type() == ViewMsg_Copy::ID ||
+         message.type() == ViewMsg_Paste::ID ||
+         message.type() == ViewMsg_PasteAndMatchStyle::ID ||
+         message.type() == ViewMsg_Delete::ID ||
+         message.type() == ViewMsg_Replace::ID ||
+         message.type() == ViewMsg_ReplaceMisspelling::ID ||
+         message.type() == ViewMsg_Delete::ID ||
+         message.type() == ViewMsg_SelectAll::ID ||
+         message.type() == ViewMsg_Unselect::ID ||
+         message.type() == ViewMsg_SelectRange::ID ||
+         message.type() == ViewMsg_MoveCaret::ID ||
+         message.type() == ViewMsg_SmoothScrollCompleted::ID;
+}
+
 bool InputEventFilter::OnMessageReceived(const IPC::Message& message) {
-  if (message.type() != ViewMsg_HandleInputEvent::ID)
+  if (!RequiresThreadBounce(message))
     return false;
 
   {
@@ -111,6 +138,14 @@ void InputEventFilter::ForwardToMainListener(const IPC::Message& message) {
 void InputEventFilter::ForwardToHandler(const IPC::Message& message) {
   DCHECK(target_loop_->BelongsToCurrentThread());
 
+  if (message.type() != ViewMsg_HandleInputEvent::ID) {
+    main_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&InputEventFilter::ForwardToMainListener,
+                   this, message));
+    return;
+  }
+
   // Save this message for later, in case we need to bounce it back up to the
   // main listener.
   //

content/renderer/gpu/input_event_filter_unittest.cc

@@ -103,59 +103,78 @@ void InitMouseEvent(WebMouseEvent* event, WebInputEvent::Type type,
   event->y = y;
 }
 
+void AddMessagesToFilter(IPC::ChannelProxy::MessageFilter* message_filter,
+                         const std::vector<IPC::Message>& events) {
+  for (size_t i = 0; i < events.size(); ++i) {
+    message_filter->OnMessageReceived(events[i]);
+  }
+
+  MessageLoop::current()->RunUntilIdle();
+}
+
 void AddEventsToFilter(IPC::ChannelProxy::MessageFilter* message_filter,
                        const WebMouseEvent events[],
                        size_t count) {
+  std::vector<IPC::Message> messages;
   for (size_t i = 0; i < count; ++i) {
-    ViewMsg_HandleInputEvent message(kTestRoutingID, &events[i], false);
-    message_filter->OnMessageReceived(message);
+    messages.push_back(
+        ViewMsg_HandleInputEvent(kTestRoutingID, &events[i], false));
   }
 
-  MessageLoop::current()->RunUntilIdle();
+  AddMessagesToFilter(message_filter, messages);
 }
 
 }  // namespace
 
-TEST(InputEventFilterTest, Basic) {
-  MessageLoop message_loop;
+class InputEventFilterTest : public testing::Test {
+ public:
+  virtual void SetUp() OVERRIDE {
+    filter_ = new InputEventFilter(
+        &message_recorder_,
+        message_loop_.message_loop_proxy(),
+        base::Bind(&InputEventRecorder::HandleInputEvent,
+            base::Unretained(&event_recorder_)));
+    event_recorder_.set_filter(filter_);
+
+    filter_->OnFilterAdded(&ipc_sink_);
+  }
+
+
+ protected:
+  MessageLoop message_loop_;
 
   // Used to record IPCs sent by the filter to the RenderWidgetHost.
-  IPC::TestSink ipc_sink;
+  IPC::TestSink ipc_sink_;
 
   // Used to record IPCs forwarded by the filter to the main thread.
-  IPCMessageRecorder message_recorder;
+  IPCMessageRecorder message_recorder_;
 
   // Used to record WebInputEvents delivered to the handler.
-  InputEventRecorder event_recorder;
+  InputEventRecorder event_recorder_;
 
-  scoped_refptr<InputEventFilter> filter =
-      new InputEventFilter(&message_recorder,
-                           message_loop.message_loop_proxy(),
-                           base::Bind(&InputEventRecorder::HandleInputEvent,
-                                      base::Unretained(&event_recorder)));
-  event_recorder.set_filter(filter);
-
-  filter->OnFilterAdded(&ipc_sink);
+  scoped_refptr<InputEventFilter> filter_;
+};
 
+TEST_F(InputEventFilterTest, Basic) {
   WebMouseEvent kEvents[3];
   InitMouseEvent(&kEvents[0], WebInputEvent::MouseDown, 10, 10);
   InitMouseEvent(&kEvents[1], WebInputEvent::MouseMove, 20, 20);
   InitMouseEvent(&kEvents[2], WebInputEvent::MouseUp, 30, 30);
 
-  AddEventsToFilter(filter, kEvents, arraysize(kEvents));
-  EXPECT_EQ(0U, ipc_sink.message_count());
-  EXPECT_EQ(0U, event_recorder.record_count());
-  EXPECT_EQ(0U, message_recorder.message_count());
+  AddEventsToFilter(filter_, kEvents, arraysize(kEvents));
+  EXPECT_EQ(0U, ipc_sink_.message_count());
+  EXPECT_EQ(0U, event_recorder_.record_count());
+  EXPECT_EQ(0U, message_recorder_.message_count());
 
-  filter->AddRoute(kTestRoutingID);
+  filter_->AddRoute(kTestRoutingID);
 
-  AddEventsToFilter(filter, kEvents, arraysize(kEvents));
-  ASSERT_EQ(arraysize(kEvents), ipc_sink.message_count());
-  ASSERT_EQ(arraysize(kEvents), event_recorder.record_count());
-  EXPECT_EQ(0U, message_recorder.message_count());
+  AddEventsToFilter(filter_, kEvents, arraysize(kEvents));
+  ASSERT_EQ(arraysize(kEvents), ipc_sink_.message_count());
+  ASSERT_EQ(arraysize(kEvents), event_recorder_.record_count());
+  EXPECT_EQ(0U, message_recorder_.message_count());
 
   for (size_t i = 0; i < arraysize(kEvents); ++i) {
-    const IPC::Message* message = ipc_sink.GetMessageAt(i);
+    const IPC::Message* message = ipc_sink_.GetMessageAt(i);
     EXPECT_EQ(kTestRoutingID, message->routing_id());
     EXPECT_EQ(ViewHostMsg_HandleInputEvent_ACK::ID, message->type());
 
@@ -166,22 +185,22 @@ TEST(InputEventFilterTest, Basic) {
     EXPECT_EQ(kEvents[i].type, event_type);
     EXPECT_EQ(ack_result, INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
 
-    const WebInputEvent* event = event_recorder.record_at(i);
+    const WebInputEvent* event = event_recorder_.record_at(i);
     ASSERT_TRUE(event);
 
     EXPECT_EQ(kEvents[i].size, event->size);
     EXPECT_TRUE(memcmp(&kEvents[i], event, event->size) == 0);
   }
 
-  event_recorder.set_send_to_widget(true);
+  event_recorder_.set_send_to_widget(true);
 
-  AddEventsToFilter(filter, kEvents, arraysize(kEvents));
-  EXPECT_EQ(arraysize(kEvents), ipc_sink.message_count());
-  EXPECT_EQ(2 * arraysize(kEvents), event_recorder.record_count());
-  EXPECT_EQ(arraysize(kEvents), message_recorder.message_count());
+  AddEventsToFilter(filter_, kEvents, arraysize(kEvents));
+  EXPECT_EQ(arraysize(kEvents), ipc_sink_.message_count());
+  EXPECT_EQ(2 * arraysize(kEvents), event_recorder_.record_count());
+  EXPECT_EQ(arraysize(kEvents), message_recorder_.message_count());
 
   for (size_t i = 0; i < arraysize(kEvents); ++i) {
-    const IPC::Message& message = message_recorder.message_at(i);
+    const IPC::Message& message = message_recorder_.message_at(i);
 
     ASSERT_EQ(ViewMsg_HandleInputEvent::ID, message.type());
     const WebInputEvent* event = InputEventFilter::CrackMessage(message);
@@ -192,19 +211,19 @@ TEST(InputEventFilterTest, Basic) {
 
   // Now reset everything, and test that DidHandleInputEvent is called.
 
-  ipc_sink.ClearMessages();
-  event_recorder.Clear();
-  message_recorder.Clear();
+  ipc_sink_.ClearMessages();
+  event_recorder_.Clear();
+  message_recorder_.Clear();
 
-  event_recorder.set_handle_events(true);
+  event_recorder_.set_handle_events(true);
 
-  AddEventsToFilter(filter, kEvents, arraysize(kEvents));
-  EXPECT_EQ(arraysize(kEvents), ipc_sink.message_count());
-  EXPECT_EQ(arraysize(kEvents), event_recorder.record_count());
-  EXPECT_EQ(0U, message_recorder.message_count());
+  AddEventsToFilter(filter_, kEvents, arraysize(kEvents));
+  EXPECT_EQ(arraysize(kEvents), ipc_sink_.message_count());
+  EXPECT_EQ(arraysize(kEvents), event_recorder_.record_count());
+  EXPECT_EQ(0U, message_recorder_.message_count());
 
   for (size_t i = 0; i < arraysize(kEvents); ++i) {
-    const IPC::Message* message = ipc_sink.GetMessageAt(i);
+    const IPC::Message* message = ipc_sink_.GetMessageAt(i);
     EXPECT_EQ(kTestRoutingID, message->routing_id());
     EXPECT_EQ(ViewHostMsg_HandleInputEvent_ACK::ID, message->type());
 
@@ -216,7 +235,57 @@ TEST(InputEventFilterTest, Basic) {
     EXPECT_EQ(ack_result, INPUT_EVENT_ACK_STATE_CONSUMED);
   }
 
-  filter->OnFilterRemoved();
+  filter_->OnFilterRemoved();
+}
+
+TEST_F(InputEventFilterTest, PreserveRelativeOrder) {
+  filter_->AddRoute(kTestRoutingID);
+  event_recorder_.set_send_to_widget(true);
+
+
+  WebMouseEvent mouse_down;
+  mouse_down.type = WebMouseEvent::MouseDown;
+  WebMouseEvent mouse_up;
+  mouse_up.type = WebMouseEvent::MouseUp;
+
+  std::vector<IPC::Message> messages;
+  messages.push_back(ViewMsg_HandleInputEvent(kTestRoutingID,
+                                              &mouse_down,
+                                              false));
+  // Control where input events are delivered.
+  messages.push_back(ViewMsg_MouseCaptureLost(kTestRoutingID));
+  messages.push_back(ViewMsg_SetFocus(kTestRoutingID, true));
+  messages.push_back(ViewMsg_SetInputMethodActive(kTestRoutingID, true));
+
+  // Editing operations
+  messages.push_back(ViewMsg_Undo(kTestRoutingID));
+  messages.push_back(ViewMsg_Redo(kTestRoutingID));
+  messages.push_back(ViewMsg_Cut(kTestRoutingID));
+  messages.push_back(ViewMsg_Copy(kTestRoutingID));
+  messages.push_back(ViewMsg_Paste(kTestRoutingID));
+  messages.push_back(ViewMsg_PasteAndMatchStyle(kTestRoutingID));
+  messages.push_back(ViewMsg_Delete(kTestRoutingID));
+  messages.push_back(ViewMsg_Replace(kTestRoutingID, string16()));
+  messages.push_back(ViewMsg_ReplaceMisspelling(kTestRoutingID, string16()));
+  messages.push_back(ViewMsg_Delete(kTestRoutingID));
+  messages.push_back(ViewMsg_SelectAll(kTestRoutingID));
+  messages.push_back(ViewMsg_Unselect(kTestRoutingID));
+  messages.push_back(ViewMsg_SelectRange(kTestRoutingID,
+                                         gfx::Point(), gfx::Point()));
+  messages.push_back(ViewMsg_MoveCaret(kTestRoutingID, gfx::Point()));
+
+  messages.push_back(ViewMsg_SmoothScrollCompleted(kTestRoutingID, 0));
+  messages.push_back(ViewMsg_HandleInputEvent(kTestRoutingID,
+                                              &mouse_up,
+                                              false));
+  AddMessagesToFilter(filter_, messages);
+
+  // We should have sent two messages back to the main thread and preserved
+  // their relative order.
+  ASSERT_EQ(message_recorder_.message_count(), messages.size());
+  for (size_t i = 0; i < messages.size(); ++i) {
+    EXPECT_EQ(message_recorder_.message_at(i).type(), messages[i].type()) << i;
+  }
 }
 
 }  // namespace content