Mac: Shift more code into C++ classes from ObjC classes

It is difficult to reason about the liftime of ObjC classes, especially
NSViews and CALayers. To simplify verifying their correctness, move the
bulk of the code for the ObjC classes into C++ helper classes (these
classes are already necessary to interface with owning structures).

Make the NSView sub-class BrowserCompositorViewCocoa be owned by
BrowserCompositorViewMacInternal.

Move the bulk of the work in CompositingIOSurfaceLayer to be done by
CompositingIOSurfaceLayerHelper, and mark that the ownership
relationship should be inverted (that isn't feasible at the moment
because we are mid-transition from non-delegated rendering to delegated
rendering).

BUG=392919

Review URL: https://codereview.chromium.org/394883007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284063 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/compositor/browser_compositor_view_mac.h

@@ -18,8 +18,6 @@
 #include "ui/events/latency_info.h"
 #include "ui/gfx/geometry/size.h"
 
-@class BrowserCompositorViewCocoa;
-
 // Additions to the NSView interface for compositor frames.
 @interface NSView (BrowserCompositorView)
 - (void)gotAcceleratedIOSurfaceFrame:(IOSurfaceID)surface_handle
@@ -36,6 +34,8 @@
 
 namespace content {
 
+class BrowserCompositorViewMacInternal;
+
 // The interface through which BrowserCompositorViewMac calls back into
 // RenderWidgetHostViewMac (or any other structure that wishes to draw a
 // NSView backed by a ui::Compositor).
@@ -74,7 +74,7 @@ class BrowserCompositorViewMac {
 
  private:
   BrowserCompositorViewMacClient* client_;
-  base::scoped_nsobject<BrowserCompositorViewCocoa> cocoa_view_;
+  scoped_ptr<BrowserCompositorViewMacInternal> internal_view_;
 };
 
 // A class to keep around whenever a BrowserCompositorViewMac may be created.

content/browser/compositor/browser_compositor_view_mac.mm

@@ -40,12 +40,12 @@ namespace {
 
 // The number of placeholder objects allocated. If this reaches zero, then
 // the BrowserCompositorViewCocoa being held on to for recycling,
-// |g_recyclable_cocoa_view|, will be freed.
+// |g_recyclable_internal_view|, will be freed.
 uint32 g_placeholder_count = 0;
 
 // A spare BrowserCompositorViewCocoa kept around for recycling.
-base::LazyInstance<base::scoped_nsobject<BrowserCompositorViewCocoa>>
-  g_recyclable_cocoa_view;
+base::LazyInstance<scoped_ptr<BrowserCompositorViewMacInternal>>
+  g_recyclable_internal_view;
 
 }  // namespace
 
@@ -56,29 +56,26 @@ BrowserCompositorViewMac::BrowserCompositorViewMac(
   // TODO(ccameron): If there exists a frame in flight (swap has been called
   // by the compositor, but the frame has not arrived from the GPU process
   // yet), then that frame may inappropriately flash in the new view.
-  swap(g_recyclable_cocoa_view.Get(), cocoa_view_);
-  if (!cocoa_view_)
-    cocoa_view_.reset([[BrowserCompositorViewCocoa alloc] init]);
-  [cocoa_view_ setClient:client_];
+  internal_view_ = g_recyclable_internal_view.Get().Pass();
+  if (!internal_view_)
+    internal_view_.reset(new BrowserCompositorViewMacInternal);
+  internal_view_->SetClient(client_);
 }
 
 BrowserCompositorViewMac::~BrowserCompositorViewMac() {
   // Make this BrowserCompositorViewCocoa recyclable for future instances.
-  [cocoa_view_ setClient:NULL];
-  [g_recyclable_cocoa_view.Get() destroyCompositor];
-  swap(g_recyclable_cocoa_view.Get(), cocoa_view_);
+  internal_view_->ResetClient();
+  g_recyclable_internal_view.Get() = internal_view_.Pass();
 
   // If there are no placeholders allocated, destroy the recyclable
   // BrowserCompositorViewCocoa that we just populated.
-  if (!g_placeholder_count) {
-    [g_recyclable_cocoa_view.Get() destroyCompositor];
-    g_recyclable_cocoa_view.Get().reset();
-  }
+  if (!g_placeholder_count)
+    g_recyclable_internal_view.Get().reset();
 }
 
 ui::Compositor* BrowserCompositorViewMac::GetCompositor() const {
-  DCHECK(cocoa_view_);
-  return [cocoa_view_ compositor];
+  DCHECK(internal_view_);
+  return internal_view_->compositor();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -94,10 +91,8 @@ BrowserCompositorViewPlaceholderMac::~BrowserCompositorViewPlaceholderMac() {
 
   // If there are no placeholders allocated, destroy the recyclable
   // BrowserCompositorViewCocoa.
-  if (!g_placeholder_count) {
-    [g_recyclable_cocoa_view.Get() destroyCompositor];
-    g_recyclable_cocoa_view.Get().reset();
-  }
+  if (!g_placeholder_count)
+    g_recyclable_internal_view.Get().reset();
 }
 
 }  // namespace content

content/browser/compositor/browser_compositor_view_private_mac.h

@@ -7,16 +7,71 @@
 
 #include "content/browser/compositor/browser_compositor_view_mac.h"
 
+@class BrowserCompositorViewCocoa;
+
 namespace content {
-class BrowserCompositorViewCocoaHelper;
-}
 
-// An NSView drawn by a ui::Compositor. This structure is expensive to create,
-// because it has a ui::Compositor. As a result, this structure may be recycled
-// across multiple BrowserCompositorViewMac objects.
-@interface BrowserCompositorViewCocoa : NSView {
+// BrowserCompositorViewCocoaClient is the interface through which
+// gfx::NativeWidget (aka NSView aka BrowserCompositorViewCocoa) calls back to
+// BrowserCompositorViewMacInternal.
+class BrowserCompositorViewCocoaClient {
+ public:
+  virtual void GotAcceleratedIOSurfaceFrame(
+      IOSurfaceID io_surface_id,
+      int output_surface_id,
+      const std::vector<ui::LatencyInfo>& latency_info,
+      gfx::Size pixel_size,
+      float scale_factor) = 0;
+
+  virtual void GotSoftwareFrame(
+      cc::SoftwareFrameData* frame_data,
+      float scale_factor,
+      SkCanvas* canvas) = 0;
+};
+
+// BrowserCompositorViewMacInternal owns a NSView and a ui::Compositor that
+// draws that view.
+class BrowserCompositorViewMacInternal
+    : public BrowserCompositorViewCocoaClient,
+      public CompositingIOSurfaceLayerClient {
+ public:
+  BrowserCompositorViewMacInternal();
+  virtual ~BrowserCompositorViewMacInternal();
+
+  void SetClient(BrowserCompositorViewMacClient* client);
+  void ResetClient();
+
+  ui::Compositor* compositor() const { return compositor_.get(); }
+
+ private:
+  // BrowserCompositorViewCocoaClient implementation:
+  virtual void GotAcceleratedIOSurfaceFrame(
+      IOSurfaceID io_surface_id,
+      int output_surface_id,
+      const std::vector<ui::LatencyInfo>& latency_info,
+      gfx::Size pixel_size,
+      float scale_factor) OVERRIDE;
+  virtual void GotSoftwareFrame(
+      cc::SoftwareFrameData* frame_data,
+      float scale_factor,
+      SkCanvas* canvas) OVERRIDE;
+
+  // CompositingIOSurfaceLayerClient implementation:
+  virtual void AcceleratedLayerDidDrawFrame(bool succeeded) OVERRIDE;
+
+  // The client of the BrowserCompositorViewMac that is using this as its
+  // internals.
+  BrowserCompositorViewMacClient* client_;
+
+  // The NSView drawn by the |compositor_|
+  base::scoped_nsobject<BrowserCompositorViewCocoa> cocoa_view_;
+
+  // The compositor drawing the contents of |cooca_view_|. Note that this must
+  // be declared after |cocoa_view_|, so that it be destroyed first (because it
+  // will reach into |cocoa_view_|).
   scoped_ptr<ui::Compositor> compositor_;
 
+
   // A flipped layer, which acts as the parent of the compositing and software
   // layers. This layer is flipped so that the we don't need to recompute the
   // origin for sub-layers when their position changes (this is impossible when
@@ -31,48 +86,21 @@ class BrowserCompositorViewCocoaHelper;
   std::vector<ui::LatencyInfo> accelerated_latency_info_;
 
   base::scoped_nsobject<SoftwareLayer> software_layer_;
+};
 
-  content::BrowserCompositorViewMacClient* client_;
-  scoped_ptr<content::BrowserCompositorViewCocoaHelper> helper_;
-}
-
-// Change the client and superview of the view. If this is set to NULL then
-// the compositor will be prepared to be recycled.
-- (void)setClient:(content::BrowserCompositorViewMacClient*)client;
-
-// This is called to destroy the underlying ui::Compositor, if it is known
-// that this will not be recycled again.
-- (void)destroyCompositor;
-
-// Access the underlying ui::Compositor for this view.
-- (ui::Compositor*)compositor;
-
-// Called when the accelerated or software layer draws its frame to the screen.
-- (void)layerDidDrawFrame;
-
-// Called when an error is encountered while drawing to the screen.
-- (void)gotAcceleratedLayerError;
-
-@end  // BrowserCompositorViewCocoa
-
-namespace content {
-
-// This class implements the parts of BrowserCompositorViewCocoa that need to
-// be a C++ class and not an Objective C class.
-class BrowserCompositorViewCocoaHelper
-    : public content::CompositingIOSurfaceLayerClient {
- public:
-  BrowserCompositorViewCocoaHelper(BrowserCompositorViewCocoa* view)
-      : view_(view) {}
-  virtual ~BrowserCompositorViewCocoaHelper() {}
+}  // namespace content
 
- private:
-  // CompositingIOSurfaceLayerClient implementation:
-  virtual void AcceleratedLayerDidDrawFrame(bool succeeded) OVERRIDE;
+// BrowserCompositorViewCocoa is the actual NSView to which the layers drawn
+// by the ui::Compositor are attached.
+@interface BrowserCompositorViewCocoa : NSView {
+  content::BrowserCompositorViewCocoaClient* client_;
+}
 
-  BrowserCompositorViewCocoa* view_;
-};
+- (id)initWithClient:(content::BrowserCompositorViewCocoaClient*)client;
 
-}  // namespace content
+// Mark that the client provided at initialization is no longer valid and may
+// not be called back into.
+- (void)resetClient;
+@end
 
 #endif  // CONTENT_BROWSER_COMPOSITOR_BROWSER_COMPOSITOR_VIEW_PRIVATE_MAC_H_

content/browser/compositor/browser_compositor_view_private_mac.mm

@@ -14,17 +14,14 @@
 #include "ui/gl/scoped_cgl.h"
 
 ////////////////////////////////////////////////////////////////////////////////
-// BrowserCompositorViewCocoa
-
-@implementation BrowserCompositorViewCocoa : NSView
+// BrowserCompositorViewMacInternal
 
-- (id)init {
-  if (self = [super init]) {
-    accelerated_layer_output_surface_id_ = 0;
-    client_ = NULL;
-    helper_.reset(new content::BrowserCompositorViewCocoaHelper(self));
+namespace content {
 
-    // Disable the fade-in animation as the layer and view are added.
+BrowserCompositorViewMacInternal::BrowserCompositorViewMacInternal()
+    : client_(NULL),
+      accelerated_layer_output_surface_id_(0) {
+  // Disable the fade-in animation as the layers are added.
   ScopedCAActionDisabler disabler;
 
   // Add a flipped transparent layer as a child, so that we don't need to
@@ -36,65 +33,75 @@
   [flipped_layer_
       setAutoresizingMask:kCALayerWidthSizable|kCALayerHeightSizable];
 
-    compositor_.reset(new ui::Compositor(self, content::GetContextFactory()));
-  }
-  return self;
+  // Set the Cocoa view to be hosting the un-flipped background layer (hosting
+  // a flipped layer results in unpredictable behavior).
+  cocoa_view_.reset([[BrowserCompositorViewCocoa alloc] initWithClient:this]);
+
+  // Create a compositor to draw the contents of |cocoa_view_|.
+  compositor_.reset(new ui::Compositor(
+      cocoa_view_, content::GetContextFactory()));
 }
 
-- (void)setClient:(content::BrowserCompositorViewMacClient*)client {
-  // Disable the fade-out as layers are removed.
-  ScopedCAActionDisabler disabler;
+BrowserCompositorViewMacInternal::~BrowserCompositorViewMacInternal() {
+  DCHECK(!client_);
+  [cocoa_view_ resetClient];
+}
 
-  // Reset all state.
-  [flipped_layer_ removeFromSuperlayer];
-  [accelerated_layer_ removeFromSuperlayer];
-  [accelerated_layer_ resetClient];
-  accelerated_layer_.reset();
-  accelerated_layer_output_surface_id_ = 0;
-  [software_layer_ removeFromSuperlayer];
-  software_layer_.reset();
-  compositor_->SetScaleAndSize(1.0, gfx::Size(0, 0));
+void BrowserCompositorViewMacInternal::SetClient(
+    BrowserCompositorViewMacClient* client) {
+  // Disable the fade-in animation as the view is added.
+  ScopedCAActionDisabler disabler;
 
+  DCHECK(client && !client_);
   client_ = client;
-  if (client_) {
-    DCHECK(compositor_);
   compositor_->SetRootLayer(client_->BrowserCompositorRootLayer());
+
   CALayer* background_layer = [client_->BrowserCompositorSuperview() layer];
   DCHECK(background_layer);
   [flipped_layer_ setBounds:[background_layer bounds]];
   [background_layer addSublayer:flipped_layer_];
-  } else {
-    compositor_->SetRootLayer(NULL);
-  }
 }
 
-- (void)destroyCompositor {
-  DCHECK(!client_);
-  compositor_.reset();
-}
-
-- (void)gotAcceleratedLayerError {
-  if (!accelerated_layer_)
+void BrowserCompositorViewMacInternal::ResetClient() {
+  if (!client_)
     return;
 
-  [accelerated_layer_ context]->PoisonContextAndSharegroup();
-  compositor_->ScheduleFullRedraw();
-}
+  // Disable the fade-out animation as the view is removed.
+  ScopedCAActionDisabler disabler;
+
+  [flipped_layer_ removeFromSuperlayer];
+
+  [accelerated_layer_ removeFromSuperlayer];
+  [accelerated_layer_ resetClient];
+  accelerated_layer_.reset();
+  accelerated_layer_output_surface_id_ = 0;
+
+  [software_layer_ removeFromSuperlayer];
+  software_layer_.reset();
 
-- (ui::Compositor*)compositor {
-  return compositor_.get();
+  compositor_->SetScaleAndSize(1.0, gfx::Size(0, 0));
+  compositor_->SetRootLayer(NULL);
+  client_ = NULL;
 }
 
-- (void)gotAcceleratedIOSurfaceFrame:(IOSurfaceID)surface_handle
-                 withOutputSurfaceID:(int)surface_id
-                     withLatencyInfo:(std::vector<ui::LatencyInfo>) latency_info
-                       withPixelSize:(gfx::Size)pixel_size
-                     withScaleFactor:(float)scale_factor {
+void BrowserCompositorViewMacInternal::GotAcceleratedIOSurfaceFrame(
+    IOSurfaceID io_surface_id,
+    int output_surface_id,
+    const std::vector<ui::LatencyInfo>& latency_info,
+    gfx::Size pixel_size,
+    float scale_factor) {
   DCHECK(!accelerated_layer_output_surface_id_);
-  accelerated_layer_output_surface_id_ = surface_id;
+  accelerated_layer_output_surface_id_ = output_surface_id;
   accelerated_latency_info_.insert(accelerated_latency_info_.end(),
                                    latency_info.begin(), latency_info.end());
 
+  // If there is no client and therefore no superview to draw into, early-out.
+  if (!client_) {
+    AcceleratedLayerDidDrawFrame(true);
+    return;
+  }
+
+  // Disable the fade-in or fade-out effect if we create or remove layers.
   ScopedCAActionDisabler disabler;
 
   // If there is already an accelerated layer, but it has the wrong scale
@@ -109,23 +116,22 @@
 
   // If there is not a layer for accelerated frames, create one.
   if (!accelerated_layer_) {
-    // Disable the fade-in animation as the layer is added.
-    ScopedCAActionDisabler disabler;
     scoped_refptr<content::CompositingIOSurfaceMac> iosurface =
         content::CompositingIOSurfaceMac::Create();
     accelerated_layer_.reset([[CompositingIOSurfaceLayer alloc]
         initWithIOSurface:iosurface
           withScaleFactor:scale_factor
-               withClient:helper_.get()]);
+               withClient:this]);
     [flipped_layer_ addSublayer:accelerated_layer_];
   }
 
+  // Open the provided IOSurface.
   {
     bool result = true;
     gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
         [accelerated_layer_ context]->cgl_context());
     result = [accelerated_layer_ iosurface]->SetIOSurfaceWithContextCurrent(
-        [accelerated_layer_ context], surface_handle, pixel_size, scale_factor);
+        [accelerated_layer_ context], io_surface_id, pixel_size, scale_factor);
     if (!result)
       LOG(ERROR) << "Failed SetIOSurface on CompositingIOSurfaceMac";
   }
@@ -149,7 +155,6 @@
   // If there was a software layer or an old accelerated layer, remove it.
   // Disable the fade-out animation as the layer is removed.
   {
-    ScopedCAActionDisabler disabler;
     [software_layer_ removeFromSuperlayer];
     software_layer_.reset();
     [old_accelerated_layer resetClient];
@@ -158,20 +163,23 @@
   }
 }
 
-- (void)gotSoftwareFrame:(cc::SoftwareFrameData*)frame_data
-         withScaleFactor:(float)scale_factor
-              withCanvas:(SkCanvas*)canvas {
-  if (!frame_data || !canvas)
+void BrowserCompositorViewMacInternal::GotSoftwareFrame(
+    cc::SoftwareFrameData* frame_data,
+    float scale_factor,
+    SkCanvas* canvas) {
+  if (!frame_data || !canvas || !client_)
     return;
 
+  // Disable the fade-in or fade-out effect if we create or remove layers.
+  ScopedCAActionDisabler disabler;
+
   // If there is not a layer for software frames, create one.
   if (!software_layer_) {
-    // Disable the fade-in animation as the layer is added.
-    ScopedCAActionDisabler disabler;
     software_layer_.reset([[SoftwareLayer alloc] init]);
     [flipped_layer_ addSublayer:software_layer_];
   }
 
+  // Set the software layer to draw the provided canvas.
   SkImageInfo info;
   size_t row_bytes;
   const void* pixels = canvas->peekPixels(&info, &row_bytes);
@@ -181,23 +189,15 @@
                      withScaleFactor:scale_factor];
 
   // If there was an accelerated layer, remove it.
-  // Disable the fade-out animation as the layer is removed.
   {
-    ScopedCAActionDisabler disabler;
     [accelerated_layer_ resetClient];
     [accelerated_layer_ removeFromSuperlayer];
     accelerated_layer_.reset();
   }
-
-  // This call can be nested insider ui::Compositor commit calls, and can also
-  // make additional ui::Compositor commit calls. Avoid the potential recursion
-  // by acknowledging the frame asynchronously.
-  [self performSelector:@selector(layerDidDrawFrame)
-             withObject:nil
-             afterDelay:0];
 }
 
-- (void)layerDidDrawFrame {
+void BrowserCompositorViewMacInternal::AcceleratedLayerDidDrawFrame(
+    bool succeeded) {
   if (accelerated_layer_output_surface_id_) {
     content::ImageTransportFactory::GetInstance()->OnSurfaceDisplayed(
         accelerated_layer_output_surface_id_);
@@ -206,22 +206,57 @@
 
   if (client_)
     client_->BrowserCompositorViewFrameSwapped(accelerated_latency_info_);
+
   accelerated_latency_info_.clear();
+
+  if (!succeeded) {
+    if (accelerated_layer_)
+      [accelerated_layer_ context]->PoisonContextAndSharegroup();
+    compositor_->ScheduleFullRedraw();
+  }
 }
 
-@end  // BrowserCompositorViewCocoa
+}  // namespace content
 
 ////////////////////////////////////////////////////////////////////////////////
-// BrowserCompositorViewCocoaHelper
+// BrowserCompositorViewCocoa
 
-namespace content {
+@implementation BrowserCompositorViewCocoa
 
-void BrowserCompositorViewCocoaHelper::AcceleratedLayerDidDrawFrame(
-    bool succeeded) {
-  [view_ layerDidDrawFrame];
-  if (!succeeded)
-    [view_ gotAcceleratedLayerError];
+- (id)initWithClient:(content::BrowserCompositorViewCocoaClient*)client {
+  if (self = [super init]) {
+    client_ = client;
+  }
+  return self;
+}
+
+- (void)dealloc {
+  DCHECK(!client_);
+  [super dealloc];
 }
 
+- (void)resetClient {
+  client_ = NULL;
 }
 
+- (void)gotAcceleratedIOSurfaceFrame:(IOSurfaceID)surface_handle
+                 withOutputSurfaceID:(int)surface_id
+                     withLatencyInfo:(std::vector<ui::LatencyInfo>)latency_info
+                       withPixelSize:(gfx::Size)pixel_size
+                     withScaleFactor:(float)scale_factor {
+  if (!client_)
+    return;
+  client_->GotAcceleratedIOSurfaceFrame(
+      surface_handle, surface_id, latency_info, pixel_size, scale_factor);
+}
+
+- (void)gotSoftwareFrame:(cc::SoftwareFrameData*)frame_data
+         withScaleFactor:(float)scale_factor
+              withCanvas:(SkCanvas*)canvas {
+  if (!client_)
+    return;
+  client_->GotSoftwareFrame(frame_data, scale_factor, canvas);
+}
+
+@end  // BrowserCompositorViewCocoa
+

content/browser/renderer_host/compositing_iosurface_layer_mac.h

@@ -11,43 +11,91 @@
 #include "base/memory/ref_counted.h"
 #include "base/timer/timer.h"
 
+@class CompositingIOSurfaceLayer;
+
 namespace content {
 class CompositingIOSurfaceMac;
 class CompositingIOSurfaceContext;
-class CompositingIOSurfaceLayerHelper;
 
+// The interface through which the CompositingIOSurfaceLayer calls back into
+// the structrue that created it (RenderWidgetHostViewMac or
+// BrowserCompositorViewMac).
 class CompositingIOSurfaceLayerClient {
  public:
   virtual void AcceleratedLayerDidDrawFrame(bool succeeded) = 0;
 };
 
-}  // namespace content
+// CompositingIOSurfaceLayerHelper provides C++ functionality needed for the
+// CompositingIOSurfaceLayer class, and does most of the heavy lifting for the
+// class.
+// TODO(ccameron): This class should own CompositingIOSurfaceLayer, rather than
+// vice versa.
+class CompositingIOSurfaceLayerHelper {
+ public:
+  CompositingIOSurfaceLayerHelper(CompositingIOSurfaceLayerClient* client,
+                                  CompositingIOSurfaceLayer* layer);
+  ~CompositingIOSurfaceLayerHelper();
 
-// The CoreAnimation layer for drawing accelerated content.
-@interface CompositingIOSurfaceLayer : CAOpenGLLayer {
- @private
-  content::CompositingIOSurfaceLayerClient* client_;
-  scoped_refptr<content::CompositingIOSurfaceMac> iosurface_;
-  scoped_refptr<content::CompositingIOSurfaceContext> context_;
+  // Called when the CompositingIOSurfaceLayer gets a new frame.
+  void GotNewFrame();
 
-  // The browser places back-pressure on the GPU by not acknowledging swap
-  // calls until they appear on the screen. This can lead to hangs if the
-  // view is moved offscreen (among other things). Prevent hangs by always
-  // acknowledging the frame after timeout of 1/6th of a second  has passed.
-  scoped_ptr<content::CompositingIOSurfaceLayerHelper> helper_;
-  scoped_ptr<base::DelayTimer<content::CompositingIOSurfaceLayerHelper>> timer_;
+  // Called whenever -[CompositingIOSurfaceLayer setNeedsDisplay] is called.
+  void SetNeedsDisplay();
+
+  // Called whenever -[CompositingIOSurfaceLayer canDrawInCGLContext] is called,
+  // to determine if a new frame should be drawn.
+  bool CanDraw();
+
+  // Called whenever -[CompositingIOSurfaceLayer drawInCGLContext] draws a
+  // frame.
+  void DidDraw(bool success);
+
+ private:
+  // Immediately draw a frame (disregarding vsync) and ensure that the frame is
+  // acknowledged.
+  void ImmediatelyForceDisplayAndAck();
+
+  // Called whenever the frame provided in GotNewFrame should be acknowledged
+  // (this may be because it was drawn, or it may be to unblock the
+  // compositor).
+  void AckPendingFrame(bool success);
+
+  void TimerFired();
+
+  // The client that the owning layer was created with.
+  content::CompositingIOSurfaceLayerClient* const client_;
+
+  // The layer that owns this helper.
+  CompositingIOSurfaceLayer* const layer_;
 
   // Used to track when canDrawInCGLContext should return YES. This can be
   // in response to receiving a new compositor frame, or from any of the events
   // that cause setNeedsDisplay to be called on the layer.
-  BOOL needs_display_;
+  bool needs_display_;
 
   // This is set when a frame is received, and un-set when the frame is drawn.
-  BOOL has_pending_frame_;
+  bool has_pending_frame_;
 
   // Incremented every time that this layer is asked to draw but does not have
   // new content to draw.
   uint64 did_not_draw_counter_;
+
+  // The browser places back-pressure on the GPU by not acknowledging swap
+  // calls until they appear on the screen. This can lead to hangs if the
+  // view is moved offscreen (among other things). Prevent hangs by always
+  // acknowledging the frame after timeout of 1/6th of a second  has passed.
+  base::DelayTimer<CompositingIOSurfaceLayerHelper> timer_;
+};
+
+}  // namespace content
+
+// The CoreAnimation layer for drawing accelerated content.
+@interface CompositingIOSurfaceLayer : CAOpenGLLayer {
+ @private
+  scoped_refptr<content::CompositingIOSurfaceMac> iosurface_;
+  scoped_refptr<content::CompositingIOSurfaceContext> context_;
+
+  scoped_ptr<content::CompositingIOSurfaceLayerHelper> helper_;
 }
 
 - (content::CompositingIOSurfaceMac*)iosurface;
@@ -58,7 +106,8 @@ class CompositingIOSurfaceLayerClient {
         withScaleFactor:(float)scale_factor
              withClient:(content::CompositingIOSurfaceLayerClient*)client;
 
-// Mark that the client is no longer valid and cannot be called back into.
+// Mark that the client is no longer valid and cannot be called back into. This
+// must be called before the layer is destroyed.
 - (void)resetClient;
 
 // Called when a new frame is received.

content/browser/renderer_host/compositing_iosurface_layer_mac.mm

@@ -17,29 +17,114 @@
 #include "ui/gfx/size_conversions.h"
 #include "ui/gl/gpu_switching_manager.h"
 
-@interface CompositingIOSurfaceLayer(Private)
-- (void)immediatelyForceDisplayAndAck;
-- (void)ackPendingFrame:(bool)success;
-- (void)timerFired;
-@end
+////////////////////////////////////////////////////////////////////////////////
+// CompositingIOSurfaceLayerHelper
 
 namespace content {
 
-// The base::DelayTimer needs a C++ class to operate on, rather than Objective C
-// class. This helper class provides a bridge between the two.
-class CompositingIOSurfaceLayerHelper {
- public:
-  CompositingIOSurfaceLayerHelper(CompositingIOSurfaceLayer* layer)
-      : layer_(layer) {}
-  void TimerFired() {
-    [layer_ timerFired];
+CompositingIOSurfaceLayerHelper::CompositingIOSurfaceLayerHelper(
+    CompositingIOSurfaceLayerClient* client,
+    CompositingIOSurfaceLayer* layer)
+        : client_(client),
+          layer_(layer),
+          needs_display_(false),
+          has_pending_frame_(false),
+          did_not_draw_counter_(0),
+          timer_(
+              FROM_HERE,
+              base::TimeDelta::FromSeconds(1) / 6,
+              this,
+              &CompositingIOSurfaceLayerHelper::TimerFired) {}
+
+CompositingIOSurfaceLayerHelper::~CompositingIOSurfaceLayerHelper() {
+  // Any acks that were waiting on this layer to draw will not occur, so ack
+  // them now to prevent blocking the renderer.
+  AckPendingFrame(true);
+}
+
+void CompositingIOSurfaceLayerHelper::GotNewFrame() {
+  has_pending_frame_ = true;
+  needs_display_ = true;
+  timer_.Reset();
+
+  if ([layer_ context] && [layer_ context]->is_vsync_disabled()) {
+    // If vsync is disabled, draw immediately and don't bother trying to use
+    // the isAsynchronous property to ensure smooth animation.
+    ImmediatelyForceDisplayAndAck();
+  } else {
+    needs_display_ = YES;
+    if (![layer_ isAsynchronous])
+      [layer_ setAsynchronous:YES];
   }
- private:
-  CompositingIOSurfaceLayer* layer_;
-};
+
+  // A trace value of 2 indicates that there is a pending swap ack. See
+  // canDrawInCGLContext for other value meanings.
+  TRACE_COUNTER_ID1("browser", "PendingSwapAck", this, 2);
+}
+
+void CompositingIOSurfaceLayerHelper::SetNeedsDisplay() {
+  needs_display_ = true;
+}
+
+bool CompositingIOSurfaceLayerHelper::CanDraw() {
+  // If we return NO 30 times in a row, switch to being synchronous to avoid
+  // burning CPU cycles on this callback.
+  if (needs_display_) {
+    did_not_draw_counter_ = 0;
+  } else {
+    did_not_draw_counter_ += 1;
+    if (did_not_draw_counter_ == 30)
+      [layer_ setAsynchronous:NO];
+  }
+
+  // Add an instantaneous blip to the PendingSwapAck state to indicate
+  // that CoreAnimation asked if a frame is ready. A blip up to to 3 (usually
+  // from 2, indicating that a swap ack is pending) indicates that we
+  // requested a draw. A blip up to 1 (usually from 0, indicating there is no
+  // pending swap ack) indicates that we did not request a draw. This would
+  // be more natural to do with a tracing pseudo-thread
+  // http://crbug.com/366300
+  TRACE_COUNTER_ID1("browser", "PendingSwapAck", this, needs_display_ ? 3 : 1);
+  TRACE_COUNTER_ID1("browser", "PendingSwapAck", this,
+                    has_pending_frame_ ? 2 : 0);
+
+  return needs_display_;
+}
+
+void CompositingIOSurfaceLayerHelper::DidDraw(bool success) {
+  needs_display_ = false;
+  AckPendingFrame(success);
+}
+
+void CompositingIOSurfaceLayerHelper::AckPendingFrame(bool success) {
+  if (!has_pending_frame_)
+    return;
+  has_pending_frame_ = false;
+  client_->AcceleratedLayerDidDrawFrame(success);
+  // A trace value of 0 indicates that there is no longer a pending swap ack.
+  TRACE_COUNTER_ID1("browser", "PendingSwapAck", this, 0);
+}
+
+void CompositingIOSurfaceLayerHelper::ImmediatelyForceDisplayAndAck() {
+  [layer_ setNeedsDisplay];
+  [layer_ displayIfNeeded];
+
+  // Calls to setNeedsDisplay can sometimes be ignored, especially if issued
+  // rapidly (e.g, with vsync off). This is unacceptable because the failure
+  // to ack a single frame will hang the renderer. Ensure that the renderer
+  // not be blocked by lying and claiming that we drew the frame.
+  AckPendingFrame(true);
+}
+
+void CompositingIOSurfaceLayerHelper::TimerFired() {
+  ImmediatelyForceDisplayAndAck();
+}
 
 }  // namespace content
 
+////////////////////////////////////////////////////////////////////////////////
+// CompositingIOSurfaceLayer
+
 @implementation CompositingIOSurfaceLayer
 
 - (content::CompositingIOSurfaceMac*)iosurface {
@@ -55,21 +140,12 @@ class CompositingIOSurfaceLayerHelper {
         withScaleFactor:(float)scale_factor
              withClient:(content::CompositingIOSurfaceLayerClient*)client {
   if (self = [super init]) {
-    iosurface_ = iosurface;
-    client_ = client;
-    helper_.reset(new content::CompositingIOSurfaceLayerHelper(self));
-    timer_.reset(new base::DelayTimer<content::CompositingIOSurfaceLayerHelper>(
-        FROM_HERE,
-        base::TimeDelta::FromSeconds(1) / 6,
-        helper_.get(),
-        &content::CompositingIOSurfaceLayerHelper::TimerFired));
+    helper_.reset(new content::CompositingIOSurfaceLayerHelper(client, self));
 
+    iosurface_ = iosurface;
     context_ = content::CompositingIOSurfaceContext::Get(
         content::CompositingIOSurfaceContext::kCALayerContextWindowNumber);
     DCHECK(context_);
-    needs_display_ = NO;
-    has_pending_frame_ = NO;
-    did_not_draw_counter_ = 0;
 
     [self setBackgroundColor:CGColorGetConstantColor(kCGColorWhite)];
     [self setAnchorPoint:CGPointMake(0, 0)];
@@ -83,58 +159,17 @@ class CompositingIOSurfaceLayerHelper {
   return self;
 }
 
-- (void)resetClient {
-  // Any acks that were waiting on this layer to draw will not occur, so ack
-  // them now to prevent blocking the renderer.
-  [self ackPendingFrame:true];
-  client_ = NULL;
-}
-
-- (void)gotNewFrame {
-  has_pending_frame_ = YES;
-  timer_->Reset();
-
-  // A trace value of 2 indicates that there is a pending swap ack. See
-  // canDrawInCGLContext for other value meanings.
-  TRACE_COUNTER_ID1("browser", "PendingSwapAck", self, 2);
-
-  if (context_ && context_->is_vsync_disabled()) {
-    // If vsync is disabled, draw immediately and don't bother trying to use
-    // the isAsynchronous property to ensure smooth animation.
-    [self immediatelyForceDisplayAndAck];
-  } else {
-    needs_display_ = YES;
-    if (![self isAsynchronous])
-      [self setAsynchronous:YES];
-  }
-}
-
-// Private methods:
-
-- (void)immediatelyForceDisplayAndAck {
-  [self setNeedsDisplay];
-  [self displayIfNeeded];
-
-  // Calls to setNeedsDisplay can sometimes be ignored, especially if issued
-  // rapidly (e.g, with vsync off). This is unacceptable because the failure
-  // to ack a single frame will hang the renderer. Ensure that the renderer
-  // not be blocked by lying and claiming that we drew the frame.
-  [self ackPendingFrame:true];
+- (void)dealloc {
+  DCHECK(!helper_);
+  [super dealloc];
 }
 
-- (void)ackPendingFrame:(bool)success {
-  if (!has_pending_frame_)
-    return;
-
-  TRACE_COUNTER_ID1("browser", "PendingSwapAck", self, 0);
-  has_pending_frame_ = NO;
-  if (client_)
-    client_->AcceleratedLayerDidDrawFrame(success);
+- (void)resetClient {
+  helper_.reset();
 }
 
-- (void)timerFired {
-  if (has_pending_frame_)
-    [self immediatelyForceDisplayAndAck];
+- (void)gotNewFrame {
+  helper_->GotNewFrame();
 }
 
 // The remaining methods implement the CAOpenGLLayer interface.
@@ -152,7 +187,8 @@ class CompositingIOSurfaceLayerHelper {
 }
 
 - (void)setNeedsDisplay {
-  needs_display_ = YES;
+  if (helper_)
+    helper_->SetNeedsDisplay();
   [super setNeedsDisplay];
 }
 
@@ -160,28 +196,9 @@ class CompositingIOSurfaceLayerHelper {
                 pixelFormat:(CGLPixelFormatObj)pixelFormat
                forLayerTime:(CFTimeInterval)timeInterval
                 displayTime:(const CVTimeStamp*)timeStamp {
-  // Add an instantaneous blip to the PendingSwapAck state to indicate
-  // that CoreAnimation asked if a frame is ready. A blip up to to 3 (usually
-  // from 2, indicating that a swap ack is pending) indicates that we requested
-  // a draw. A blip up to 1 (usually from 0, indicating there is no pending swap
-  // ack) indicates that we did not request a draw. This would be more natural
-  // to do with a tracing pseudo-thread
-  // http://crbug.com/366300
-  TRACE_COUNTER_ID1("browser", "PendingSwapAck", self, needs_display_ ? 3 : 1);
-  TRACE_COUNTER_ID1("browser", "PendingSwapAck", self,
-                    has_pending_frame_ ? 2 : 0);
-
-  // If we return NO 30 times in a row, switch to being synchronous to avoid
-  // burning CPU cycles on this callback.
-  if (needs_display_) {
-    did_not_draw_counter_ = 0;
-  } else {
-    did_not_draw_counter_ += 1;
-    if (did_not_draw_counter_ > 30)
-      [self setAsynchronous:NO];
-  }
-
-  return needs_display_;
+  if (helper_)
+    return helper_->CanDraw();
+  return NO;
 }
 
 - (void)drawInCGLContext:(CGLContextObj)glContext
@@ -211,8 +228,8 @@ class CompositingIOSurfaceLayerHelper {
   bool draw_succeeded = iosurface_->DrawIOSurface(
       context_, window_rect, window_scale_factor);
 
-  [self ackPendingFrame:draw_succeeded];
-  needs_display_ = NO;
+  if (helper_)
+    helper_->DidDraw(draw_succeeded);
 
   [super drawInCGLContext:glContext
               pixelFormat:pixelFormat