Revert 165814 - Aura: fix failing DCHECK(!swap_posted_) in ui::Compositor

Failing on CrOS: [6351:6351:1102/185319:675054389:FATAL:compositor.cc(463)] Check failed: posted_swaps_->AreSwapsPosted(). 
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/11961

Fix the DCHECKs so that we can have an arbitrary number of Compositor::ReadPixels originating swaps in flight, but at most one Compositor::Draw based swap in flight.

BUG=158415
TEST=by hand as per repro on bug


Review URL: https://chromiumcodereview.appspot.com/11366005

TBR=backer@chromium.org
Review URL: https://codereview.chromium.org/11293083

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

2 files changed, 21 insertions, 118 deletions

diff --git a/ui/compositor/compositor.cc b/ui/compositor/compositor.cc
index e1b8bc1..e41128a 100644
--- a/ui/compositor/compositor.cc
+++ b/ui/compositor/compositor.cc
@@ -5,7 +5,6 @@
 #include "ui/compositor/compositor.h"
 
 #include <algorithm>
-#include <deque>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -37,11 +36,6 @@ namespace {
 const double kDefaultRefreshRate = 60.0;
 const double kTestRefreshRate = 100.0;
 
-enum SwapType {
-  DRAW_SWAP,
-  READPIXELS_SWAP,
-};
-
 webkit_glue::WebThreadImpl* g_compositor_thread = NULL;
 
 bool test_compositor_enabled = false;
@@ -51,9 +45,9 @@ ui::ContextFactory* g_context_factory = NULL;
 const int kCompositorLockTimeoutMs = 67;
 
 // Adapts a pure WebGraphicsContext3D into a WebCompositorOutputSurface.
-class WebGraphicsContextToOutputSurfaceAdapter
-    : public WebKit::WebCompositorOutputSurface {
- public:
+class WebGraphicsContextToOutputSurfaceAdapter :
+    public WebKit::WebCompositorOutputSurface {
+public:
   explicit WebGraphicsContextToOutputSurfaceAdapter(
       WebKit::WebGraphicsContext3D* context)
       : context3D_(context),
@@ -81,30 +75,12 @@ class WebGraphicsContextToOutputSurfaceAdapter
       const WebKit::WebCompositorFrame&) OVERRIDE {
   }
 
- private:
+private:
   scoped_ptr<WebKit::WebGraphicsContext3D> context3D_;
   Capabilities capabilities_;
   WebKit::WebCompositorOutputSurfaceClient* client_;
 };
 
-class PendingSwap {
- public:
-  PendingSwap(SwapType type, ui::PostedSwapQueue* posted_swaps);
-  ~PendingSwap();
-
-  SwapType type() const { return type_; }
-  bool posted() const { return posted_; }
-
- private:
-  friend class ui::PostedSwapQueue;
-
-  SwapType type_;
-  bool posted_;
-  ui::PostedSwapQueue* posted_swaps_;
-
-  DISALLOW_COPY_AND_ASSIGN(PendingSwap);
-};
-
 }  // namespace
 
 namespace ui {
@@ -217,78 +193,12 @@ void CompositorLock::CancelLock() {
   compositor_ = NULL;
 }
 
-class PostedSwapQueue {
- public:
-  PostedSwapQueue() : pending_swap_(NULL) {
-  }
-
-  ~PostedSwapQueue() {
-    DCHECK(!pending_swap_);
-  }
-
-  SwapType NextPostedSwap() const {
-    return queue_.front();
-  }
-
-  bool AreSwapsPosted() const {
-    return !queue_.empty();
-  }
-
-  int NumSwapsPosted(SwapType type) const {
-    int count = 0;
-    for (std::deque<SwapType>::const_iterator it = queue_.begin();
-         it != queue_.end(); ++it) {
-      if (*it == type)
-        count++;
-    }
-    return count;
-  }
-
-  void PostSwap() {
-    DCHECK(pending_swap_);
-    queue_.push_back(pending_swap_->type());
-    pending_swap_->posted_ = true;
-  }
-
-  void EndSwap() {
-    queue_.pop_front();
-  }
-
- private:
-  friend class ::PendingSwap;
-
-  PendingSwap* pending_swap_;
-  std::deque<SwapType> queue_;
-
-  DISALLOW_COPY_AND_ASSIGN(PostedSwapQueue);
-};
-
-}  // namespace ui
-
-namespace {
-
-PendingSwap::PendingSwap(SwapType type, ui::PostedSwapQueue* posted_swaps)
-    : type_(type), posted_(false), posted_swaps_(posted_swaps) {
-  // Only one pending swap in flight.
-  DCHECK_EQ(static_cast<PendingSwap*>(NULL), posted_swaps_->pending_swap_);
-  posted_swaps_->pending_swap_ = this;
-}
-
-PendingSwap::~PendingSwap() {
-  DCHECK_EQ(this, posted_swaps_->pending_swap_);
-  posted_swaps_->pending_swap_ = NULL;
-}
-
-}  // namespace
-
-namespace ui {
-
 Compositor::Compositor(CompositorDelegate* delegate,
                        gfx::AcceleratedWidget widget)
     : delegate_(delegate),
       root_layer_(NULL),
       widget_(widget),
-      posted_swaps_(new PostedSwapQueue()),
+      swap_posted_(false),
       device_scale_factor_(0.0f),
       last_started_frame_(0),
       last_ended_frame_(0),
@@ -381,20 +291,17 @@ void Compositor::SetHostHasTransparentBackground(
 }
 
 void Compositor::Draw(bool force_clear) {
-  DCHECK(!g_compositor_thread);
-
   if (!root_layer_)
     return;
 
   last_started_frame_++;
-  PendingSwap pending_swap(DRAW_SWAP, posted_swaps_.get());
-  if (!IsLocked()) {
+  if (!g_compositor_thread && !IsLocked()) {
     // TODO(nduca): Temporary while compositor calls
     // compositeImmediately() directly.
     layout();
     host_->composite();
   }
-  if (!pending_swap.posted())
+  if (!g_compositor_thread && !swap_posted_)
     NotifyEnd();
 }
 
@@ -413,7 +320,6 @@ bool Compositor::ReadPixels(SkBitmap* bitmap,
   SkAutoLockPixels lock_image(*bitmap);
   unsigned char* pixels = static_cast<unsigned char*>(bitmap->getPixels());
   CancelCompositorLock();
-  PendingSwap pending_swap(READPIXELS_SWAP, posted_swaps_.get());
   return host_->compositeAndReadback(pixels, bounds_in_pixel);
 }
 
@@ -445,26 +351,20 @@ bool Compositor::HasObserver(CompositorObserver* observer) {
 }
 
 void Compositor::OnSwapBuffersPosted() {
-  DCHECK(!g_compositor_thread);
-  posted_swaps_->PostSwap();
+  swap_posted_ = true;
 }
 
 void Compositor::OnSwapBuffersComplete() {
-  DCHECK(!g_compositor_thread);
-  DCHECK(posted_swaps_->AreSwapsPosted());
-  DCHECK_GE(1, posted_swaps_->NumSwapsPosted(DRAW_SWAP));
-  if (posted_swaps_->NextPostedSwap() == DRAW_SWAP)
-    NotifyEnd();
-  posted_swaps_->EndSwap();
+  DCHECK(swap_posted_);
+  swap_posted_ = false;
+  NotifyEnd();
 }
 
 void Compositor::OnSwapBuffersAborted() {
-  DCHECK(!g_compositor_thread);
-  DCHECK(posted_swaps_->AreSwapsPosted());
-  if (posted_swaps_->NextPostedSwap() == DRAW_SWAP)
+  if (swap_posted_) {
+    swap_posted_ = false;
     NotifyEnd();
-  posted_swaps_->EndSwap();
-
+  }
   FOR_EACH_OBSERVER(CompositorObserver,
                     observer_list_,
                     OnCompositingAborted(this));
@@ -514,7 +414,6 @@ void Compositor::didCommitAndDrawFrame() {
 }
 
 void Compositor::didCompleteSwapBuffers() {
-  DCHECK(g_compositor_thread);
   NotifyEnd();
 }
 
diff --git a/ui/compositor/compositor.h b/ui/compositor/compositor.h
index 10f8efc..6ab4f9f 100644
--- a/ui/compositor/compositor.h
+++ b/ui/compositor/compositor.h
@@ -33,7 +33,6 @@ namespace ui {
 class Compositor;
 class CompositorObserver;
 class Layer;
-class PostedSwapQueue;
 
 // This class abstracts the creation of the 3D context for the compositor. It is
 // a global object.
@@ -219,6 +218,10 @@ class COMPOSITOR_EXPORT Compositor
   void RemoveObserver(CompositorObserver* observer);
   bool HasObserver(CompositorObserver* observer);
 
+  // Returns whether a draw is pending, that is, if we're between the Draw call
+  // and the OnCompositingEnded.
+  bool DrawPending() const { return swap_posted_; }
+
   // Creates a compositor lock. Returns NULL if it is not possible to lock at
   // this time (i.e. we're waiting to complete a previous unlock).
   scoped_refptr<CompositorLock> GetCompositorLock();
@@ -277,8 +280,9 @@ class COMPOSITOR_EXPORT Compositor
   scoped_ptr<WebKit::WebLayer> root_web_layer_;
   scoped_ptr<WebKit::WebLayerTreeView> host_;
 
-  // Used to verify that we have at most one draw swap in flight.
-  scoped_ptr<PostedSwapQueue> posted_swaps_;
+  // This is set to true when the swap buffers has been posted and we're waiting
+  // for completion.
+  bool swap_posted_;
 
   // The device scale factor of the monitor that this compositor is compositing
   // layers on.