Cleanups to well-behaved accelerated plugin CL requested by

stuartmorgan in http://codereview.chromium.org/3010054 .

BUG=51748
TEST=visited YouTube and ensured video shows up; 3D CSS demos; WebGL demos

Review URL: http://codereview.chromium.org/3185004

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

6 files changed, 69 insertions, 96 deletions

diff --git a/chrome/browser/renderer_host/accelerated_surface_container_mac.cc b/chrome/browser/renderer_host/accelerated_surface_container_mac.cc
index 3ca3328..92727f0 100644
--- a/chrome/browser/renderer_host/accelerated_surface_container_mac.cc
+++ b/chrome/browser/renderer_host/accelerated_surface_container_mac.cc
@@ -18,11 +18,11 @@ AcceleratedSurfaceContainerMac::AcceleratedSurfaceContainerMac(
       width_(0),
       height_(0),
       texture_(0),
-      texture_needs_upload_(true) {
+      texture_needs_upload_(true),
+      texture_pending_deletion_(0) {
 }
 
 AcceleratedSurfaceContainerMac::~AcceleratedSurfaceContainerMac() {
-  EnqueueTextureForDeletion();
   ReleaseIOSurface();
 }
 
@@ -60,7 +60,7 @@ void AcceleratedSurfaceContainerMac::SetSizeAndTransportDIB(
   }
 }
 
-void AcceleratedSurfaceContainerMac::MoveTo(
+void AcceleratedSurfaceContainerMac::SetGeometry(
     const webkit_glue::WebPluginGeometry& geom) {
   // TODO(kbr): may need to pay attention to cutout rects.
   if (geom.visible)
@@ -72,6 +72,19 @@ void AcceleratedSurfaceContainerMac::MoveTo(
 void AcceleratedSurfaceContainerMac::Draw(CGLContextObj context) {
   IOSurfaceSupport* io_surface_support = IOSurfaceSupport::Initialize();
   GLenum target = GL_TEXTURE_RECTANGLE_ARB;
+  if (texture_pending_deletion_) {
+    // Clean up an old texture object. This is essentially a pre-emptive
+    // cleanup, as the resources will be released when the OpenGL context
+    // associated with our containing NSView is destroyed. However, if we
+    // resize a plugin often, we might generate a lot of textures, so we
+    // should try to eagerly reclaim their resources. Note also that the
+    // OpenGL context must be current when performing the deletion, and it
+    // seems risky to make the OpenGL context current at an arbitrary point
+    // in time, which is why the deletion does not occur in the container's
+    // destructor.
+    glDeleteTextures(1, &texture_pending_deletion_);
+    texture_pending_deletion_ = 0;
+  }
   if (!texture_) {
     if ((io_surface_support && !surface_) ||
         (!io_surface_support && !transport_dib_.get()))
@@ -191,7 +204,8 @@ void AcceleratedSurfaceContainerMac::Draw(CGLContextObj context) {
 
 void AcceleratedSurfaceContainerMac::EnqueueTextureForDeletion() {
   if (texture_) {
-    manager_->EnqueueTextureForDeletion(texture_);
+    DCHECK(texture_pending_deletion_ == 0);
+    texture_pending_deletion_ = texture_;
     texture_ = 0;
   }
 }
diff --git a/chrome/browser/renderer_host/accelerated_surface_container_mac.h b/chrome/browser/renderer_host/accelerated_surface_container_mac.h
index 8e816f2..b7a6883 100644
--- a/chrome/browser/renderer_host/accelerated_surface_container_mac.h
+++ b/chrome/browser/renderer_host/accelerated_surface_container_mac.h
@@ -59,9 +59,12 @@ class AcceleratedSurfaceContainerMac {
                               int32 height,
                               TransportDIB::Handle transport_dib);
 
-  // Tells the accelerated surface container that it has moved relative to the
-  // origin of the window, for example because of a scroll event.
-  void MoveTo(const webkit_glue::WebPluginGeometry& geom);
+  // Tells the accelerated surface container that its geometry has changed,
+  // for example because of a scroll event. (Note that the container
+  // currently only pays attention to the clip width and height, since the
+  // view in which it is hosted is responsible for positioning it on the
+  // page.)
+  void SetGeometry(const webkit_glue::WebPluginGeometry& geom);
 
   // Draws this accelerated surface's contents, texture mapped onto a quad in
   // the given OpenGL context. TODO(kbr): figure out and define exactly how the
@@ -107,6 +110,10 @@ class AcceleratedSurfaceContainerMac {
   // True if we need to upload the texture again during the next draw.
   bool texture_needs_upload_;
 
+  // This may refer to an old version of the texture if the container is
+  // resized, for example.
+  GLuint texture_pending_deletion_;
+
   // Releases the IOSurface reference, if any, retained by this object.
   void ReleaseIOSurface();
 
diff --git a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
index 9f1d9f3..bbd4418 100644
--- a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
+++ b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
@@ -65,34 +65,16 @@ void AcceleratedSurfaceContainerManagerMac::SetSizeAndTransportDIB(
     container->SetSizeAndTransportDIB(width, height, transport_dib);
 }
 
-void AcceleratedSurfaceContainerManagerMac::MovePluginContainer(
+void AcceleratedSurfaceContainerManagerMac::SetPluginContainerGeometry(
     const webkit_glue::WebPluginGeometry& move) {
   AcceleratedSurfaceContainerMac* container = MapIDToContainer(move.window);
   if (container)
-    container->MoveTo(move);
+    container->SetGeometry(move);
 }
 
 void AcceleratedSurfaceContainerManagerMac::Draw(CGLContextObj context,
                                                  gfx::PluginWindowHandle id,
                                                  bool draw_root_container) {
-  // Clean up old texture objects. This is essentially a pre-emptive
-  // cleanup, as the resources will be released when the OpenGL
-  // context associated with the CAOpenGLLayer is destroyed. However,
-  // if we render many plugins in the same layer, we should try to
-  // eagerly reclaim their resources. Note also that the OpenGL
-  // context must be current when performing the deletion, and it
-  // seems risky to make the OpenGL context current at an arbitrary
-  // point in time, which is why the deletion does not occur in the
-  // container's destructor.
-  for (std::vector<GLuint>::iterator iter =
-           textures_pending_deletion_.begin();
-       iter != textures_pending_deletion_.end();
-       ++iter) {
-    GLuint texture = *iter;
-    glDeleteTextures(1, &texture);
-  }
-  textures_pending_deletion_.clear();
-
   glColorMask(true, true, true, true);
   glClearColor(0, 0, 0, 0);
   glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
@@ -123,13 +105,6 @@ void AcceleratedSurfaceContainerManagerMac::ForceTextureReload() {
   }
 }
 
-void AcceleratedSurfaceContainerManagerMac::EnqueueTextureForDeletion(
-    GLuint texture) {
-  if (texture) {
-    textures_pending_deletion_.push_back(texture);
-  }
-}
-
 AcceleratedSurfaceContainerMac*
     AcceleratedSurfaceContainerManagerMac::MapIDToContainer(
         gfx::PluginWindowHandle id) {
diff --git a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.h b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.h
index cdd2f6b..101945f 100644
--- a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.h
+++ b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.h
@@ -34,9 +34,9 @@ class AcceleratedSurfaceContainerManagerMac {
   // Destroys a fake PluginWindowHandle and associated storage.
   void DestroyFakePluginWindowHandle(gfx::PluginWindowHandle id);
 
-  // Indicates whether a "root" PluginWindowHandle has been allocated,
-  // which means that we are using accelerated compositing and should
-  // short-circuit the normal drawing process.
+  // Indicates whether the given PluginWindowHandle is "root", which
+  // means that we are using accelerated compositing and that this one
+  // contains the compositor's output.
   bool IsRootContainer(gfx::PluginWindowHandle id);
 
   // Sets the size and backing store of the plugin instance.  There are two
@@ -54,10 +54,10 @@ class AcceleratedSurfaceContainerManagerMac {
 
   // Takes an update from WebKit about a plugin's position and size and moves
   // the plugin accordingly.
-  void MovePluginContainer(const webkit_glue::WebPluginGeometry& move);
+  void SetPluginContainerGeometry(const webkit_glue::WebPluginGeometry& move);
 
-  // Draws all of the managed plugin containers into the given OpenGL
-  // context, which must already be current.
+  // Draws the plugin container associated with the given id into the given
+  // OpenGL context, which must already be current.
   void Draw(CGLContextObj context,
             gfx::PluginWindowHandle id,
             bool draw_root_container);
@@ -66,10 +66,6 @@ class AcceleratedSurfaceContainerManagerMac {
   // Should be called any time the drawing context has changed.
   void ForceTextureReload();
 
-  // Called by the container to enqueue its OpenGL texture objects for
-  // deletion.
-  void EnqueueTextureForDeletion(GLuint texture);
-
  private:
   uint32 current_id_;
 
@@ -92,9 +88,6 @@ class AcceleratedSurfaceContainerManagerMac {
   // compositor is active.
   AcceleratedSurfaceContainerMac* root_container_;
 
-  // A list of OpenGL textures waiting to be deleted
-  std::vector<GLuint> textures_pending_deletion_;
-
   DISALLOW_COPY_AND_ASSIGN(AcceleratedSurfaceContainerManagerMac);
 };
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h
index 798a07e..f0ec372 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h
@@ -62,10 +62,6 @@ class RWHVMEditCommandHelper;
 
   NSWindow* lastWindow_;  // weak
 
-  // The Core Animation layer, if any, hosting the accelerated
-  // plugins' and accelerated compositor's output.
-  scoped_nsobject<CALayer> acceleratedPluginLayer_;
-
   // Variables used by our implementaion of the NSTextInput protocol.
   // An input method of Mac calls the methods of this protocol not only to
   // notify an application of its status, but also to retrieve the status of
@@ -133,9 +129,6 @@ class RWHVMEditCommandHelper;
 - (void)setCanBeKeyView:(BOOL)can;
 - (void)setCloseOnDeactivate:(BOOL)b;
 - (void)setToolTipAtMousePoint:(NSString *)string;
-// Triggers a refresh of the accelerated plugin layer; should be called whenever
-// the shared surface for one of the plugins is updated.
-- (void)drawAcceleratedPluginLayer;
 // Set frame, then notify the RenderWidgetHost that the frame has been changed,
 // but do it in a separate task, using |performSelector:withObject:afterDelay:|.
 // This stops the flickering issue in http://crbug.com/31970
@@ -242,7 +235,8 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
   virtual void AcceleratedSurfaceBuffersSwapped(gfx::PluginWindowHandle window);
   void DrawAcceleratedSurfaceInstance(
       CGLContextObj context, gfx::PluginWindowHandle plugin_handle);
-  // Informs the plug-in instances that their drawing context has changed.
+  // Forces the textures associated with any accelerated plugin instances
+  // to be reloaded.
   void ForceTextureReload();
 
   virtual void SetVisuallyDeemphasized(bool deemphasized);
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.mm b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
index 9236bbe..b4e9969 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
@@ -175,7 +175,7 @@ void DisablePasswordInput() {
 }
 
 // This is the renderer output callback function
-static CVReturn MyDisplayLinkCallback(
+static CVReturn DrawOneAcceleratedPluginCallback(
     CVDisplayLinkRef displayLink,
     const CVTimeStamp* now,
     const CVTimeStamp* outputTime,
@@ -213,7 +213,8 @@ static CVReturn MyDisplayLinkCallback(
 
     // Set up a display link to do OpenGL rendering on a background thread.
     CVDisplayLinkCreateWithActiveCGDisplays(&displayLink_);
-    CVDisplayLinkSetOutputCallback(displayLink_, &MyDisplayLinkCallback, self);
+    CVDisplayLinkSetOutputCallback(displayLink_,
+        &DrawOneAcceleratedPluginCallback, self);
     CVDisplayLinkSetCurrentCGDisplayFromOpenGLContext(
         displayLink_, cglContext_, cglPixelFormat_);
     CVDisplayLinkStart(displayLink_);
@@ -435,12 +436,15 @@ void RenderWidgetHostViewMac::MovePluginWindows(
       }
 
       PluginViewMap::iterator it = plugin_views_.find(geom.window);
-      CHECK(plugin_views_.end() != it);
+      DCHECK(plugin_views_.end() != it);
+      if (plugin_views_.end() == it) {
+        continue;
+      }
       NSRect new_rect([cocoa_view_ RectToNSRect:rect]);
       [it->second setFrame:new_rect];
       [it->second setNeedsDisplay:YES];
 
-      plugin_container_manager_.MovePluginContainer(geom);
+      plugin_container_manager_.SetPluginContainerGeometry(geom);
     }
   }
 }
@@ -591,7 +595,7 @@ void RenderWidgetHostViewMac::Destroy() {
     // deepest-first ordering.
     for (NSView* subview in [cocoa_view_ subviews]) {
       if (![subview isKindOfClass:[RenderWidgetHostViewCocoa class]])
-        continue;  // Skip plugin views.
+        continue;  // Skip accelerated views.
 
       [static_cast<RenderWidgetHostViewCocoa*>(subview)
           renderWidgetHostViewMac]->ShutdownHost();
@@ -756,10 +760,12 @@ RenderWidgetHostViewMac::AllocateFakePluginWindowHandle(bool opaque,
 void RenderWidgetHostViewMac::DestroyFakePluginWindowHandle(
     gfx::PluginWindowHandle window) {
   PluginViewMap::iterator it = plugin_views_.find(window);
-  CHECK(plugin_views_.end() != it);
+  DCHECK(plugin_views_.end() != it);
+  if (plugin_views_.end() == it) {
+    return;
+  }
   [it->second removeFromSuperview];
   plugin_views_.erase(it);
-
   plugin_container_manager_.DestroyFakePluginWindowHandle(window);
 }
 
@@ -768,9 +774,6 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceSetIOSurface(
     int32 width,
     int32 height,
     uint64 io_surface_identifier) {
-  PluginViewMap::iterator it = plugin_views_.find(window);
-  CHECK(plugin_views_.end() != it);
-
   plugin_container_manager_.SetSizeAndIOSurface(window,
                                                 width,
                                                 height,
@@ -795,9 +798,6 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceSetTransportDIB(
     int32 width,
     int32 height,
     TransportDIB::Handle transport_dib) {
-  PluginViewMap::iterator it = plugin_views_.find(window);
-  CHECK(plugin_views_.end() != it);
-
   plugin_container_manager_.SetSizeAndTransportDIB(window,
                                                    width,
                                                    height,
@@ -807,10 +807,14 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceSetTransportDIB(
 void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
     gfx::PluginWindowHandle window) {
   PluginViewMap::iterator it = plugin_views_.find(window);
-  CHECK(plugin_views_.end() != it);
-  CHECK([it->second isKindOfClass:[AcceleratedPluginView class]]);
+  DCHECK(plugin_views_.end() != it);
+  if (plugin_views_.end() == it) {
+    return;
+  }
+  DCHECK([it->second isKindOfClass:[AcceleratedPluginView class]]);
 
-  AcceleratedPluginView* view = static_cast<AcceleratedPluginView*>(it->second);
+  AcceleratedPluginView* view =
+      static_cast<AcceleratedPluginView*>(it->second);
   [view setHidden:NO];
   [view setSurfaceWasSwapped:YES];
 }
@@ -819,8 +823,10 @@ void RenderWidgetHostViewMac::DrawAcceleratedSurfaceInstance(
       CGLContextObj context, gfx::PluginWindowHandle plugin_handle) {
   // Called on the display link thread.
   PluginViewMap::iterator it = plugin_views_.find(plugin_handle);
-  CHECK(plugin_views_.end() != it);
-
+  DCHECK(plugin_views_.end() != it);
+  if (plugin_views_.end() == it) {
+    return;
+  }
   CGLSetCurrentContext(context);
   // TODO(thakis): Pixel or view coordinates?
   NSSize size = [it->second frame].size;
@@ -834,7 +840,9 @@ void RenderWidgetHostViewMac::DrawAcceleratedSurfaceInstance(
   glLoadIdentity();
 
   plugin_container_manager_.Draw(
-      context, plugin_handle, GetRenderWidgetHost()->is_gpu_rendering_active());
+      context,
+      plugin_handle,
+      GetRenderWidgetHost()->is_gpu_rendering_active());
 }
 
 void RenderWidgetHostViewMac::ForceTextureReload() {
@@ -1340,11 +1348,6 @@ void RenderWidgetHostViewMac::SetTextInputActive(bool active) {
   }
 
   if (renderWidgetHostView_->render_widget_host_->is_gpu_rendering_active()) {
-    // In this mode the accelerated plugin layer is considered to be
-    // opaque. We do not want its contents to be blended with anything
-    // underneath it.
-    acceleratedPluginLayer_.get().opaque = YES;
-    [acceleratedPluginLayer_.get() setNeedsDisplay];
     return;
   }
 
@@ -1461,15 +1464,6 @@ void RenderWidgetHostViewMac::SetTextInputActive(bool active) {
     if (renderWidgetHostView_->whiteout_start_time_.is_null())
       renderWidgetHostView_->whiteout_start_time_ = base::TimeTicks::Now();
   }
-
-  // If we get here then the accelerated plugin layer is not supposed
-  // to be considered opaque -- plugins overlay the browser's normal
-  // painting.
-  acceleratedPluginLayer_.get().opaque = NO;
-
-  // This helps keep accelerated plugins' output in better sync with the
-  // window as it resizes.
-  [acceleratedPluginLayer_.get() setNeedsDisplay];
 }
 
 - (BOOL)canBecomeKeyView {
@@ -2139,15 +2133,15 @@ extern NSString *NSTextInputReplacementRangeAttributeName;
 }
 
 - (void)viewDidMoveToWindow {
-  // If we move into a new window, refresh the frame information. We don't need
-  // to do it if it was the same window as it used to be in, since that case
-  // is covered by DidBecomeSelected.
-  // We only want to do this for real browser views, not popups.
   if (canBeKeyView_) {
     NSWindow* newWindow = [self window];
     // Pointer comparison only, since we don't know if lastWindow_ is still
     // valid.
     if (newWindow) {
+      // If we move into a new window, refresh the frame information. We
+      // don't need to do it if it was the same window as it used to be in,
+      // since that case is covered by DidBecomeSelected. We only want to
+      // do this for real browser views, not popups.
       if (newWindow != lastWindow_) {
         lastWindow_ = newWindow;
         renderWidgetHostView_->WindowFrameChanged();
@@ -2218,10 +2212,6 @@ extern NSString *NSTextInputReplacementRangeAttributeName;
   }
 }
 
-- (void)drawAcceleratedPluginLayer {
-  [acceleratedPluginLayer_.get() setNeedsDisplay];
-}
-
 - (void)cancelComposition {
   if (!hasMarkedText_)
     return;