Fix a bug where an infinite stream of BeginFrame messages are sent to extensions.

content/browser/renderer_host/render_widget_host_view_mac.mm

 // 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/browser/renderer_host/render_widget_host_view_mac.h"
 
 #import <Carbon/Carbon.h>
 #import <objc/runtime.h>
 #include <OpenGL/gl.h>
 #include <QuartzCore/QuartzCore.h>
@@ skipping 466 matching lines @@
 
   // Let the page-level input event router know about our surface ID
   // namespace for surface-based hit testing.
   if (render_widget_host_->delegate() &&
       render_widget_host_->delegate()->GetInputEventRouter()) {
     render_widget_host_->delegate()->GetInputEventRouter()->AddFrameSinkIdOwner(
         GetFrameSinkId(), this);
   }
 
   RenderViewHost* rvh = RenderViewHost::From(render_widget_host_);
+  bool needs_begin_frames = true;
   if (rvh) {

[danakj, 2016/12/15 15:56:59]

So, when is this null? I got as far RenderViewHostImpl seems to always call
set_owner_delegate() which is how we get this |rvh| here..
(https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_vie...).
When do we have a RWHVMac but there's not the owner delegate?

[erikchen, 2016/12/15 16:51:11]

I agree that I don't see how rvh could be nullptr. At the same time, note
"""ignore_result(rvh->GetWebkitPreferences());""" seems pretty suspicious too.
That line should perhaps be removed as well. I've added a comment to the
original CL:
https://codereview.chromium.org/2163953002/#msg25

If you think I should remove the conditional, I can do that, but I would want to
do it in a follow up CL, since I intend to merge this to M-56 and I want this CL
to be simple as possible.

     // TODO(mostynb): actually use prefs.  Landing this as a separate CL
     // first to rebaseline some unreliable layout tests.
     ignore_result(rvh->GetWebkitPreferences());
+    needs_begin_frames = !rvh->GetDelegate()->IsNeverVisible();
   }
 
   if (GetTextInputManager())
     GetTextInputManager()->AddObserver(this);
 
   // Because of the way Mac pumps messages during resize, (see the code
   // in RenderMessageFilter::OnMessageReceived), SetNeedsBeginFrame
   // messages are not delayed on Mac.  This leads to creation-time
   // raciness where renderer sends a SetNeedsBeginFrame(true) before
-  // the renderer host is created to recieve it.  In general, all
-  // renderers want begin frames initially anyway, so start this value
-  // at true here to avoid startup raciness and decrease latency.
-  needs_begin_frames_ = true;
+  // the renderer host is created to recieve it.
+  //
+  // A renderer wants begin frames initially if and only if

[danakj, 2016/12/15 15:56:59]

IsNeverVisible should not change, thats why its name includes "Never". I would
reword this a bit. Something like "Any renderer that will produce frames needs
to have begin frames sent to it. So unless it is never visible, start this value
at true here to avoid startup raciness and decrease latency."?

[erikchen, 2016/12/15 16:51:11]

Done.

+  // |rvh->GetDelegate()->IsNeverVisible()| evalutes to false. If this statement
+  // becomes false, then we will need to update this logic to ensure that
+  // |needs_begin_frames_| stays synchronized between the browser and the
+  // renderer. The benefit of starting it as true is that it reduces startup
+  // raciness and decreases latency.
+  needs_begin_frames_ = needs_begin_frames;
   UpdateNeedsBeginFramesInternal();
 }
 
 RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
   display::Screen::GetScreen()->RemoveObserver(this);
 
   // This is being called from |cocoa_view_|'s destructor, so invalidate the
   // pointer.
   cocoa_view_ = nil;
 
@@ skipping 2929 matching lines @@
 
 // "-webkit-app-region: drag | no-drag" is implemented on Mac by excluding
 // regions that are not draggable. (See ControlRegionView in
 // native_app_window_cocoa.mm). This requires the render host view to be
 // draggable by default.
 - (BOOL)mouseDownCanMoveWindow {
   return YES;
 }
 
 @end