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 556 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) {
+  // 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 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;
+  }
+
+  // The renderer did not send messages for smaller frame tokens. We kill it
+  // because it's misbehaving.
+  if (queued_tokens_.front() < frame_token) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RWH_INVALID_FRAME_TOKEN);
+    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.

[piman, 2017/04/03 23:48:26]

nit: shouldn't this be consistent with the queued_tokens_.front() < frame_token
case above, and kill the renderer too?

If we're assuming that proper renderer behavior is to sends a token with the
frame if and only if there are messages for it, and that both frames and
messages are FIFO in their respective queue, then it's either one of 2 cases:
- queued_tokens_ is empty, because we haven't received the corresponding frame
yet (messages arrived first)
- or queued_tokens_.front() is frame_token (frame arrived first).

Anything else would be a violation of the protocol, right?

[Saman Sami, 2017/04/03 23:56:01]

I'm making the assumption that frames can be dropped but their messages will
always arrive. Frames won't be dropped right now, but in the future if frame
tokens come from the gpu process then it's possible and I'm trying to make my
implementation as future-proof as possible.

[Saman Sami, 2017/04/03 23:58:09]

Maybe we should actually process the messages if the frame is dropped? Please
let me know what you think.

[piman, 2017/04/04 00:08:11]

Do you have a scenario in mind where frames get dropped? I suspect that yes, it
would make sense to process messages in that case.

[Fady Samuel, 2017/04/04 00:10:10]

If the GPU process crashes, perhaps? Note that this is all subtle so please try
to capture the intent of this code in comments.

[Saman Sami, 2017/04/04 00:13:51]

Yes, GPU can crash before being able to send the token to browser (when frames
actually go to the GPU instead of browser). I'll add more comments.

[Saman Sami, 2017/04/04 00:16:17]

If it makes sense to process the messages of frames that were lost, then I 
guess I can replace queued_tokens_ array with a single integer
last_received_frame_token_ and process every message with token less than or
equal to last_received_frame_token_. That should simplify the code.

[piman, 2017/04/04 00:21:53]

sgtm

+}
+
 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) {
+  // Frame tokens always increase.
+  if (queued_tokens_.size() && frame_token <= queued_tokens_.back()) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RWH_INVALID_FRAME_TOKEN);
+    return;
+  }
+
+  // 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)
+    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.
+  // This should not happen. The renderer is misbehaving.
+  bad_message::ReceivedBadMessage(GetProcess(),
+                                  bad_message::RWH_INVALID_FRAME_TOKEN);
+}
+
+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