Cleanups to well-behaved accelerated plugin CL requested by...

chrome/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2010 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 <QuartzCore/QuartzCore.h>
 
 #include "chrome/browser/renderer_host/render_widget_host_view_mac.h"
 
 #include "app/surface/io_surface_support_mac.h"
 #import "base/chrome_application_mac.h"
@@ skipping 155 matching lines @@
 
   if (![self surfaceWasSwapped])
     return kCVReturnSuccess;
 
   [self drawView];
   [self setSurfaceWasSwapped:NO];
   return kCVReturnSuccess;
 }
 
 // This is the renderer output callback function
-static CVReturn MyDisplayLinkCallback(
+static CVReturn DrawOneAcceleratedPluginCallback(
     CVDisplayLinkRef displayLink,
     const CVTimeStamp* now,
     const CVTimeStamp* outputTime,
     CVOptionFlags flagsIn,
     CVOptionFlags* flagsOut,
     void* displayLinkContext) {
   CVReturn result =
       [(AcceleratedPluginView*)displayLinkContext getFrameForTime:outputTime];
   return result;
 }
@@ skipping 17 matching lines @@
     cglContext_ = (CGLContextObj)[glContext_ CGLContextObj];
     cglPixelFormat_ = (CGLPixelFormatObj)[glPixelFormat_ CGLPixelFormatObj];
 
     // Draw at beam vsync.
     GLint swapInterval = 1;
     [glContext_ setValues:&swapInterval forParameter:NSOpenGLCPSwapInterval];
 
 
     // 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_);
   }
   return self;
 }
 
 - (void)drawView {
   // Called on a background thread. Synchronized via the CGL context lock.
   CGLLockContext(cglContext_);
@@ skipping 201 matching lines @@
 
       gfx::Rect rect = geom.window_rect;
       if (geom.visible) {
         rect.set_x(rect.x() + geom.clip_rect.x());
         rect.set_y(rect.y() + geom.clip_rect.y());
         rect.set_width(geom.clip_rect.width());
         rect.set_height(geom.clip_rect.height());
       }
 
       PluginViewMap::iterator it = plugin_views_.find(geom.window);
-      CHECK(plugin_views_.end() != it);
+      DCHECK(plugin_views_.end() != it);

[Nico, 2010/08/19 22:24:22]

what's the rationale for replacing these CHECKs with DCHECKs and early returns?
If this is ever hit, this is a serious problem.

See
http://www.chromium.org/developers/coding-style#TOC-CHECK-vs-DCHECK-and-NOTRE...

[Ken Russell (switch to Gerrit), 2010/08/19 22:28:47]

Stuart was concerned about the use of CHECK. I could go either way. There might
be slight race conditions during page shutdown that we might want to handle more
gracefully. From my standpoint, feel free to strengthen these if stress testing
indicates that CHECKs don't crash.

[stuartmorgan, 2010/08/19 22:35:59]

From that document:

"Sometimes it is preferable to force a crash to happen via CHECK. Those cases
include situations where you are dealing with untrusted input (provided there is
no reasonable recovery path) and the consequences of continuing execution could
result in a security vulnerability."

What is the security vulnerability of just ignoring an attempt to move a plugin
that doesn't exist? I can't see what the problem is here that is so bad that
crashing the user's browser is our most graceful option.

I'm fine with DLOG instead of DCHECK, but either seems far, far better than
CHECK.

+      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);
     }
   }
 }
 
 void RenderWidgetHostViewMac::Focus() {
   [[cocoa_view_ window] makeFirstResponder:cocoa_view_];
 }
 
 void RenderWidgetHostViewMac::Blur() {
   [[cocoa_view_ window] makeFirstResponder:nil];
@@ skipping 130 matching lines @@
   // outside of themselves. On Windows, if Destroy is being called on a view,
   // then the event causing the destroy had also cancelled any popups by the
   // time Destroy() was called. On the Mac we have to destroy all the popups
   // ourselves.
 
   if (!is_popup_menu_) {
     // Depth-first destroy all popups. Use ShutdownHost() to enforce
     // 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();
     }
 
     // We've been told to destroy.
     [cocoa_view_ retain];
     [cocoa_view_ removeFromSuperview];
     [cocoa_view_ autorelease];
   } else {
@@ skipping 144 matching lines @@
 
   [cocoa_view_ addSubview:plugin_view];
   plugin_views_[handle] = plugin_view;
 
   return handle;
 }
 
 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);
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceSetIOSurface(
     gfx::PluginWindowHandle window,
     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,
                                                 io_surface_identifier);
 
   if (plugin_container_manager_.IsRootContainer(window)) {
     // Fake up a WebPluginGeometry for the root window to set the
     // container's size; we will never get a notification from the
     // browser about the root window, only plugins.
     webkit_glue::WebPluginGeometry geom;
     gfx::Rect rect(0, 0, width, height);
     geom.window = window;
     geom.window_rect = rect;
     geom.clip_rect = rect;
     geom.visible = true;
     MovePluginWindows(std::vector<webkit_glue::WebPluginGeometry>(1, geom));
   }
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceSetTransportDIB(
     gfx::PluginWindowHandle window,
     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,
                                                    transport_dib);
 }
 
 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];
 }
 
 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;
 
   glMatrixMode(GL_PROJECTION);
   glLoadIdentity();
   // Note that we place the origin at the upper left corner with +y
   // going down
   glOrtho(0, size.width, size.height, 0, -1, 1);
   glMatrixMode(GL_MODELVIEW);
   glLoadIdentity();
 
   plugin_container_manager_.Draw(
-      context, plugin_handle, GetRenderWidgetHost()->is_gpu_rendering_active());
+      context,
+      plugin_handle,
+      GetRenderWidgetHost()->is_gpu_rendering_active());
 }
 
 void RenderWidgetHostViewMac::ForceTextureReload() {
   plugin_container_manager_.ForceTextureReload();
 }
 
 void RenderWidgetHostViewMac::SetVisuallyDeemphasized(bool deemphasized) {
   // Mac uses tab-modal sheets, so this is a no-op.
 }
 
@@ skipping 485 matching lines @@
 
 - (void)drawRect:(NSRect)dirtyRect {
   if (!renderWidgetHostView_->render_widget_host_) {
     // TODO(shess): Consider using something more noticable?
     [[NSColor whiteColor] set];
     NSRectFill(dirtyRect);
     return;
   }
 
   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;
   }
 
   DCHECK(
       renderWidgetHostView_->render_widget_host_->process()->HasConnection());
   DCHECK(!renderWidgetHostView_->about_to_validate_and_paint_);
 
   renderWidgetHostView_->about_to_validate_and_paint_ = true;
   BackingStoreMac* backing_store = static_cast<BackingStoreMac*>(
       renderWidgetHostView_->render_widget_host_->GetBackingStore(true));
@@ skipping 96 matching lines @@
       // Reset tab_switch_paint_time_ to 0 so future tab selections are
       // recorded.
       renderWidgetHostView_->tab_switch_paint_time_ = base::TimeTicks();
     }
   } else {
     [[NSColor whiteColor] set];
     NSRectFill(dirtyRect);
     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 {
   if (!renderWidgetHostView_->render_widget_host_)
     return NO;
 
   return canBeKeyView_;
 }
 
 - (BOOL)acceptsFirstResponder {
@@ skipping 649 matching lines @@
   } else {
     renderWidgetHostView_->render_widget_host_->ImeConfirmComposition(
         base::SysNSStringToUTF16(im_text));
   }
 
   // Inserting text will delete all marked text automatically.
   hasMarkedText_ = NO;
 }
 
 - (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();
       }
       renderWidgetHostView_->ForceTextureReload();
     }
   }
 
   // If we switch windows (or are removed from the view hierarchy), cancel any
   // open mouse-downs.
@@ skipping 50 matching lines @@
   }
 }
 
 - (void)pasteAsPlainText:(id)sender {
   if (renderWidgetHostView_->render_widget_host_->IsRenderView()) {
     static_cast<RenderViewHost*>(renderWidgetHostView_->render_widget_host_)->
       ForwardEditCommand("PasteAndMatchStyle", "");
   }
 }
 
-- (void)drawAcceleratedPluginLayer {
-  [acceleratedPluginLayer_.get() setNeedsDisplay];
-}
-
 - (void)cancelComposition {
   if (!hasMarkedText_)
     return;
 
   // Cancel the ongoing composition. [NSInputManager markedTextAbandoned:]
   // doesn't call any NSTextInput functions, such as setMarkedText or
   // insertText. So, we need to send an IPC message to a renderer so it can
   // delete the composition node.
   NSInputManager *currentInputManager = [NSInputManager currentInputManager];
   [currentInputManager markedTextAbandoned:self];
@@ skipping 11 matching lines @@
     renderWidgetHostView_->render_widget_host_->ImeConfirmComposition();
 
   [self cancelComposition];
 }
 
 - (ViewID)viewID {
   return VIEW_ID_TAB_CONTAINER_FOCUS_VIEW;
 }
 
 @end