Send FrameSwapMessageQueue's messages with a separate IPC

content/browser/renderer_host/render_widget_host_impl.cc

Index: content/browser/renderer_host/render_widget_host_impl.cc
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 5caf7eab7947f091d5262799123294bcac397bac..0b4a728a109bd0e578b825e07d5b55ab011f81c3 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -574,6 +574,8 @@ bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(ViewHostMsg_FocusedNodeTouched, OnFocusedNodeTouched)
     IPC_MESSAGE_HANDLER(DragHostMsg_StartDragging, OnStartDragging)
     IPC_MESSAGE_HANDLER(DragHostMsg_UpdateDragCursor, OnUpdateDragCursor)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_FrameSwapMessages,
+                        OnFrameSwapMessagesReceived)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -1576,6 +1578,31 @@ void RenderWidgetHostImpl::OnUpdateDragCursor(WebDragOperation current_op) {
     view->UpdateDragCursor(current_op);
 }
 
+void RenderWidgetHostImpl::OnFrameSwapMessagesReceived(
+    uint32_t frame_token,
+    std::vector<IPC::Message> messages) {
+  // Zero token is invalid.
+  if (!frame_token) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RWH_INVALID_FRAME_TOKEN);
+    return;
+  }
+
+  // Frame tokens always increase.
+  if (queued_messages_.size() && frame_token <= queued_messages_.back().first) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RWH_INVALID_FRAME_TOKEN);
+    return;
+  }
+
+  if (frame_token <= last_received_frame_token_) {
+    ProcessSwapMessages(std::move(messages));
+    return;
+  }
+
+  queued_messages_.push(std::make_pair(frame_token, std::move(messages)));
+}
+
 void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
                                           int exit_code) {
   if (!renderer_initialized_)
@@ -1628,6 +1655,12 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
       process_, this, this, routing_id_, GetInputRouterConfigForPlatform()));
 
   synthetic_gesture_controller_.reset();
+
+  last_received_frame_token_ = 0;
+  while (queued_messages_.size()) {
+    ProcessSwapMessages(std::move(queued_messages_.front().second));

[Saman Sami, 2017/04/04 19:43:12]

OK, maybe this is not a good idea. Maybe it's better not to process messages
coming from a renderer that has exited, because it's kind of equivalent to
receiving messages from a dead process. This might leave the recipient of these
messages in a bad state for example. Please correct me if I'm wrong.

[piman, 2017/04/04 19:49:49]

Recipients have to deal with render process dying and never giving them these
messages today, so I think it's ok to not process the message (i.e. today it
would be equivalent to never receiving the frame).

It's also probably ok to process them, if done at the right place (maybe earlier
in this function), but I think you're right that not doing so is safer.

+    queued_messages_.pop();
+  }
 }
 
 void RenderWidgetHostImpl::UpdateTextDirection(WebTextDirection direction) {
@@ -1849,8 +1882,6 @@ bool RenderWidgetHostImpl::OnSwapCompositorFrame(
   uint32_t compositor_frame_sink_id = std::get<0>(param);
   cc::LocalSurfaceId local_surface_id = std::get<1>(param);
   cc::CompositorFrame frame(std::move(std::get<2>(param)));
-  std::vector<IPC::Message> messages_to_deliver_with_frame;
-  messages_to_deliver_with_frame.swap(std::get<3>(param));
 
   if (compositor_frame_sink_id != last_compositor_frame_sink_id_) {
     if (view_)
@@ -1860,17 +1891,6 @@ bool RenderWidgetHostImpl::OnSwapCompositorFrame(
 
   SubmitCompositorFrame(local_surface_id, std::move(frame));
 
-  RenderProcessHost* rph = GetProcess();
-  for (std::vector<IPC::Message>::const_iterator i =
-           messages_to_deliver_with_frame.begin();
-       i != messages_to_deliver_with_frame.end();
-       ++i) {
-    rph->OnMessageReceived(*i);
-    if (i->dispatch_error())
-      rph->OnBadMessageReceived(*i);
-  }
-  messages_to_deliver_with_frame.clear();
-
   return true;
 }
 
@@ -2609,6 +2629,8 @@ void RenderWidgetHostImpl::SubmitCompositorFrame(
     return;
   }
 
+  uint32_t frame_token = frame.metadata.frame_token;
+
   last_local_surface_id_ = local_surface_id;
   last_frame_size_ = frame_size;
   last_device_scale_factor_ = device_scale_factor;
@@ -2659,6 +2681,37 @@ void RenderWidgetHostImpl::SubmitCompositorFrame(
       new_content_rendering_timeout_->IsRunning()) {
     new_content_rendering_timeout_->Stop();
   }
+
+  if (frame_token)
+    DidProcessFrame(frame_token);
+}
+
+void RenderWidgetHostImpl::DidProcessFrame(uint32_t frame_token) {
+  // Frame tokens always increase.
+  if (frame_token <= last_received_frame_token_) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RWH_INVALID_FRAME_TOKEN);
+    return;
+  }
+
+  last_received_frame_token_ = frame_token;
+
+  while (queued_messages_.size() &&
+         queued_messages_.front().first <= frame_token) {
+    ProcessSwapMessages(std::move(queued_messages_.front().second));
+    queued_messages_.pop();
+  }
+}
+
+void RenderWidgetHostImpl::ProcessSwapMessages(
+    std::vector<IPC::Message> messages) {
+  RenderProcessHost* rph = GetProcess();
+  for (std::vector<IPC::Message>::const_iterator i = messages.begin();
+       i != messages.end(); ++i) {
+    rph->OnMessageReceived(*i);
+    if (i->dispatch_error())
+      rph->OnBadMessageReceived(*i);
+  }
 }
 
 }  // namespace content