Aura: Resize locks with --ui-enable-threaded-compositing

ui/compositor/compositor.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/compositor/compositor.h"
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/command_line.h"
+#include "base/message_loop.h"
 #include "base/threading/thread_restrictions.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/images/SkImageEncoder.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebFloatPoint.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebSize.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebCompositor.h"
 #include "ui/compositor/compositor_observer.h"
 #include "ui/compositor/compositor_switches.h"
 #include "ui/compositor/dip_util.h"
@@ skipping 13 matching lines @@
 
 const double kDefaultRefreshRate = 60.0;
 const double kTestRefreshRate = 100.0;
 
 webkit_glue::WebThreadImpl* g_compositor_thread = NULL;
 
 bool test_compositor_enabled = false;
 
 ui::ContextFactory* g_context_factory = NULL;
 
+const int kCompositorLockTimeoutMs = 67;
+
 }  // namespace
 
 namespace ui {
 
 // static
 ContextFactory* ContextFactory::GetInstance() {
   // We leak the shared resources so that we don't race with
   // the tear down of the gl_bindings.
   if (!g_context_factory) {
     DVLOG(1) << "Using DefaultSharedResource";
@@ skipping 69 matching lines @@
 
 Texture::Texture(bool flipped, const gfx::Size& size)
     : texture_id_(0),
       flipped_(flipped),
       size_(size) {
 }
 
 Texture::~Texture() {
 }
 
+CompositorLock::CompositorLock(Compositor* compositor)
+    : compositor_(compositor) {
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&CompositorLock::CancelLock, AsWeakPtr()),
+      base::TimeDelta::FromMilliseconds(kCompositorLockTimeoutMs));
+}
+
+CompositorLock::~CompositorLock() {
+  CancelLock();
+}
+
+void CompositorLock::CancelLock() {
+  if (!compositor_)
+    return;
+  compositor_->Unlock();
+  compositor_ = NULL;
+}
+
 Compositor::Compositor(CompositorDelegate* delegate,
                        gfx::AcceleratedWidget widget)
     : delegate_(delegate),
       root_layer_(NULL),
       widget_(widget),
       root_web_layer_(WebKit::WebLayer::create()),
       swap_posted_(false),
       device_scale_factor_(0.0f),
       last_started_frame_(0),
       last_ended_frame_(0),
-      disable_schedule_composite_(false) {
+      disable_schedule_composite_(false),
+      commits_allowed_(ENABLED_NEEDS_COMMIT),
+      compositor_lock_(NULL) {
   WebKit::WebLayerTreeView::Settings settings;
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   settings.showFPSCounter =
       command_line->HasSwitch(switches::kUIShowFPSCounter);
   settings.showPlatformLayerTree =
       command_line->HasSwitch(switches::kUIShowLayerTree);
   settings.refreshRate =
       test_compositor_enabled ? kTestRefreshRate : kDefaultRefreshRate;
 
 #if !defined(WEBCOMPOSITOR_OWNS_SETTINGS)
   settings.partialSwapEnabled =
       command_line->HasSwitch(switches::kUIEnablePartialSwap);
   settings.perTilePainting =
       command_line->HasSwitch(switches::kUIEnablePerTilePainting);
 #endif
 
   host_.initialize(this, root_web_layer_, settings);
   host_.setSurfaceReady();
   root_web_layer_.setAnchorPoint(WebKit::WebFloatPoint(0.f, 0.f));
 }
 
 Compositor::~Compositor() {
+  if (compositor_lock_) {
+    compositor_lock_->CancelLock();
+    DCHECK(!compositor_lock_);
+  }
+
   // Don't call |CompositorDelegate::ScheduleDraw| from this point.
   delegate_ = NULL;
   // There's a cycle between |root_web_layer_| and |host_|, which results in
   // leaking and/or crashing. Explicitly set the root layer to NULL so the cycle
   // is broken.
   host_.setRootLayer(NULL);
   if (root_layer_)
     root_layer_->SetCompositor(NULL);
 
   // Stop all outstanding draws before telling the ContextFactory to tear
@@ skipping 48 matching lines @@
   root_web_layer_.removeAllChildren();
   if (root_layer_)
     root_web_layer_.addChild(root_layer_->web_layer());
 }
 
 void Compositor::Draw(bool force_clear) {
   if (!root_layer_)
     return;
 
   last_started_frame_++;
-  if (!g_compositor_thread)
+  if (!g_compositor_thread) {
     FOR_EACH_OBSERVER(CompositorObserver,
                       observer_list_,
                       OnCompositingWillStart(this));
+  }
 
-  // TODO(nduca): Temporary while compositor calls
-  // compositeImmediately() directly.
-  layout();
-  host_.composite();
+  // Fast track draw because visible state could not have changed.
+  if (!g_compositor_thread && commits_allowed_ == DISABLED) {
+    FOR_EACH_OBSERVER(CompositorObserver,
+                      observer_list_,
+                      OnCompositingStarted(this));
+  } else {
+    // TODO(nduca): Temporary while compositor calls
+    // compositeImmediately() directly.
+    layout();
+    host_.composite();
+  }
+
   if (!g_compositor_thread && !swap_posted_)
     NotifyEnd();
 }
 
 void Compositor::ScheduleFullDraw() {
   host_.setNeedsRedraw();
 }
 
 bool Compositor::ReadPixels(SkBitmap* bitmap,
                             const gfx::Rect& bounds_in_pixel) {
@@ skipping 92 matching lines @@
     test_context->Initialize();
     return test_context;
   } else {
     return ContextFactory::GetInstance()->CreateContext(this);
   }
 }
 
 void Compositor::didRebindGraphicsContext(bool success) {
 }
 
-// Called once per draw in single-threaded compositor mode and potentially
-// many times between draws in the multi-threaded compositor mode.
 void Compositor::didCommit() {
-  FOR_EACH_OBSERVER(CompositorObserver,
-                    observer_list_,
-                    OnCompositingDidCommit(this));
+  if (commits_allowed_ != DISABLED) {
+    FOR_EACH_OBSERVER(CompositorObserver,
+                      observer_list_,
+                      OnCompositingDidCommit(this));
+  }
+
+  if (commits_allowed_ == DISABLED_PENDING_COMMIT) {
+    commits_allowed_ = DISABLED;
+    root_web_layer_.setDeferUpdates(true);
+  }
+
+  if (commits_allowed_ == ENABLED_NEEDS_COMMIT)
+    commits_allowed_ = ENABLED_DID_COMMIT;
 }
 
 void Compositor::didCommitAndDrawFrame() {
   // TODO(backer): Plumb through an earlier impl side will start.
   if (g_compositor_thread)
     FOR_EACH_OBSERVER(CompositorObserver,
                       observer_list_,
                       OnCompositingWillStart(this));
 
   FOR_EACH_OBSERVER(CompositorObserver,
                     observer_list_,
                     OnCompositingStarted(this));
 }
 
 void Compositor::didCompleteSwapBuffers() {
   NotifyEnd();
 }
 
 void Compositor::scheduleComposite() {
   if (!disable_schedule_composite_)
     ScheduleDraw();
 }
 
+scoped_refptr<CompositorLock> Compositor::GetCompositorLock() {
+  if (!compositor_lock_) {
+    compositor_lock_ = new CompositorLock(this);
+    if (commits_allowed_ == ENABLED_NEEDS_COMMIT) {
+      commits_allowed_ = DISABLED_PENDING_COMMIT;

[piman, 2012/08/01 18:25:34]

So, I think I understand why we're doing this - the case where we unlock the
compositor (we got the right-sized buffer) and draw a frame with it, before we
lock it again for the next resize.
My worry is that by waiting on a commit (it can take a long time in the
single-thread case, if the unlock comes while we're waiting for the swap), the
next resize might "sneak in" before we have a chance to lock again, causing us
to draw a "bad" frame.
The logic in RWHVA should prevent it from happening (we block resizes until the
commit did happen), but then, it feels that this logic (don't lock again until
the next commit) also belongs there. Otherwise an unrelated RWHVA (e.g. another
window) may want to lock as well, but will silently fail to do so (at least,
until the next commit).
WDYT? I'm afraid moving the logic there will make the logic more complicated,
and maybe this is "good enough" but OTOH I'm afraid that we may have to
work-around the limitation and complexity will build up at the wrong place.

+    } else {
+      commits_allowed_ = DISABLED;
+      root_web_layer_.setDeferUpdates(true);
+    }
+  }
+  return compositor_lock_;
+}
+
+void Compositor::Unlock() {
+  DCHECK(compositor_lock_);
+  compositor_lock_ = NULL;
+
+  commits_allowed_ = ENABLED_NEEDS_COMMIT;
+  root_web_layer_.setDeferUpdates(false);
+}
+
 void Compositor::SwizzleRGBAToBGRAAndFlip(unsigned char* pixels,
                                           const gfx::Size& image_size) {
   // Swizzle from RGBA to BGRA
   size_t bitmap_size = 4 * image_size.width() * image_size.height();
   for (size_t i = 0; i < bitmap_size; i += 4)
     std::swap(pixels[i], pixels[i + 2]);
 
   // Vertical flip to transform from GL co-ords
   size_t row_size = 4 * image_size.width();
   scoped_array<unsigned char> tmp_row(new unsigned char[row_size]);
@@ skipping 32 matching lines @@
 
 COMPOSITOR_EXPORT void DisableTestCompositor() {
   test_compositor_enabled = false;
 }
 
 COMPOSITOR_EXPORT bool IsTestCompositorEnabled() {
   return test_compositor_enabled;
 }
 
 }  // namespace ui