Make plugin buffer changes robust against synchronous plugin call nesting

Makes SetWindowlessBuffer take a rect argument so it doesn't depend on the delegate being updated, then makes UpdateGeometry call that first so that changes can't accidentally be handled in reverse order if calling into the plugin's SetWindow results in nesting.

Also combines updating the plugin geometry and context on the Mac into one call, since we don't want the plugin to have mismatched context and size information. Unblacklists Move Networks player on the Mac since these changes fix its crash.

BUG=28298
TEST=Open the Move Networks sample on the Mac, and resize it. It should play without crashing.

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

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

5 files changed, 49 insertions, 39 deletions

diff --git a/chrome/plugin/webplugin_proxy.cc b/chrome/plugin/webplugin_proxy.cc
index 95f6545..6326065 100644
--- a/chrome/plugin/webplugin_proxy.cc
+++ b/chrome/plugin/webplugin_proxy.cc
@@ -436,12 +436,21 @@ void WebPluginProxy::UpdateGeometry(
   gfx::Rect old = delegate_->GetRect();
   gfx::Rect old_clip_rect = delegate_->GetClipRect();
 
-  delegate_->UpdateGeometry(window_rect, clip_rect);
+  // Update the buffers before doing anything that could call into plugin code,
+  // so that we don't process buffer changes out of order if plugins make
+  // synchronous calls that lead to nested UpdateGeometry calls.
   if (TransportDIB::is_valid(windowless_buffer)) {
     // The plugin's rect changed, so now we have a new buffer to draw into.
-    SetWindowlessBuffer(windowless_buffer, background_buffer);
+    SetWindowlessBuffer(windowless_buffer, background_buffer, window_rect);
   }
 
+#if defined(OS_MACOSX)
+  delegate_->UpdateGeometryAndContext(window_rect, clip_rect,
+                                      windowless_context_);
+#else
+  delegate_->UpdateGeometry(window_rect, clip_rect);
+#endif
+
   // Send over any pending invalidates which occured when the plugin was
   // off screen.
   if (delegate_->IsWindowless() && !clip_rect.IsEmpty() &&
@@ -460,14 +469,15 @@ void WebPluginProxy::UpdateGeometry(
 #if defined(OS_WIN)
 void WebPluginProxy::SetWindowlessBuffer(
     const TransportDIB::Handle& windowless_buffer,
-    const TransportDIB::Handle& background_buffer) {
+    const TransportDIB::Handle& background_buffer,
+    const gfx::Rect& window_rect) {
   // Create a canvas that will reference the shared bits. We have to handle
   // errors here since we're mapping a large amount of memory that may not fit
   // in our address space, or go wrong in some other way.
   windowless_canvas_.reset(new skia::PlatformCanvas);
   if (!windowless_canvas_->initialize(
-          delegate_->GetRect().width(),
-          delegate_->GetRect().height(),
+          window_rect.width(),
+          window_rect.height(),
           true,
           win_util::GetSectionFromProcess(windowless_buffer,
               channel_->renderer_handle(), false))) {
@@ -479,8 +489,8 @@ void WebPluginProxy::SetWindowlessBuffer(
   if (background_buffer) {
     background_canvas_.reset(new skia::PlatformCanvas);
     if (!background_canvas_->initialize(
-            delegate_->GetRect().width(),
-            delegate_->GetRect().height(),
+            window_rect.width(),
+            window_rect.height(),
             true,
             win_util::GetSectionFromProcess(background_buffer,
                 channel_->renderer_handle(), false))) {
@@ -495,46 +505,44 @@ void WebPluginProxy::SetWindowlessBuffer(
 
 void WebPluginProxy::SetWindowlessBuffer(
     const TransportDIB::Handle& windowless_buffer,
-    const TransportDIB::Handle& background_buffer) {
+    const TransportDIB::Handle& background_buffer,
+    const gfx::Rect& window_rect) {
   // Convert the shared memory handle to a handle that works in our process,
   // and then use that to create a CGContextRef.
   windowless_dib_.reset(TransportDIB::Map(windowless_buffer));
   background_dib_.reset(TransportDIB::Map(background_buffer));
   windowless_context_.reset(CGBitmapContextCreate(
       windowless_dib_->memory(),
-      delegate_->GetRect().width(),
-      delegate_->GetRect().height(),
-      8, 4 * delegate_->GetRect().width(),
+      window_rect.width(),
+      window_rect.height(),
+      8, 4 * window_rect.width(),
       mac_util::GetSystemColorSpace(),
       kCGImageAlphaPremultipliedFirst |
       kCGBitmapByteOrder32Host));
-  CGContextTranslateCTM(windowless_context_, 0, delegate_->GetRect().height());
+  CGContextTranslateCTM(windowless_context_, 0, window_rect.height());
   CGContextScaleCTM(windowless_context_, 1, -1);
   if (background_dib_.get()) {
     background_context_.reset(CGBitmapContextCreate(
         background_dib_->memory(),
-        delegate_->GetRect().width(),
-        delegate_->GetRect().height(),
-        8, 4 * delegate_->GetRect().width(),
+        window_rect.width(),
+        window_rect.height(),
+        8, 4 * window_rect.width(),
         mac_util::GetSystemColorSpace(),
         kCGImageAlphaPremultipliedFirst |
         kCGBitmapByteOrder32Host));
-    CGContextTranslateCTM(background_context_, 0,
-                          delegate_->GetRect().height());
+    CGContextTranslateCTM(background_context_, 0, window_rect.height());
     CGContextScaleCTM(background_context_, 1, -1);
   }
-
-  static_cast<WebPluginDelegateImpl*>(delegate_)->UpdateContext(
-      windowless_context_);
 }
 
 #elif defined(OS_LINUX)
 
 void WebPluginProxy::SetWindowlessBuffer(
     const TransportDIB::Handle& windowless_buffer,
-    const TransportDIB::Handle& background_buffer) {
-  int width = delegate_->GetRect().width();
-  int height = delegate_->GetRect().height();
+    const TransportDIB::Handle& background_buffer,
+    const gfx::Rect& window_rect) {
+  int width = window_rect.width();
+  int height = window_rect.height();
   windowless_dib_.reset(TransportDIB::Map(windowless_buffer));
   if (windowless_dib_.get()) {
     windowless_canvas_.reset(windowless_dib_->GetPlatformCanvas(width, height));
diff --git a/chrome/plugin/webplugin_proxy.h b/chrome/plugin/webplugin_proxy.h
index da8f420..16a6593 100644
--- a/chrome/plugin/webplugin_proxy.h
+++ b/chrome/plugin/webplugin_proxy.h
@@ -140,7 +140,8 @@ class WebPluginProxy : public webkit_glue::WebPlugin {
 
   // Updates the shared memory section where windowless plugins paint.
   void SetWindowlessBuffer(const TransportDIB::Handle& windowless_buffer,
-                           const TransportDIB::Handle& background_buffer);
+                           const TransportDIB::Handle& background_buffer,
+                           const gfx::Rect& window_rect);
 
   typedef base::hash_map<int, webkit_glue::WebPluginResourceClient*>
       ResourceClientMap;
diff --git a/webkit/glue/plugins/plugin_list_mac.mm b/webkit/glue/plugins/plugin_list_mac.mm
index af3cc04..fee65b5 100644
--- a/webkit/glue/plugins/plugin_list_mac.mm
+++ b/webkit/glue/plugins/plugin_list_mac.mm
@@ -84,7 +84,6 @@ bool PluginList::ShouldLoadPlugin(const WebPluginInfo& info,
   // Plugins that we know don't work at all.
   const char* blacklisted_plugin_mimes[] = {
     "application/x-googlegears",          // Safari-specific.
-    "application/x-vnd.movenetworks.qm",  // Crashes on Snow Leopard.
     "application/vnd.o3d.auto",           // Doesn't render, and having it
                                           // detected can prevent fallbacks.
     "video/divx",                         // Crashes on 10.5.
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.h b/webkit/glue/plugins/webplugin_delegate_impl.h
index 520da4c..1327464 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.h
+++ b/webkit/glue/plugins/webplugin_delegate_impl.h
@@ -112,9 +112,11 @@ class WebPluginDelegateImpl : public webkit_glue::WebPluginDelegate {
   int GetQuirks() const { return quirks_; }
 
 #if defined(OS_MACOSX)
-  // Informs the delegate that the context used for painting windowless plugins
-  // has changed.
-  void UpdateContext(gfx::NativeDrawingContext context);
+  // Informs the plugin that the geometry has changed, as with UpdateGeometry,
+  // but also includes the new buffer context for that new geometry.
+  void UpdateGeometryAndContext(const gfx::Rect& window_rect,
+                                const gfx::Rect& clip_rect,
+                                gfx::NativeDrawingContext context);
   // Returns the delegate currently processing events.
   static WebPluginDelegateImpl* GetActiveDelegate();
   // Returns a vector of currently active delegates in this process.
diff --git a/webkit/glue/plugins/webplugin_delegate_impl_mac.mm b/webkit/glue/plugins/webplugin_delegate_impl_mac.mm
index fc2bf31..0a1e84e 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl_mac.mm
+++ b/webkit/glue/plugins/webplugin_delegate_impl_mac.mm
@@ -288,22 +288,22 @@ void WebPluginDelegateImpl::PlatformDestroyInstance() {
 #endif
 }
 
-void WebPluginDelegateImpl::UpdateContext(CGContextRef context) {
+void WebPluginDelegateImpl::UpdateGeometryAndContext(
+    const gfx::Rect& window_rect, const gfx::Rect& clip_rect,
+    CGContextRef context) {
   buffer_context_ = context;
 #ifndef NP_NO_CARBON
-  // Under the Carbon event model, CoreGraphics plugins can cache the context
-  // they are given in NPP_SetWindow (and at least Flash does), and continue to
-  // use it even when the contents of the struct have changed, so we need to
-  // call NPP_SetWindow again if the context changes.
-  // In the Cocoa event model plugins are only given a context during paint
-  // events, so we don't have to tell the plugin anything here.
-  if (instance()->event_model() == NPEventModelCarbon &&
-      instance()->drawing_model() == NPDrawingModelCoreGraphics &&
-      context != np_cg_context_.context) {
+  if (instance()->event_model() == NPEventModelCarbon) {
+    // Update the structure that is passed to Carbon+CoreGraphics plugins in
+    // NPP_SetWindow before calling UpdateGeometry, since that will trigger an
+    // NPP_SetWindow call if the geometry changes (which is the only time the
+    // context would be different), and some plugins (e.g., Flash) have an
+    // internal cache of the context that they only update when NPP_SetWindow
+    // is called.
     np_cg_context_.context = context;
-    WindowlessSetWindow(true);
   }
 #endif
+  UpdateGeometry(window_rect, clip_rect);
 }
 
 void WebPluginDelegateImpl::Paint(CGContextRef context, const gfx::Rect& rect) {