Send FrameSwapMessageQueue's messages with a separate IPC

content/browser/renderer_host/render_widget_host_impl.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_widget_host_impl.h"
 
 #include <math.h>
 
 #include <set>
 #include <tuple>
@@ skipping 554 matching lines @@
     IPC_MESSAGE_HANDLER(ViewHostMsg_ShowDisambiguationPopup,
                         OnShowDisambiguationPopup)
     IPC_MESSAGE_HANDLER(ViewHostMsg_SelectionBoundsChanged,
                         OnSelectionBoundsChanged)
     IPC_MESSAGE_HANDLER(InputHostMsg_ImeCompositionRangeChanged,
                         OnImeCompositionRangeChanged)
     IPC_MESSAGE_HANDLER(ViewHostMsg_SetNeedsBeginFrames, OnSetNeedsBeginFrames)
     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()
 
   if (!handled && input_router_ && input_router_->OnMessageReceived(msg))
     return true;
 
   if (!handled && view_ && view_->OnMessageReceived(msg))
     return true;
 
   return handled;
@@ skipping 982 matching lines @@
 
 void RenderWidgetHostImpl::OnUpdateDragCursor(WebDragOperation current_op) {
   if (delegate_ && delegate_->OnUpdateDragCursor())
     return;
 
   RenderViewHostDelegateView* view = delegate_->GetDelegateView();
   if (view)
     view->UpdateDragCursor(current_op);
 }
 
+void RenderWidgetHostImpl::OnFrameSwapMessagesReceived(
+    uint32_t frame_token,
+    std::vector<IPC::Message> messages) {
+  // We will never receive messages for tokens smaller than |frame_token| so
+  // remove them from the queue.
+  while (queued_tokens_.size() && queued_tokens_.front() < frame_token)

[Fady Samuel, 2017/03/31 16:45:14]

Why would the current frame_token not be at the front of the queue?

[Saman Sami, 2017/04/03 19:22:28]

If we assume the renderer->browser connection is reliable (which is true, unlike
the renderer->gpu->browser connection), then you're right, this should not be
necessary.

+    queued_tokens_.pop_front();
+
+  // If there are no tokens queued, queue |messages| so it can be processed
+  // once a matching token arrives.
+  if (queued_tokens_.empty()) {
+    queued_messages_.push_back(
+        std::make_pair(frame_token, std::move(messages)));
+    return;
+  }
+
+  // If the first element of |queued_tokens_| matches |frame_token|,
+  // process |messages| immediately.
+  if (queued_tokens_.front() == frame_token) {
+    queued_tokens_.pop_front();
+    ProcessSwapMessages(std::move(messages));
+    return;
+  }
+
+  // The first element of |queued_tokens_| is larger than |frame_token|.
+  // Discard |messages| because we will never receive |frame_token| in the
+  // future.
+}
+
 void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
                                           int exit_code) {
   if (!renderer_initialized_)
     return;
 
   // Clear this flag so that we can ask the next renderer for composition
   // updates.
   monitoring_composition_info_ = false;
 
   // Clearing this flag causes us to re-create the renderer when recovering
@@ skipping 264 matching lines @@
   messages_to_deliver_with_frame.swap(std::get<3>(param));
 
   if (compositor_frame_sink_id != last_compositor_frame_sink_id_) {
     if (view_)
       view_->DidCreateNewRendererCompositorFrameSink();
     last_compositor_frame_sink_id_ = compositor_frame_sink_id;
   }
 
   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;
 }
 
 void RenderWidgetHostImpl::OnBeginFrameDidNotSwap(
     const cc::BeginFrameAck& ack) {
   if (ack.sequence_number < cc::BeginFrameArgs::kStartingFrameNumber) {
     // Received an invalid ack, renderer misbehaved.
     bad_message::ReceivedBadMessage(
         GetProcess(), bad_message::RWH_INVALID_BEGIN_FRAME_ACK_DID_NOT_SWAP);
     return;
@@ skipping 718 matching lines @@
       (frame_size != last_frame_size_ ||
        device_scale_factor != last_device_scale_factor_)) {
     DLOG(ERROR) << "Renderer submitted frame of wrong size to its surface."
                 << " Expected: size=" << last_frame_size_.ToString()
                 << ",scale=" << last_device_scale_factor_
                 << " Received: size=" << frame_size.ToString()
                 << ",scale=" << device_scale_factor;
     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;
 
   last_received_content_source_id_ = frame.metadata.content_source_id;
 
   if (frame.metadata.begin_frame_ack.sequence_number <
       cc::BeginFrameArgs::kStartingFrameNumber) {
     // Received an invalid ack, renderer misbehaved.
     bad_message::ReceivedBadMessage(
@@ skipping 30 matching lines @@
     cc::TransferableResource::ReturnResources(frame.resource_list, &resources);
     SendReclaimCompositorResources(true /* is_swap_ack */, resources);
   }
 
   // After navigation, if a frame belonging to the new page is received, stop
   // the timer that triggers clearing the graphics of the last page.
   if (last_received_content_source_id_ >= current_content_source_id_ &&
       new_content_rendering_timeout_->IsRunning()) {
     new_content_rendering_timeout_->Stop();
   }
+
+  if (frame_token)
+    DidProcessFrame(frame_token);
+}
+
+void RenderWidgetHostImpl::DidProcessFrame(uint32_t frame_token) {
+  // We won't receive frames with tokens smaller than |frame_token| in the
+  // future. Get rid of all messages queued for such frames.
+  while (queued_messages_.size() &&
+         queued_messages_.front().first < frame_token)

[Fady Samuel, 2017/03/31 16:45:14]

This seems like a bug to get into this state? If not, this needs more comments.

[Saman Sami, 2017/04/03 19:22:28]

If a frame was dropped (for example, on gpu crash), this can happen.

+    queued_messages_.pop_front();
+
+  // If there are no queued messages, add |frame_token| to |queued_tokens_| so
+  // that we can process its messages once they arrive.
+  if (queued_messages_.empty()) {
+    queued_tokens_.push_back(frame_token);
+    return;
+  }
+
+  // If the first element of |queued_messages_| belongs to |frame_token|,
+  // process the queued messages.
+  if (queued_messages_.front().first == frame_token) {
+    ProcessSwapMessages(std::move(queued_messages_.front().second));
+    queued_messages_.pop_front();
+    return;
+  }
+
+  // The first element of |queued_messages_| belongs to a larger frame token.
+  // We discard |frame_token| because its messages will never arrive.

[Fady Samuel, 2017/03/31 16:45:14]

A well-behaved renderer should never get into this state right? Maybe a comment
to say that the renderer is misbehaving here? Should we kill the renderer or
report a bad message or something?

[Saman Sami, 2017/04/03 19:22:28]

If we assume the renderer->browser connection is reliable (which is true, unlike
the renderer->gpu->browser connection), then you're right, this should not
happen.

+}
+
+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