RenderWidget/RenderView: encapsulate ViewHostMsg_Show, etc, in a callback

content/common/swapped_out_messages.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/common/swapped_out_messages.h"
 
 #include "content/common/accessibility_messages.h"
 #include "content/common/frame_messages.h"
 #include "content/common/input_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/common/content_client.h"
 
 namespace content {
 
 bool SwappedOutMessages::CanSendWhileSwappedOut(const IPC::Message* msg) {
   // We filter out most IPC messages when swapped out.  However, some are
   // important (e.g., ACKs) for keeping the browser and renderer state
   // consistent in case we later return to the same renderer.
   switch (msg->type()) {
     // Handled by RenderWidgetHost.
     case InputHostMsg_HandleInputEvent_ACK::ID:
     case ViewHostMsg_UpdateRect::ID:
     // Handled by RenderWidgetHostView.
     case ViewHostMsg_SetNeedsBeginFrames::ID:
     // Allow targeted navigations while swapped out.
     case FrameHostMsg_OpenURL::ID:
     case ViewHostMsg_Focus::ID:
     // Handled by RenderViewHost.
     case ViewHostMsg_ClosePage_ACK::ID:
+    case ViewHostMsg_ShowFullscreenWidget::ID:
+    case ViewHostMsg_ShowView::ID:
+    case ViewHostMsg_ShowWidget::ID:
     case ViewHostMsg_SwapCompositorFrame::ID:
     // Handled by SharedWorkerMessageFilter.
     case ViewHostMsg_DocumentDetached::ID:
     // Allow cross-process JavaScript calls.
     case ViewHostMsg_RouteCloseEvent::ID:
     // Send page scale factor reset notification upon cross-process navigations.
     case ViewHostMsg_PageScaleFactorChanged::ID:
     // Handled by RenderFrameHost.
     case FrameHostMsg_BeforeUnload_ACK::ID:
     case FrameHostMsg_SwapOut_ACK::ID:
@@ skipping 21 matching lines @@
   // be handled by the browser.
   if (CanSendWhileSwappedOut(&msg))
     return true;
 
   // We drop most other messages that arrive from a swapped out renderer.
   // However, some are important (e.g., ACKs) for keeping the browser and
   // renderer state consistent in case we later return to the renderer.
   // Note that synchronous messages that are not handled will receive an
   // error reply instead, to avoid leaving the renderer in a stuck state.
   switch (msg.type()) {
-    // Sends an ACK.
-    case ViewHostMsg_ShowView::ID:
-    // Sends an ACK.
-    case ViewHostMsg_ShowWidget::ID:
-    // Sends an ACK.
-    case ViewHostMsg_ShowFullscreenWidget::ID:

[dcheng, 2016/11/21 11:19:52]

Out of curiosity, why did we move these?

I guess I would have expected the set of can send and can handle messages to
always be in sync, and I'm a bit surprised that that's not true.

[ncarter (slow), 2016/11/21 19:51:24]

I didn't change when these messages were sent, but I did change the IPC::Sender
via which they were sent -- they used to be sent via the RenderWidget targeting
a cached value of the view's route_id. When I moved them to the RenderView, it
broke window.open from oopifs and this was the fix.

I agree with you that CanHandle but not CanSend is a weird convention. Here is
what I previously assumed: that it was plausible in the ACK-required case, where
the RFH swaps out but still wants to ACK messages sent by the RF's RenderView
that were sent prior to the RF before the RF knew it was swapped out.

However, looking at the code, the CanHandleWhileSwappedOut restriction seems to
start being applied only after after the SwapOut_ACK arrives in the RFH, so this
isn't plausible. In other words, I think you're right.

So we probably ought to combine these two cases. I also strongly suspect that
the FrameHostMsg's are totally unnecessary in this list here, since we don't
seem to actually apply this filter for the RFH's OnMessageReceived, nor does the
RenderFrame's Send doesn't go through the RenderView. I'll try to clean it all
up in a subsequent patch.

     // Updates the previous navigation entry.
     case ViewHostMsg_UpdateState::ID:
     // Sends an ACK.
     case ViewHostMsg_UpdateTargetURL::ID:
     // We allow closing even if we are in the process of swapping out.
     case ViewHostMsg_Close::ID:
     // Sends an ACK.
     case ViewHostMsg_RequestMove::ID:
     // Sends an ACK.
     case AccessibilityHostMsg_Events::ID:
       return true;
     default:
       break;
   }
 
   return false;
 }
 
 }  // namespace content