Deliver IPC messages together with SwapCompositorFrame

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 5b4a19dd07b6edded6032d615ca38bdcd42fd2ab..c2998ae3c5d3c981933cbbe9ea2ce9810b8c89f5 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -1343,6 +1343,38 @@ void RenderWidget::DidCommitCompositorFrame() {
                     DidCommitCompositorFrame());
 }
 
+namespace {
+
+class RWSwapPromise: public cc::SwapPromise {
+  public:
+   RWSwapPromise(IPC::Sender* sender, int routing_id, IPC::Message msg):
+       fallback_sender_(sender),
+       fallback_routing_id_(routing_id),
+       msg_(msg) {}
+   virtual void DidSwap(cc::CompositorFrameMetadata* metadata) OVERRIDE {
+     metadata->payload.push_back(msg);
+   }
+   virtual void DidNotSwap(DidNotSwapReason reason) OVERRIDE {
+     fallback_sender_->Send(
+         new ViewHostMsg_DeliverPayload(fallback_routing_id_, msg));

[piman, 2014/05/07 01:21:46]

How about fallback_sender_->Send(msg)? Is there a reason why we would want this
message to be treated specially?

[piman, 2014/05/07 01:21:46]

Also, beware, DidNotSwap can be called on either thread. It's not ok to call
RenderWidget::Send from the compositor thread.

You can use RenderThreadImpl::current()->sync_message_filter() to send from the
compositor thread, and RenderThreadImpl::current() to send from the main thread.

[mkosiba (inactive), 2014/05/07 18:27:20]

thanks for pointing that out!
yes. The way I envisioned this was that you'd only ever call QueueMessage with
control IPCs (that is I don't want whoever's calling QueueMessage to be able to
specify a routing id). I was planning on using IPC for their serialization
ability and the dispatch mechanism, not for the routing.
Ideally I'd use a Pickle that can be dispatched, and I'm not sure which is worse
- fiddling with the ipc_messages_macros file or the current solution :P

The other reason for wrapping msg in ViewHostMsg_DeliverPayload is to make it
easier to deliver the message to the designated WebContentsObserver callback.
Otherwise it would be confusing, I think:
1) you wouldn't be able to distinguish whether the message can be delivered in
this special way,
2) It would be cumbersome to plumb through a way for the receiver to know
whether the message was sent from DidNotSwap or DidSwap.
3) Re-injecting the messages from RenderWidgetHost into the RenderProcessHost's
message dispatcher feels hacky.

+   }
+  private:
+   IPC::Sender* fallback_sender_;
+   int fallback_routing_id_;
+   IPC::Message msg_;
+};
+
+} //  namespace
+
+void RenderWidget::QueueMessage(const IPC::Message& msg) {

[mkosiba (inactive), 2014/05/06 22:30:33]

so this would be the entry point

+  if (!compositor_->commitRequested()) {

[piman, 2014/05/07 01:21:46]

I'm not sure if that isn't racy.

I think the contract of the SwapPromise is that it will call DidNotSwap if it
doesn't commit?

[mkosiba (inactive), 2014/05/07 18:27:20]

oh.. I assumed that this tracks whether the Blink thread requested a commit. Is
that not the case?

+    Send(ViewHostMsg_DeliverPayload(routing_id_, msg);
+  } else {
+    scoped_ptr<cc::SwapPromise> promise(new RWSwapPromise());
+    compositor_->QueueSwapPromise(promise.Pass());
+  }
+}
+
 void RenderWidget::didCommitAndDrawCompositorFrame() {
   TRACE_EVENT0("gpu", "RenderWidget::didCommitAndDrawCompositorFrame");
   // Accelerated FPS tick for performance tests. See