Make rendering timeouts work even if paint data received early

content/browser/renderer_host/render_widget_host_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_IMPL_H_
 
 #include <list>
 #include <map>
 #include <string>
@@ skipping 249 matching lines @@
   void StopHangMonitorTimeout();
 
   // Starts the rendering timeout, which will clear displayed graphics if
   // a new compositor frame is not received before it expires.
   void StartNewContentRenderingTimeout();
 
   // Stops the rendering timeout and prevents it from clearing any displayed
   // graphics.
   void StopNewContentRenderingTimeout();
 
+  // Notification that a new compositor frame has been generated following
+  // a page load. This stops |new_content_rendering_timeout_|.

[Charlie Reis, 2015/09/30 20:36:19]

nit: "...or prevents the timer from running if it hasn't started yet."

[kenrb, 2015/09/30 21:25:02]

Done.

+  void OnFirstPaintAfterLoad();

[Charlie Reis, 2015/09/30 20:36:19]

Are we guaranteed to always get both this and the commit IPC before seeing any
other instances?

For example, I don't want to accidentally allow this:
1) OnFirstPaintAfterLoad for page 1.
(No commit of page 1 for some reason)
2) OnDidCommitProvisionalLoad for page 2.
(No timer starts because of #1, but page 2 can now be spoofed.)

[kenrb, 2015/09/30 21:25:02]

It shouldn't be possible to get a ViewHostMsg_DidFirstPaintAfterLoad without a
FrameHostMsg_DidCommitProvisionalLoad. Both are triggered in the renderer by
FrameLoaderClientImpl::dispatchDidCommitLoad. The only bailout I see is that
FrameHostMsg_DidCommitProvisionalLoad won't be sent if the WebFrame has no
client (i.e. no RenderFrameImpl). In this case I imagine the frame has been
destroyed so we don't have to worry about any spoofing problems, but I would be
interested if Daniel can think of a way that could cause a problem.

It's tricky to reason through this, but does that guarantee we won't run into
problems? I would assume that if the load commit for page 1 is sent, that it
isn't possible for a load commit for page 2 to arrive before it (this would seem
really bad).

+
   // Forwards the given message to the renderer. These are called by the view
   // when it has received a message.
   void ForwardGestureEventWithLatencyInfo(
       const blink::WebGestureEvent& gesture_event,
       const ui::LatencyInfo& ui_latency);
   void ForwardTouchEventWithLatencyInfo(
       const blink::WebTouchEvent& touch_event,
       const ui::LatencyInfo& ui_latency);
   void ForwardMouseEventWithLatencyInfo(
       const blink::WebMouseEvent& mouse_event,
@@ skipping 523 matching lines @@
   scoped_ptr<SyntheticGestureController> synthetic_gesture_controller_;
 
   scoped_ptr<TouchEmulator> touch_emulator_;
 
   // Receives and handles all input events.
   scoped_ptr<InputRouter> input_router_;
 
   scoped_ptr<TimeoutMonitor> hang_monitor_timeout_;
 
   scoped_ptr<TimeoutMonitor> new_content_rendering_timeout_;
+  bool received_paint_after_load_;

[Charlie Reis, 2015/09/30 20:36:19]

Let's say when we expect this to be true, since it's not clear.  I think it's
exactly the time interval between the first paint and the navigation commit, if
they happen in that order.  It's never true if the commit happens first, and
(hopefully?) we're guaranteed to always receive those IPCs in pairs without
other messages from other navigations interleaved.

[kenrb, 2015/09/30 21:25:02]

Done.

 
 #if defined(OS_WIN)
   std::list<HWND> dummy_windows_for_activation_;
 #endif
 
   RenderWidgetHostLatencyTracker latency_tracker_;
 
   int next_browser_snapshot_id_;
   using PendingSnapshotMap = std::map<int, GetSnapshotFromBrowserCallback>;
   PendingSnapshotMap pending_browser_snapshots_;
 
   // Indicates whether a RenderFramehost has ownership, in which case this
   // object does not self destroy.
   bool owned_by_render_frame_host_;
 
   // Indicates whether this RenderWidgetHost thinks is focused. This is trying
   // to match what the renderer process knows. It is different from
   // RenderWidgetHostView::HasFocus in that in that the focus request may fail,
   // causing HasFocus to return false when is_focused_ is true.
   bool is_focused_;
 
   base::WeakPtrFactory<RenderWidgetHostImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_IMPL_H_