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

content/browser/compositor/browser_compositor_view_private_mac.mm

Index: content/browser/compositor/browser_compositor_view_private_mac.mm
diff --git a/content/browser/compositor/browser_compositor_view_private_mac.mm b/content/browser/compositor/browser_compositor_view_private_mac.mm
index b6b69f31ceb02f81d11eba424d08d7e99b47123e..c1b5d53abeed8468e7a5d128dbc16e2dce0c3837 100644
--- a/content/browser/compositor/browser_compositor_view_private_mac.mm
+++ b/content/browser/compositor/browser_compositor_view_private_mac.mm
@@ -14,91 +14,100 @@
 #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));
-    [self setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
-
-    // Disable the fade-in animation as the layer and view are added.
-    ScopedCAActionDisabler disabler;
-
-    // Make this view host a transparent layer.
-    background_layer_.reset([[CALayer alloc] init]);
-    [self setLayer:background_layer_];
-    [self setWantsLayer:YES];
-
-    // Add a flipped transparent layer as a child, so that we don't need to
-    // fiddle with the position of sub-layers -- they will always be at the
-    // origin.
-    flipped_layer_.reset([[CALayer alloc] init]);
-    [flipped_layer_ setGeometryFlipped:YES];
-    [flipped_layer_
-        setAutoresizingMask:kCALayerWidthSizable|kCALayerHeightSizable];
-    [background_layer_ addSublayer:flipped_layer_];
-
-    compositor_.reset(new ui::Compositor(self, content::GetContextFactory()));
-  }
-  return self;
-}
+namespace content {
 
-- (void)setClient:(content::BrowserCompositorViewMacClient*)client {
-  // Disable the fade-out as layers are removed.
+BrowserCompositorViewMacInternal::BrowserCompositorViewMacInternal()
+    : client_(NULL),
+      accelerated_layer_output_surface_id_(0) {
+  // Disable the fade-in animation as the layers are added.
   ScopedCAActionDisabler disabler;
-  [self removeFromSuperview];
-
-  // Reset all state.
-  [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));
 
-  client_ = client;
-  if (client_) {
-    DCHECK(compositor_);
-    compositor_->SetRootLayer(client_->BrowserCompositorRootLayer());
-    [self setFrame:[client_->BrowserCompositorSuperview() frame]];
-    [client_->BrowserCompositorSuperview() addSubview:self];
-  } else {
-    compositor_->SetRootLayer(NULL);
-  }
+  // Make this view host a transparent layer.
+  background_layer_.reset([[CALayer alloc] init]);
+
+  // Add a flipped transparent layer as a child, so that we don't need to
+  // fiddle with the position of sub-layers -- they will always be at the
+  // origin.
+  flipped_layer_.reset([[CALayer alloc] init]);
+  [flipped_layer_ setGeometryFlipped:YES];
+  [flipped_layer_
+      setAutoresizingMask:kCALayerWidthSizable|kCALayerHeightSizable];
+  [background_layer_ addSublayer:flipped_layer_];
+
+  // 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]);
+  [cocoa_view_ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+  [cocoa_view_ setLayer:background_layer_];
+  [cocoa_view_ setWantsLayer:YES];
+
+  // Create a compositor to draw the contents of |cocoa_view_|.
+  compositor_.reset(new ui::Compositor(
+      cocoa_view_, content::GetContextFactory()));
 }
 
-- (void)destroyCompositor {
+BrowserCompositorViewMacInternal::~BrowserCompositorViewMacInternal() {
   DCHECK(!client_);
+  [cocoa_view_ resetClient];
+  // Ensure that the compositor be destroyed before the NSView that it is
+  // using.
   compositor_.reset();

[miu, 2014/07/16 22:31:55]

nit: Your choice, but you could let C++ destruction order achieve this by having
the compositor_ member declared after the cocoa_view_ member.  The comment is
good, so you'd want to move that to the header file where compositor_ is
declared.

[ccameron, 2014/07/17 02:30:02]

Moved this to the header variable ordering.

 }
 
-- (void)gotAcceleratedLayerError {
-  if (!accelerated_layer_)
-    return;
+void BrowserCompositorViewMacInternal::SetClient(
+    BrowserCompositorViewMacClient* client) {
+  // Disable the fade-in animation as the view is added.
+  ScopedCAActionDisabler disabler;
 
-  [accelerated_layer_ context]->PoisonContextAndSharegroup();
-  compositor_->ScheduleFullRedraw();
+  DCHECK(client && !client_);
+  client_ = client;
+  compositor_->SetRootLayer(client_->BrowserCompositorRootLayer());
+  [cocoa_view_ setFrame:[client_->BrowserCompositorSuperview() frame]];
+  [client_->BrowserCompositorSuperview() addSubview:cocoa_view_];
 }
 
-- (ui::Compositor*)compositor {
-  return compositor_.get();
+void BrowserCompositorViewMacInternal::ResetClient() {
+  if (!client_)
+    return;
+
+  // Disable the fade-out animation as the view is removed.
+  ScopedCAActionDisabler disabler;
+
+  [cocoa_view_ removeFromSuperview];
+
+  [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));
+  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
@@ -113,23 +122,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";
   }
@@ -153,7 +161,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];
@@ -162,20 +169,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);
@@ -185,23 +195,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_);
@@ -210,22 +212,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
+