Implement more of Flush properly. This adds support for waiting on the message

loop so we can now enforce that there is only one pending Flush at a time.
Also removes the ability to have multiple flushes pending at the same time.

TEST=covered by ppapi unit test
BUG=none
Review URL: http://codereview.chromium.org/2862002

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

5 files changed, 96 insertions, 32 deletions

diff --git a/webkit/glue/plugins/pepper_device_context_2d.cc b/webkit/glue/plugins/pepper_device_context_2d.cc
index e549f0e..9eae9dc 100644
--- a/webkit/glue/plugins/pepper_device_context_2d.cc
+++ b/webkit/glue/plugins/pepper_device_context_2d.cc
@@ -185,7 +185,8 @@ void DeviceContext2D::FlushCallbackData::Execute(PP_Resource device_context) {
 DeviceContext2D::DeviceContext2D(PluginModule* module)
     : Resource(module),
       bound_instance_(NULL),
-      flushed_any_data_(false) {
+      flushed_any_data_(false),
+      offscreen_flush_pending_(false) {
 }
 
 DeviceContext2D::~DeviceContext2D() {
@@ -292,14 +293,15 @@ bool DeviceContext2D::ReplaceContents(PP_Resource image) {
 
 bool DeviceContext2D::Flush(PPB_DeviceContext2D_FlushCallback callback,
                             void* callback_data) {
+  // Don't allow more than one pending flush at a time.
+  if (HasPendingFlush())
+    return false;
+
   // TODO(brettw) check that the current thread is not the main one and
   // implement blocking flushes in this case.
   if (!callback)
     return false;
 
-  if (queued_operations_.empty())
-    return true;  // Nothing to do.
-
   gfx::Rect changed_rect;
   for (size_t i = 0; i < queued_operations_.size(); i++) {
     QueuedOperation& operation = queued_operations_[i];
@@ -334,8 +336,8 @@ bool DeviceContext2D::Flush(PPB_DeviceContext2D_FlushCallback callback,
     visible_changed_rect = bound_instance_->clip().Intersect(changed_rect);
 
   if (bound_instance_ && !visible_changed_rect.IsEmpty()) {
-    unpainted_flush_callbacks_.push_back(FlushCallbackData(callback,
-                                                           callback_data));
+    unpainted_flush_callback_.reset(new FlushCallbackData(callback,
+                                                          callback_data));
     bound_instance_->InvalidateRect(visible_changed_rect);
   } else {
     // There's nothing visible to invalidate so just schedule the callback to
@@ -396,12 +398,14 @@ bool DeviceContext2D::BindToInstance(PluginInstance* new_instance) {
     // When the device is detached, we'll not get any more paint callbacks so
     // we need to clear the list, but we still want to issue any pending
     // callbacks to the plugin.
-    for (size_t i = 0; i < unpainted_flush_callbacks_.size(); i++)
-      ScheduleOffscreenCallback(unpainted_flush_callbacks_[i]);
-    for (size_t i = 0; i < painted_flush_callbacks_.size(); i++)
-      ScheduleOffscreenCallback(painted_flush_callbacks_[i]);
-    unpainted_flush_callbacks_.clear();
-    painted_flush_callbacks_.clear();
+    if (unpainted_flush_callback_.get()) {
+      ScheduleOffscreenCallback(*unpainted_flush_callback_.get());
+      unpainted_flush_callback_.reset();
+    }
+    if (painted_flush_callback_.get()) {
+      ScheduleOffscreenCallback(*painted_flush_callback_.get());
+      painted_flush_callback_.reset();
+    }
   } else if (flushed_any_data_) {
     // Only schedule a paint if this backing store has had any data flushed to
     // it. This is an optimization. A "normal" plugin will first allocated a
@@ -463,20 +467,25 @@ void DeviceContext2D::Paint(WebKit::WebCanvas* canvas,
 }
 
 void DeviceContext2D::ViewInitiatedPaint() {
-  // Move all "unpainted" callbacks to the painted state. See
-  // |unpainted_flush_callbacks_| in the header for more.
-  std::copy(unpainted_flush_callbacks_.begin(),
-            unpainted_flush_callbacks_.end(),
-            std::back_inserter(painted_flush_callbacks_));
-  unpainted_flush_callbacks_.clear();
+  // Move any "unpainted" callback to the painted state. See
+  // |unpainted_flush_callback_| in the header for more.
+  if (unpainted_flush_callback_.get()) {
+    DCHECK(!painted_flush_callback_.get());
+    painted_flush_callback_.swap(unpainted_flush_callback_);
+  }
 }
 
 void DeviceContext2D::ViewFlushedPaint() {
-  // Notify all "painted" callbacks. See |unpainted_flush_callbacks_| in the
+  // Notify any "painted" callback. See |unpainted_flush_callback_| in the
   // header for more.
-  for (size_t i = 0; i < painted_flush_callbacks_.size(); i++)
-    painted_flush_callbacks_[i].Execute(GetResource());
-  painted_flush_callbacks_.clear();
+  if (painted_flush_callback_.get()) {
+    // We must clear this variable before issuing the callback. It will be
+    // common for the plugin to issue another invalidate in response to a flush
+    // callback, and we don't want to think that a callback is already pending.
+    scoped_ptr<FlushCallbackData> callback;
+    callback.swap(painted_flush_callback_);
+    callback->Execute(GetResource());
+  }
 }
 
 void DeviceContext2D::ExecutePaintImageData(ImageData* image,
@@ -512,7 +521,7 @@ void DeviceContext2D::ExecutePaintImageData(ImageData* image,
 
 void DeviceContext2D::ExecuteScroll(const gfx::Rect& clip, int dx, int dy,
                                     gfx::Rect* invalidated_rect) {
-  // FIXME(brettw)
+  // TODO(brettw): implement this.
 }
 
 void DeviceContext2D::ExecuteReplaceContents(ImageData* image,
@@ -524,6 +533,8 @@ void DeviceContext2D::ExecuteReplaceContents(ImageData* image,
 
 void DeviceContext2D::ScheduleOffscreenCallback(
     const FlushCallbackData& callback) {
+  DCHECK(!HasPendingFlush());
+  offscreen_flush_pending_ = true;
   MessageLoop::current()->PostTask(
       FROM_HERE,
       NewRunnableMethod(this,
@@ -532,7 +543,18 @@ void DeviceContext2D::ScheduleOffscreenCallback(
 }
 
 void DeviceContext2D::ExecuteOffscreenCallback(FlushCallbackData data) {
+  DCHECK(offscreen_flush_pending_);
+
+  // We must clear this flag before issuing the callback. It will be
+  // common for the plugin to issue another invalidate in response to a flush
+  // callback, and we don't want to think that a callback is already pending.
+  offscreen_flush_pending_ = false;
   data.Execute(GetResource());
 }
 
+bool DeviceContext2D::HasPendingFlush() const {
+  return unpainted_flush_callback_.get() || painted_flush_callback_.get() ||
+         offscreen_flush_pending_;
+}
+
 }  // namespace pepper
diff --git a/webkit/glue/plugins/pepper_device_context_2d.h b/webkit/glue/plugins/pepper_device_context_2d.h
index bb488ab..2d5bbb3 100644
--- a/webkit/glue/plugins/pepper_device_context_2d.h
+++ b/webkit/glue/plugins/pepper_device_context_2d.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/scoped_ptr.h"
 #include "third_party/ppapi/c/ppb_device_context_2d.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebCanvas.h"
 #include "webkit/glue/plugins/pepper_resource.h"
@@ -79,7 +80,6 @@ class DeviceContext2D : public Resource {
     PPB_DeviceContext2D_FlushCallback callback_;
     void* callback_data_;
   };
-  typedef std::vector<FlushCallbackData> FlushCallbackVector;
 
   // Called internally to execute the different queued commands. The
   // parameters to these functions will have already been validated. The last
@@ -103,6 +103,9 @@ class DeviceContext2D : public Resource {
   // issues the offscreen callbacks.
   void ExecuteOffscreenCallback(FlushCallbackData data);
 
+  // Returns true if there is any type of flush callback pending.
+  bool HasPendingFlush() const;
+
   scoped_refptr<ImageData> image_data_;
 
   // Non-owning pointer to the plugin instance this device context is currently
@@ -119,20 +122,32 @@ class DeviceContext2D : public Resource {
   // This is initially false and is set to true at the first Flush() call.
   bool flushed_any_data_;
 
-  // The plugin may be constantly giving us paint messages. "Unpainted" ones
-  // are paint requests which have never been painted. These could have been
-  // done while the RenderView was already waiting for an ACK from a previous
-  // paint, so won't generate a new one yet.
+  // The plugin can give us one "Flush" at a time. This flush will either be in
+  // the "unpainted" state (in which case unpainted_flush_callback_ will be
+  // non-NULL) or painted, in which case painted_flush_callback_ will be
+  // non-NULL). There can also be an offscreen callback which is handled
+  // separately (see offscreen_callback_pending_). Only one of these three
+  // things may be set at a time to enforce the "only one pending flush at a
+  // time" constraint.
+  //
+  // "Unpainted" ones are flush requests which have never been painted. These
+  // could have been done while the RenderView was already waiting for an ACK
+  // from a previous paint, so won't generate a new one yet.
   //
-  // "Painted" ones are those paints that have been painted by RenderView, but
+  // "Painted" ones are those flushes that have been painted by RenderView, but
   // for which the ACK from the browser has not yet been received.
   //
   // When we get updates from a plugin with a callback, it is first added to
   // the unpainted callbacks. When the renderer has initiated a paint, we'll
   // move it to the painted callbacks list. When the renderer receives a flush,
   // we'll execute the callback and remove it from the list.
-  FlushCallbackVector unpainted_flush_callbacks_;
-  FlushCallbackVector painted_flush_callbacks_;
+  scoped_ptr<FlushCallbackData> unpainted_flush_callback_;
+  scoped_ptr<FlushCallbackData> painted_flush_callback_;
+
+  // When doing offscreen flushes, we issue a task that issues the callback
+  // later. This is set when one of those tasks is pending so that we can
+  // enforce the "only one pending flush at a time" constraint in the API.
+  bool offscreen_flush_pending_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceContext2D);
 };
diff --git a/webkit/glue/plugins/pepper_plugin_delegate.h b/webkit/glue/plugins/pepper_plugin_delegate.h
index 9515b17..c00a9b3 100644
--- a/webkit/glue/plugins/pepper_plugin_delegate.h
+++ b/webkit/glue/plugins/pepper_plugin_delegate.h
@@ -13,6 +13,8 @@ class PlatformCanvas;
 
 namespace pepper {
 
+class PluginInstance;
+
 // Virtual interface that the browser implements to implement features for
 // Pepper plugins.
 class PluginDelegate {
@@ -32,6 +34,14 @@ class PluginDelegate {
     virtual intptr_t GetSharedMemoryHandle() const = 0;
   };
 
+  // Indicates that the given instance has been created.
+  virtual void InstanceCreated(pepper::PluginInstance* instance) = 0;
+
+  // Indicates that the given instance is being destroyed. This is called from
+  // the destructor, so it's important that the instance is not dereferenced
+  // from this call.
+  virtual void InstanceDeleted(pepper::PluginInstance* instance) = 0;
+
   // The caller will own the pointer returned from this.
   virtual PlatformImage2D* CreateImage2D(int width, int height) = 0;
 };
diff --git a/webkit/glue/plugins/pepper_plugin_instance.cc b/webkit/glue/plugins/pepper_plugin_instance.cc
index a2f83b8..d86fffc 100644
--- a/webkit/glue/plugins/pepper_plugin_instance.cc
+++ b/webkit/glue/plugins/pepper_plugin_instance.cc
@@ -21,6 +21,7 @@
 #include "third_party/WebKit/WebKit/chromium/public/WebPluginContainer.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebRect.h"
 #include "webkit/glue/plugins/pepper_device_context_2d.h"
+#include "webkit/glue/plugins/pepper_plugin_delegate.h"
 #include "webkit/glue/plugins/pepper_plugin_module.h"
 #include "webkit/glue/plugins/pepper_resource_tracker.h"
 #include "webkit/glue/plugins/pepper_var.h"
@@ -146,9 +147,11 @@ PluginInstance::PluginInstance(PluginDelegate* delegate,
       container_(NULL) {
   DCHECK(delegate);
   module_->InstanceCreated(this);
+  delegate_->InstanceCreated(this);
 }
 
 PluginInstance::~PluginInstance() {
+  delegate_->InstanceDeleted(this);
   module_->InstanceDeleted(this);
 }
 
diff --git a/webkit/glue/plugins/pepper_plugin_module.cc b/webkit/glue/plugins/pepper_plugin_module.cc
index 99448f2..2eb63a1 100644
--- a/webkit/glue/plugins/pepper_plugin_module.cc
+++ b/webkit/glue/plugins/pepper_plugin_module.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/command_line.h"
+#include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
@@ -110,8 +111,21 @@ bool ReadImageData(PP_Resource device_context_2d,
   return context->ReadImageData(image, x, y);
 }
 
+void RunMessageLoop() {
+  bool old_state = MessageLoop::current()->NestableTasksAllowed();
+  MessageLoop::current()->SetNestableTasksAllowed(true);
+  MessageLoop::current()->Run();
+  MessageLoop::current()->SetNestableTasksAllowed(old_state);
+}
+
+void QuitMessageLoop() {
+  MessageLoop::current()->Quit();
+}
+
 const PPB_Testing testing_interface = {
-  &ReadImageData
+  &ReadImageData,
+  &RunMessageLoop,
+  &QuitMessageLoop,
 };
 
 // GetInterface ----------------------------------------------------------------