Deliver IPC messages together with SwapCompositorFrame

content/renderer/render_widget.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/renderer/render_widget.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/debug/trace_event_synthetic_delay.h"
@@ skipping 1325 matching lines @@
 void RenderWidget::didBecomeReadyForAdditionalInput() {
   TRACE_EVENT0("renderer", "RenderWidget::didBecomeReadyForAdditionalInput");
   FlushPendingInputEventAck();
 }
 
 void RenderWidget::DidCommitCompositorFrame() {
   FOR_EACH_OBSERVER(RenderFrameImpl, swapped_out_frames_,
                     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
   // tab_capture_performancetest.cc.  NOTE: Tests may break if this event is
   // renamed or moved.
   UNSHIPPED_TRACE_EVENT_INSTANT0("test_fps", "TestFrameTickGPU",
                                  TRACE_EVENT_SCOPE_THREAD);
   // Notify subclasses that we initiated the paint operation.
   DidInitiatePaint();
 }
@@ skipping 872 matching lines @@
 
 void RenderWidget::RegisterSwappedOutChildFrame(RenderFrameImpl* frame) {
   swapped_out_frames_.AddObserver(frame);
 }
 
 void RenderWidget::UnregisterSwappedOutChildFrame(RenderFrameImpl* frame) {
   swapped_out_frames_.RemoveObserver(frame);
 }
 
 }  // namespace content