Don't copy the Graphics2D when binding a new one. This brings the

implementation in line with the interface documentation (the old behavior would
clobber any content).

The copying behavior was introduced to avoid flickering when resizing. To avoid
this, the PaintManager now doesn't bind the new device until it's been painted
to. This gives flicker-free painting.

This fixes a bug in the paint manager example that made it not paint.

TEST=manual
BUG=86568
Review URL: http://codereview.chromium.org/7215030

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

2 files changed, 83 insertions, 43 deletions

diff --git a/ppapi/cpp/paint_manager.cc b/ppapi/cpp/paint_manager.cc
index 1508333..bc02016 100644
--- a/ppapi/cpp/paint_manager.cc
+++ b/ppapi/cpp/paint_manager.cc
@@ -17,7 +17,8 @@ PaintManager::PaintManager()
       is_always_opaque_(false),
       callback_factory_(NULL),
       manual_callback_pending_(false),
-      flush_pending_(false) {
+      flush_pending_(false),
+      has_pending_resize_(false) {
   // Set the callback object outside of the initializer list to avoid a
   // compiler warning about using "this" in an initializer list.
   callback_factory_.Initialize(this);
@@ -31,7 +32,8 @@ PaintManager::PaintManager(Instance* instance,
       is_always_opaque_(is_always_opaque),
       callback_factory_(NULL),
       manual_callback_pending_(false),
-      flush_pending_(false) {
+      flush_pending_(false),
+      has_pending_resize_(false) {
   // Set the callback object outside of the initializer list to avoid a
   // compiler warning about using "this" in an initializer list.
   callback_factory_.Initialize(this);
@@ -53,35 +55,29 @@ void PaintManager::Initialize(Instance* instance,
 }
 
 void PaintManager::SetSize(const Size& new_size) {
-  if (new_size == graphics_.size())
+  if (GetEffectiveSize() == new_size)
     return;
 
-  graphics_ = Graphics2D(instance_, new_size, is_always_opaque_);
-  if (graphics_.is_null())
-    return;
-  instance_->BindGraphics(graphics_);
-
-  manual_callback_pending_ = false;
-  flush_pending_ = false;
-  callback_factory_.CancelAll();
+  has_pending_resize_ = true;
+  pending_size_ = new_size;
 
   Invalidate();
 }
 
 void PaintManager::Invalidate() {
-  // You must call SetDevice before using.
-  PP_DCHECK(!graphics_.is_null());
+  // You must call SetSize before using.
+  PP_DCHECK(!graphics_.is_null() || has_pending_resize_);
 
   EnsureCallbackPending();
-  aggregator_.InvalidateRect(Rect(graphics_.size()));
+  aggregator_.InvalidateRect(Rect(GetEffectiveSize()));
 }
 
 void PaintManager::InvalidateRect(const Rect& rect) {
-  // You must call SetDevice before using.
-  PP_DCHECK(!graphics_.is_null());
+  // You must call SetSize before using.
+  PP_DCHECK(!graphics_.is_null() || has_pending_resize_);
 
   // Clip the rect to the device area.
-  Rect clipped_rect = rect.Intersect(Rect(graphics_.size()));
+  Rect clipped_rect = rect.Intersect(Rect(GetEffectiveSize()));
   if (clipped_rect.IsEmpty())
     return;  // Nothing to do.
 
@@ -90,13 +86,17 @@ void PaintManager::InvalidateRect(const Rect& rect) {
 }
 
 void PaintManager::ScrollRect(const Rect& clip_rect, const Point& amount) {
-  // You must call SetDevice before using.
-  PP_DCHECK(!graphics_.is_null());
+  // You must call SetSize before using.
+  PP_DCHECK(!graphics_.is_null() || has_pending_resize_);
 
   EnsureCallbackPending();
   aggregator_.ScrollRect(clip_rect, amount);
 }
 
+Size PaintManager::GetEffectiveSize() const {
+  return has_pending_resize_ ? pending_size_ : graphics_.size();
+}
+
 void PaintManager::EnsureCallbackPending() {
   // The best way for us to do the next update is to get a notification that
   // a previous one has completed. So if we're already waiting for one, we
@@ -125,32 +125,56 @@ void PaintManager::DoPaint() {
   PaintAggregator::PaintUpdate update = aggregator_.GetPendingUpdate();
   aggregator_.ClearPendingUpdate();
 
+  // Apply any pending resize. Setting the graphics to this class must happen
+  // before asking the plugin to paint in case it requests the Graphics2D during
+  // painting. However, the bind must not happen until afterward since we don't
+  // want to have an unpainted device bound. The needs_binding flag tells us
+  // whether to do this later.
+  bool needs_binding = false;
+  if (has_pending_resize_) {
+    graphics_ = Graphics2D(instance_, pending_size_, is_always_opaque_);
+    needs_binding = true;
+
+    // Since we're binding a new one, all of the callbacks have been canceled.
+    manual_callback_pending_ = false;
+    flush_pending_ = false;
+    callback_factory_.CancelAll();
+
+    // This must be cleared before calling into the plugin since it may do
+    // additional invalidation or sizing operations.
+    has_pending_resize_ = false;
+    pending_size_ = Size();
+  }
+
   // Apply any scroll before asking the client to paint.
   if (update.has_scroll)
     graphics_.Scroll(update.scroll_rect, update.scroll_delta);
 
-  if (!client_->OnPaint(graphics_, update.paint_rects, update.paint_bounds))
-    return;  // Nothing was painted, don't schedule a flush.
-
-  int32_t result = graphics_.Flush(
-      callback_factory_.NewCallback(&PaintManager::OnFlushComplete));
-
-  // If you trigger this assertion, then your plugin has called Flush()
-  // manually. When using the PaintManager, you should not call Flush, it will
-  // handle that for you because it needs to know when it can do the next paint
-  // by implementing the flush callback.
-  //
-  // Another possible cause of this assertion is re-using devices. If you
-  // use one device, swap it with another, then swap it back, we won't know
-  // that we've already scheduled a Flush on the first device. It's best to not
-  // re-use devices in this way.
-  PP_DCHECK(result != PP_ERROR_INPROGRESS);
-
-  if (result == PP_OK_COMPLETIONPENDING) {
-    flush_pending_ = true;
-  } else {
-    PP_DCHECK(result == PP_OK);  // Catch all other errors in debug mode.
+  if (client_->OnPaint(graphics_, update.paint_rects, update.paint_bounds)) {
+    // Something was painted, schedule a flush.
+    int32_t result = graphics_.Flush(
+        callback_factory_.NewCallback(&PaintManager::OnFlushComplete));
+
+    // If you trigger this assertion, then your plugin has called Flush()
+    // manually. When using the PaintManager, you should not call Flush, it
+    // will handle that for you because it needs to know when it can do the
+    // next paint by implementing the flush callback.
+    //
+    // Another possible cause of this assertion is re-using devices. If you
+    // use one device, swap it with another, then swap it back, we won't know
+    // that we've already scheduled a Flush on the first device. It's best to
+    // not re-use devices in this way.
+    PP_DCHECK(result != PP_ERROR_INPROGRESS);
+
+    if (result == PP_OK_COMPLETIONPENDING) {
+      flush_pending_ = true;
+    } else {
+      PP_DCHECK(result == PP_OK);  // Catch all other errors in debug mode.
+    }
   }
+
+  if (needs_binding)
+    instance_->BindGraphics(graphics_);
 }
 
 void PaintManager::OnFlushComplete(int32_t) {
@@ -175,4 +199,5 @@ void PaintManager::OnManualCallbackComplete(int32_t) {
     DoPaint();
 }
 
+
 }  // namespace pp
diff --git a/ppapi/cpp/paint_manager.h b/ppapi/cpp/paint_manager.h
index d8cba66..5f14862 100644
--- a/ppapi/cpp/paint_manager.h
+++ b/ppapi/cpp/paint_manager.h
@@ -146,9 +146,12 @@ class PaintManager {
   // position changed).
   void SetSize(const Size& new_size);
 
-  // Provides access to the underlying device in case you need it. Note: if
-  // you call Flush on this device the paint manager will get very confused,
-  // don't do this!
+  // Provides access to the underlying device in case you need it. If you have
+  // done a SetSize, note that the graphics context won't be updated until
+  // right before the next OnPaint call.
+  //
+  // Note: if you call Flush on this device the paint manager will get very
+  // confused, don't do this!
   const Graphics2D& graphics() const { return graphics_; }
   Graphics2D& graphics() { return graphics_; }
 
@@ -161,6 +164,12 @@ class PaintManager {
   // The given rect should be scrolled by the given amounts.
   void ScrollRect(const Rect& clip_rect, const Point& amount);
 
+  // Returns the size of the graphics context for the next paint operation.
+  // This is the pending size if a resize is pending (the plugin has called
+  // SetSize but we haven't actually painted it yet), or the current size of
+  // no resize is pending.
+  Size GetEffectiveSize() const;
+
  private:
   // Disallow copy and assign (these are unimplemented).
   PaintManager(const PaintManager&);
@@ -200,6 +209,12 @@ class PaintManager {
   // See comment for EnsureCallbackPending for more on how these work.
   bool manual_callback_pending_;
   bool flush_pending_;
+
+  // When we get a resize, we don't bind right away (see SetSize). The
+  // has_pending_resize_ tells us that we need to do a resize for the next
+  // paint operation. When true, the new size is in pending_size_.
+  bool has_pending_resize_;
+  Size pending_size_;
 };
 
 }  // namespace pp