Fixes a flash plugin hang caused by opening google finance ticker symbols in a background tab.

From what I can tell this bug always existed in Chrome.

The flash plugin gets instantiated and gets initial geometry updates during initialization. We download the plugin url after the geometry update. After these geometry updates the webkit layout timer runs and the plugin gets additional geometry updates. However these don't get sent over to the flash plugin instance in the plugin process as the geometry updates for windowed plugins are only flushed during paint, which does not happen in this case. The flash plugin ends up receiving data before geometry update and ends up behaving in a wierd fashion, i.e. not peeking for messages, etc.

The fact that this is a windowed plugin results in the browser ui thread also getting hung until the plugin gets out of this state.

The fix for the geometry update issue is to remove the deferred geometry update stuff. This only exists
for windowed plugins anyway. The geometry update IPC is a plain routed message and it should not be a big
overhead to send these IPCs to the plugin process.

I found that while this change fixes the basic issue, we still see some hangs in the flash plugin because of it receiving
data before the valid geometry update kicks in. John is working on a fix where we never have to call setFrameRect ourselves
and always honor the setFrameRect calls made by Webkit. This issue can be marked as fixed once both fixes get checked in.

This fixes http://code.google.com/p/chromium/issues/detail?id=12993

Bug=12993

TEST=Open google finance and Ctrl Click on the tickers in the page like Basic Materials, etc.

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

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

6 files changed, 26 insertions, 64 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 4e0ec16..7882860 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2507,14 +2507,6 @@ void RenderView::OnSetAltErrorPageURL(const GURL& url) {
   alternate_error_page_url_ = url;
 }
 
-void RenderView::DidPaint() {
-  PluginDelegateList::iterator it = plugin_delegates_.begin();
-  while (it != plugin_delegates_.end()) {
-    (*it)->FlushGeometryUpdates();
-    ++it;
-  }
-}
-
 void RenderView::OnInstallMissingPlugin() {
   // This could happen when the first default plugin is deleted.
   if (first_default_plugin_ == NULL)
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index bd216ae..926ea72 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -604,9 +604,6 @@ class RenderView : public RenderWidget,
   // UI that is going to be hosted by this RenderView.
   void CreateDevToolsClient();
 
-  // Called by RenderWidget after it paints.
-  virtual void DidPaint();
-
   // Locates a sub frame with given xpath
   WebFrame* GetChildFrame(const std::wstring& frame_xpath) const;
 
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 25afdbb..5332d74 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -336,9 +336,6 @@ void RenderWidget::PaintRect(const gfx::Rect& rect,
 
   // Flush to underlying bitmap.  TODO(darin): is this needed?
   canvas->getTopPlatformDevice().accessBitmap(false);
-
-  // Let the subclass observe this paint operations.
-  DidPaint();
 }
 
 void RenderWidget::DoDeferredPaint() {
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index a337f54..440a7e2 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -123,10 +123,6 @@ class RenderWidget : public IPC::Channel::Listener,
   void DoDeferredClose();
   void DoDeferredSetWindowRect(const WebKit::WebRect& pos);
 
-  // This method is called immediately after PaintRect but before the
-  // corresponding paint or scroll message is send to the widget host.
-  virtual void DidPaint() {}
-
   // Set the background of the render widget to a bitmap. The bitmap will be
   // tiled in both directions if it isn't big enough to fill the area. This is
   // mainly intended to be used in conjuction with WebView::SetIsTransparent().
diff --git a/chrome/renderer/webplugin_delegate_proxy.cc b/chrome/renderer/webplugin_delegate_proxy.cc
index 5b16ce6..b00687c 100644
--- a/chrome/renderer/webplugin_delegate_proxy.cc
+++ b/chrome/renderer/webplugin_delegate_proxy.cc
@@ -165,7 +165,6 @@ WebPluginDelegateProxy::WebPluginDelegateProxy(const std::string& mime_type,
       windowless_(false),
       mime_type_(mime_type),
       clsid_(clsid),
-      send_deferred_update_geometry_(false),
       npobject_(NULL),
       window_script_object_(NULL),
       sad_plugin_(NULL),
@@ -204,17 +203,6 @@ void WebPluginDelegateProxy::PluginDestroyed() {
   MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
-void WebPluginDelegateProxy::FlushGeometryUpdates() {
-  if (send_deferred_update_geometry_) {
-    send_deferred_update_geometry_ = false;
-    Send(new PluginMsg_UpdateGeometry(instance_id_,
-                                      plugin_rect_,
-                                      deferred_clip_rect_,
-                                      TransportDIB::Id(),
-                                      TransportDIB::Id()));
-  }
-}
-
 bool WebPluginDelegateProxy::Initialize(const GURL& url, char** argn,
                                         char** argv, int argc,
                                         WebPlugin* plugin,
@@ -372,46 +360,43 @@ void WebPluginDelegateProxy::UpdateGeometry(
                 const gfx::Rect& window_rect,
                 const gfx::Rect& clip_rect) {
   plugin_rect_ = window_rect;
-  if (!windowless_) {
-    deferred_clip_rect_ = clip_rect;
-    send_deferred_update_geometry_ = true;
-    return;
-  }
 
   // Be careful to explicitly call the default constructors for these ids,
   // as they can be POD on some platforms and we want them initialized.
   TransportDIB::Id transport_store_id = TransportDIB::Id();
   TransportDIB::Id background_store_id = TransportDIB::Id();
 
+  if (windowless_) {
 #if defined(OS_WIN)
-  // TODO(port): use TransportDIB instead of allocating these directly.
-  if (!backing_store_canvas_.get() ||
-      (window_rect.width() != backing_store_canvas_->getDevice()->width() ||
-       window_rect.height() != backing_store_canvas_->getDevice()->height())) {
-    // Create a shared memory section that the plugin paints into
-    // asynchronously.
-    ResetWindowlessBitmaps();
-    if (!window_rect.IsEmpty()) {
-      if (!CreateBitmap(&backing_store_, &backing_store_canvas_) ||
-          !CreateBitmap(&transport_store_, &transport_store_canvas_) ||
-          (transparent_ &&
-           !CreateBitmap(&background_store_, &background_store_canvas_))) {
-        DCHECK(false);
-        ResetWindowlessBitmaps();
-        return;
+    // TODO(port): use TransportDIB instead of allocating these directly.
+    if (!backing_store_canvas_.get() ||
+        (window_rect.width() != backing_store_canvas_->getDevice()->width() ||
+         window_rect.height() != backing_store_canvas_->getDevice()->height())) {
+      // Create a shared memory section that the plugin paints into
+      // asynchronously.
+      ResetWindowlessBitmaps();
+      if (!window_rect.IsEmpty()) {
+        if (!CreateBitmap(&backing_store_, &backing_store_canvas_) ||
+            !CreateBitmap(&transport_store_, &transport_store_canvas_) ||
+            (transparent_ &&
+             !CreateBitmap(&background_store_, &background_store_canvas_))) {
+          DCHECK(false);
+          ResetWindowlessBitmaps();
+          return;
+        }
+
+        // TODO(port): once we use TransportDIB we will properly fill in these
+        // ids; for now we just fill in the HANDLE field.
+        transport_store_id.handle = transport_store_->handle();
+        if (background_store_.get())
+          background_store_id.handle = background_store_->handle();
       }
-
-      // TODO(port): once we use TransportDIB we will properly fill in these
-      // ids; for now we just fill in the HANDLE field.
-      transport_store_id.handle = transport_store_->handle();
-      if (background_store_.get())
-        background_store_id.handle = background_store_->handle();
     }
-  }
 #else
-  // TODO(port): refactor our allocation of backing stores.
-  NOTIMPLEMENTED();
+    // TODO(port): refactor our allocation of backing stores.
+    NOTIMPLEMENTED();
 #endif
+  }
 
   IPC::Message* msg = new PluginMsg_UpdateGeometry(
       instance_id_, window_rect, clip_rect,
diff --git a/chrome/renderer/webplugin_delegate_proxy.h b/chrome/renderer/webplugin_delegate_proxy.h
index 257a5aaa..3b8d616 100644
--- a/chrome/renderer/webplugin_delegate_proxy.h
+++ b/chrome/renderer/webplugin_delegate_proxy.h
@@ -48,9 +48,6 @@ class WebPluginDelegateProxy : public WebPluginDelegate,
   // Called to drop our pointer to the window script object.
   void DropWindowScriptObject() { window_script_object_ = NULL; }
 
-  // Called to flush any deferred geometry changes to the plugin process.
-  virtual void FlushGeometryUpdates();
-
   // WebPluginDelegate implementation:
   virtual void PluginDestroyed();
   virtual bool Initialize(const GURL& url, char** argn, char** argv, int argc,
@@ -174,8 +171,6 @@ class WebPluginDelegateProxy : public WebPluginDelegate,
   FilePath plugin_path_;
 
   gfx::Rect plugin_rect_;
-  gfx::Rect deferred_clip_rect_;
-  bool send_deferred_update_geometry_;
 
   NPObject* npobject_;
   NPObjectStub* window_script_object_;