Send input message acks for swapped out renderers.

content/renderer/render_view_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/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <memory>
 
@@ skipping 1282 matching lines @@
 
 // IPC::Listener implementation ----------------------------------------------
 
 bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
   WebFrame* main_frame = webview() ? webview()->mainFrame() : NULL;
   if (main_frame && main_frame->isWebLocalFrame())
     GetContentClient()->SetActiveURL(main_frame->document().url());
 
   // Input IPC messages must not be processed if the RenderView is in
   // swapped out state.
-  if (is_swapped_out_ && IPC_MESSAGE_ID_CLASS(message.type()) == InputMsgStart)
+  bool discard_input =
+      is_swapped_out_ && IPC_MESSAGE_ID_CLASS(message.type()) == InputMsgStart;
+
+  // TODO(dtapuska): Remove this histogram once we have seen that it actually
+  // produces results true. See crbug.com/615090
+  UMA_HISTOGRAM_BOOLEAN("Event.RenderView.DiscardInput", discard_input);

[Charlie Reis, 2016/10/27 16:57:00]

I would recommend only sending this if discard_input is true.  As Ken mentioned,
this gets hit for all input events for the main frame, which would be a lot of
unnecessary UMA activity.

(If you do need to keep it as a bool even in that case, I've heard your supposed
to add a more specific bool enum in histograms.xml for each case.)

[dtapuska, 2016/10/27 18:55:36]

I fear shipping the dcheck/notreached because I think this truly does exist as a
scenario whereby a swapped out renderer occurs but it is still getting input
events before it responds with the swap ack where events are then not routed.

Especially since the InputMsg channel is different than the ViewMsg channel.

Can we go with the UMA metric for now and see if it gets increased at all? If we
get no signal at all then we can dcheck or check?

[Charlie Reis, 2016/10/27 19:28:59]

I already said yes to that in my previous reply.  :)

On 2016/10/27 16:57:00, Charlie Reis wrote:
FWIW, DCHECKs and NOTREACHEDs are debug only and won't crash release builds, but
I agree with your reasoning.

My only remaining comment was about moving the histogram inside the
discard_input conditional.

+  if (discard_input) {
+    IPC_BEGIN_MESSAGE_MAP(RenderViewImpl, message)
+      IPC_MESSAGE_HANDLER(InputMsg_HandleInputEvent, OnDiscardInputEvent)
+    IPC_END_MESSAGE_MAP()
     return false;
+  }
 
   for (auto& observer : observers_) {
     if (observer.OnMessageReceived(message))
       return true;
   }
 
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderViewImpl, message)
     IPC_MESSAGE_HANDLER(InputMsg_ExecuteEditCommand, OnExecuteEditCommand)
     IPC_MESSAGE_HANDLER(InputMsg_MoveCaret, OnMoveCaret)
@@ skipping 1715 matching lines @@
     return;
   if (IsUseZoomForDSFEnabled()) {
     webview()->setZoomFactorForDeviceScaleFactor(device_scale_factor_);
   } else {
     webview()->setDeviceScaleFactor(device_scale_factor_);
   }
   webview()->settings()->setPreferCompositingToLCDTextEnabled(
       PreferCompositingToLCDText(compositor_deps_, device_scale_factor_));
 }
 
+void RenderViewImpl::OnDiscardInputEvent(
+    const blink::WebInputEvent* input_event,
+    const ui::LatencyInfo& latency_info,
+    InputEventDispatchType dispatch_type) {
+  if (!input_event || (dispatch_type != DISPATCH_TYPE_BLOCKING &&
+                       dispatch_type != DISPATCH_TYPE_BLOCKING_NOTIFY_MAIN)) {
+    return;
+  }
+
+  if (dispatch_type == DISPATCH_TYPE_BLOCKING_NOTIFY_MAIN) {
+    NotifyInputEventHandled(input_event->type,
+                            INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  }
+
+  std::unique_ptr<InputEventAck> ack(
+      new InputEventAck(input_event->type, INPUT_EVENT_ACK_STATE_NOT_CONSUMED));
+  OnInputEventAck(std::move(ack));
+}
+
 }  // namespace content