Add acknowledgement messages for PluginMsg_UpdateGeometry. On the Mac, use

these acknowledgements to know when it's safe to dump old TransportDIBs in the
renderer process.  The Mac TransportDIB implementation uses
base::SharedMemory, which cannot be disposed of if an in-flight UpdateGeometry
message refers to the shared memory file descriptor.

BUG=27510, 26754
TEST=1. From bug 25710:
         a. Visit http://www.dkmsoftware.com/Yubotu.htm
         b. Click "Play Now"
         c. Resize vigorously.
        Expect: no sad plug-in icon.
     2. Test case from bug 26754 (affected machines only):
         a. Visit http://news.google.com/
         b. Click the [+] to the left of a YouTube link.  On affected
            machines, you'll get a sad plug-in icon.
         c. Click the [+] to the left of a different YouTube link.
        Expect: no crash.
     3. Test case from bug 26754 comment 9 (affected machines only):
         a. Have lots of bookmarks (import Safari defaults)
         b. Right-click on bookmark bar, and choose "Open All Bookmarks"
        Expect: no crash.
        This change may not actually fix the third test case.
Review URL: http://codereview.chromium.org/417005

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

7 files changed, 151 insertions, 13 deletions

diff --git a/chrome/common/plugin_messages.h b/chrome/common/plugin_messages.h
index c4c70fb..0bfddee 100644
--- a/chrome/common/plugin_messages.h
+++ b/chrome/common/plugin_messages.h
@@ -105,6 +105,14 @@ struct PluginMsg_UpdateGeometry_Param {
   gfx::Rect clip_rect;
   TransportDIB::Handle windowless_buffer;
   TransportDIB::Handle background_buffer;
+
+#if defined(OS_MACOSX)
+  // This field contains a key that the plug-in process is expected to return
+  // to the renderer in its ACK message, unless the value is -1, in which case
+  // no ACK message is required.  Other than the special -1 value, the values
+  // used in ack_key are opaque to the plug-in process.
+  int ack_key;
+#endif
 };
 
 
@@ -418,13 +426,21 @@ struct ParamTraits<PluginMsg_UpdateGeometry_Param> {
     WriteParam(m, p.clip_rect);
     WriteParam(m, p.windowless_buffer);
     WriteParam(m, p.background_buffer);
+#if defined(OS_MACOSX)
+    WriteParam(m, p.ack_key);
+#endif
   }
   static bool Read(const Message* m, void** iter, param_type* r) {
     return
       ReadParam(m, iter, &r->window_rect) &&
       ReadParam(m, iter, &r->clip_rect) &&
       ReadParam(m, iter, &r->windowless_buffer) &&
-      ReadParam(m, iter, &r->background_buffer);
+      ReadParam(m, iter, &r->background_buffer)
+#if defined(OS_MACOSX)
+      &&
+      ReadParam(m, iter, &r->ack_key)
+#endif
+      ;
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(L"(");
@@ -435,6 +451,10 @@ struct ParamTraits<PluginMsg_UpdateGeometry_Param> {
     LogParam(p.windowless_buffer, l);
     l->append(L", ");
     LogParam(p.background_buffer, l);
+#if defined(OS_MACOSX)
+    l->append(L", ");
+    LogParam(p.ack_key, l);
+#endif
     l->append(L")");
   }
 };
diff --git a/chrome/common/plugin_messages_internal.h b/chrome/common/plugin_messages_internal.h
index 0cfbc9f..48c3bcf 100644
--- a/chrome/common/plugin_messages_internal.h
+++ b/chrome/common/plugin_messages_internal.h
@@ -372,6 +372,11 @@ IPC_BEGIN_MESSAGES(PluginHost)
   IPC_SYNC_MESSAGE_CONTROL1_0(PluginHostMsg_SetException,
                               std::string /* message */)
 
+#if defined(OS_MACOSX)
+  IPC_MESSAGE_ROUTED1(PluginHostMsg_UpdateGeometry_ACK,
+                      int /* ack_key */)
+#endif
+
 IPC_END_MESSAGES(PluginHost)
 
 //-----------------------------------------------------------------------------
diff --git a/chrome/plugin/webplugin_delegate_stub.cc b/chrome/plugin/webplugin_delegate_stub.cc
index 6807b7b..d0fe483 100644
--- a/chrome/plugin/webplugin_delegate_stub.cc
+++ b/chrome/plugin/webplugin_delegate_stub.cc
@@ -294,7 +294,12 @@ void WebPluginDelegateStub::OnUpdateGeometry(
     const PluginMsg_UpdateGeometry_Param& param) {
   webplugin_->UpdateGeometry(
       param.window_rect, param.clip_rect,
-      param.windowless_buffer, param.background_buffer);
+      param.windowless_buffer, param.background_buffer
+#if defined(OS_MACOSX)
+      ,
+      param.ack_key
+#endif
+      );
 }
 
 void WebPluginDelegateStub::OnGetPluginScriptableObject(int* route_id,
diff --git a/chrome/plugin/webplugin_proxy.cc b/chrome/plugin/webplugin_proxy.cc
index b6a6973..9f75444 100644
--- a/chrome/plugin/webplugin_proxy.cc
+++ b/chrome/plugin/webplugin_proxy.cc
@@ -441,7 +441,12 @@ void WebPluginProxy::UpdateGeometry(
     const gfx::Rect& window_rect,
     const gfx::Rect& clip_rect,
     const TransportDIB::Handle& windowless_buffer,
-    const TransportDIB::Handle& background_buffer) {
+    const TransportDIB::Handle& background_buffer
+#if defined(OS_MACOSX)
+    ,
+    int ack_key
+#endif
+    ) {
   gfx::Rect old = delegate_->GetRect();
   gfx::Rect old_clip_rect = delegate_->GetClipRect();
 
@@ -457,6 +462,13 @@ void WebPluginProxy::UpdateGeometry(
       old_clip_rect.IsEmpty() && !damaged_rect_.IsEmpty()) {
     InvalidateRect(damaged_rect_);
   }
+
+#if defined(OS_MACOSX)
+  // The renderer is expecting an ACK message if ack_key is not -1.
+  if (ack_key != -1) {
+    Send(new PluginHostMsg_UpdateGeometry_ACK(route_id_, ack_key));
+  }
+#endif
 }
 
 #if defined(OS_WIN)
diff --git a/chrome/plugin/webplugin_proxy.h b/chrome/plugin/webplugin_proxy.h
index 7b283f6..fabf5d0 100644
--- a/chrome/plugin/webplugin_proxy.h
+++ b/chrome/plugin/webplugin_proxy.h
@@ -111,7 +111,12 @@ class WebPluginProxy : public webkit_glue::WebPlugin {
   void UpdateGeometry(const gfx::Rect& window_rect,
                       const gfx::Rect& clip_rect,
                       const TransportDIB::Handle& windowless_buffer,
-                      const TransportDIB::Handle& background_buffer);
+                      const TransportDIB::Handle& background_buffer
+#if defined(OS_MACOSX)
+                      ,
+                      int ack_key
+#endif
+                      );
 
   void CancelDocumentLoad();
 
diff --git a/chrome/renderer/webplugin_delegate_proxy.cc b/chrome/renderer/webplugin_delegate_proxy.cc
index e2c27a6..823080c 100644
--- a/chrome/renderer/webplugin_delegate_proxy.cc
+++ b/chrome/renderer/webplugin_delegate_proxy.cc
@@ -381,6 +381,12 @@ void WebPluginDelegateProxy::OnMessageReceived(const IPC::Message& msg) {
                         OnInitiateHTTPRangeRequest)
     IPC_MESSAGE_HANDLER(PluginHostMsg_DeferResourceLoading,
                         OnDeferResourceLoading)
+
+#if defined(OS_MACOSX)
+    IPC_MESSAGE_HANDLER(PluginHostMsg_UpdateGeometry_ACK,
+                        OnUpdateGeometry_ACK)
+#endif
+
     IPC_MESSAGE_UNHANDLED_ERROR()
   IPC_END_MESSAGE_MAP()
 }
@@ -403,11 +409,30 @@ void WebPluginDelegateProxy::UpdateGeometry(const gfx::Rect& window_rect,
 
   bool bitmaps_changed = false;
 
+  PluginMsg_UpdateGeometry_Param param;
+#if defined(OS_MACOSX)
+  param.ack_key = -1;
+#endif
+
   if (windowless_) {
     if (!backing_store_canvas_.get() ||
         (window_rect.width() != backing_store_canvas_->getDevice()->width() ||
-         window_rect.height() != backing_store_canvas_->getDevice()->height())) {
+         window_rect.height() != backing_store_canvas_->getDevice()->height()))
+        {
       bitmaps_changed = true;
+
+#if defined(OS_MACOSX)
+      if (backing_store_.get()) {
+        // ResetWindowlessBitmaps inserts the old TransportDIBs into
+        // old_transport_dibs_ using the backing store's file descriptor as
+        // the key.  The constraints on the keys are that -1 is reserved
+        // to mean "no ACK required," and in-flight keys must be unique.
+        // File descriptors will never be -1, and because they won't be closed
+        // until receipt of the ACK, they're unique.
+        param.ack_key = backing_store_->handle().fd;
+      }
+#endif
+
       // Create a shared memory section that the plugin paints into
       // asynchronously.
       ResetWindowlessBitmaps();
@@ -424,7 +449,6 @@ void WebPluginDelegateProxy::UpdateGeometry(const gfx::Rect& window_rect,
     }
   }
 
-  PluginMsg_UpdateGeometry_Param param;
   param.window_rect = window_rect;
   param.clip_rect = clip_rect;
   param.windowless_buffer = TransportDIB::DefaultHandleValue();
@@ -446,7 +470,6 @@ void WebPluginDelegateProxy::UpdateGeometry(const gfx::Rect& window_rect,
 
   IPC::Message* msg;
 #if defined (OS_WIN)
-  std::wstring filename = StringToLowerASCII(info_.path.BaseName().value());
   if (info_.name.find(L"Windows Media Player") != std::wstring::npos) {
     // Need to update geometry synchronously with WMP, otherwise if a site
     // scripts the plugin to start playing while it's in the middle of handling
@@ -474,17 +497,37 @@ static void ReleaseTransportDIB(TransportDIB *dib) {
 
 void WebPluginDelegateProxy::ResetWindowlessBitmaps() {
 #if defined(OS_MACOSX)
-  // tell the browser to relase these TransportDIBs
-  ReleaseTransportDIB(backing_store_.get());
-  ReleaseTransportDIB(transport_store_.get());
-  ReleaseTransportDIB(background_store_.get());
-#endif
+  if (backing_store_.get()) {
+    int ack_key = backing_store_->handle().fd;
+
+    DCHECK_NE(ack_key, -1);
+
+    // DCHECK_EQ does not work with base::hash_map.
+    DCHECK(old_transport_dibs_.find(ack_key) == old_transport_dibs_.end());
 
+    // Stash the old TransportDIBs in the map.  They'll be released when an
+    // ACK message comes in.
+    RelatedTransportDIBs old_dibs;
+    old_dibs.backing_store =
+        linked_ptr<TransportDIB>(backing_store_.release());
+    old_dibs.transport_store =
+        linked_ptr<TransportDIB>(transport_store_.release());
+    old_dibs.background_store =
+        linked_ptr<TransportDIB>(background_store_.release());
+
+    old_transport_dibs_[ack_key] = old_dibs;
+  } else {
+    DCHECK(!transport_store_.get());
+    DCHECK(!background_store_.get());
+  }
+#else
   backing_store_.reset();
   transport_store_.reset();
+  background_store_.reset();
+#endif
+
   backing_store_canvas_.reset();
   transport_store_canvas_.reset();
-  background_store_.reset();
   background_store_canvas_.release();
   backing_store_painted_ = gfx::Rect();
 }
@@ -1109,3 +1152,24 @@ void WebPluginDelegateProxy::OnDeferResourceLoading(int resource_id,
                                                     bool defer) {
   plugin_->SetDeferResourceLoading(resource_id, defer);
 }
+
+#if defined(OS_MACOSX)
+void WebPluginDelegateProxy::OnUpdateGeometry_ACK(int ack_key) {
+  DCHECK_NE(ack_key, -1);
+
+  OldTransportDIBMap::iterator iterator = old_transport_dibs_.find(ack_key);
+
+  // DCHECK_NE does not work with base::hash_map.
+  DCHECK(iterator != old_transport_dibs_.end());
+
+  // Now that the ACK has been received, the TransportDIBs that were used
+  // prior to the UpdateGeometry message now being acknowledged are known to
+  // be no longer needed.  Release them, and take the stale entry out of the
+  // map.
+  ReleaseTransportDIB(iterator->second.backing_store.get());
+  ReleaseTransportDIB(iterator->second.transport_store.get());
+  ReleaseTransportDIB(iterator->second.background_store.get());
+
+  old_transport_dibs_.erase(iterator);
+}
+#endif
diff --git a/chrome/renderer/webplugin_delegate_proxy.h b/chrome/renderer/webplugin_delegate_proxy.h
index d9cb3b3..02dd644 100644
--- a/chrome/renderer/webplugin_delegate_proxy.h
+++ b/chrome/renderer/webplugin_delegate_proxy.h
@@ -22,6 +22,11 @@
 #include "webkit/glue/webplugininfo.h"
 #include "webkit/glue/webplugin_delegate.h"
 
+#if defined(OS_MACOSX)
+#include "base/hash_tables.h"
+#include "base/linked_ptr.h"
+#endif
+
 struct NPObject;
 class NPObjectStub;
 struct NPVariant_Param;
@@ -133,6 +138,10 @@ class WebPluginDelegateProxy
                                   intptr_t notify_data);
   void OnDeferResourceLoading(int resource_id, bool defer);
 
+#if defined(OS_MACOSX)
+  void OnUpdateGeometry_ACK(int ack_key);
+#endif
+
   // Draw a graphic indicating a crashed plugin.
   void PaintSadPlugin(WebKit::WebCanvas* canvas, const gfx::Rect& rect);
 
@@ -156,6 +165,24 @@ class WebPluginDelegateProxy
   // point the window has already been destroyed).
   void WillDestroyWindow();
 
+#if defined(OS_MACOSX)
+  // The Mac TransportDIB implementation uses base::SharedMemory, which
+  // cannot be disposed of if an in-flight UpdateGeometry message refers to
+  // the shared memory file descriptor.  The old_transport_dibs_ map holds
+  // old TransportDIBs waiting to die.  It's keyed by the |ack_key| values
+  // used in UpdateGeometry messages.  When an UpdateGeometry_ACK message
+  // arrives, the associated RelatedTransportDIBs can be released.
+  struct RelatedTransportDIBs {
+    linked_ptr<TransportDIB> backing_store;
+    linked_ptr<TransportDIB> transport_store;
+    linked_ptr<TransportDIB> background_store;
+  };
+
+  typedef base::hash_map<int, RelatedTransportDIBs> OldTransportDIBMap;
+
+  OldTransportDIBMap old_transport_dibs_;
+#endif  // OS_MACOSX
+
   base::WeakPtr<RenderView> render_view_;
   webkit_glue::WebPlugin* plugin_;
   bool windowless_;