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/WebKit/Source/Platform/chromium/public/Platform.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebCompositorSupport.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebFloatPoint.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebCompositorOutputSurface.h"
 #include "ui/compositor/compositor_observer.h"
 #include "ui/compositor/compositor_switches.h"
 #include "ui/compositor/dip_util.h"
 #include "ui/compositor/layer.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 72 matching lines @@
 
 Texture::Texture(bool flipped, const gfx::Size& size, float device_scale_factor)
     : flipped_(flipped),
       size_(size),
       device_scale_factor_(device_scale_factor) {
 }
 
 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),
       swap_posted_(false),
       device_scale_factor_(0.0f),
       last_started_frame_(0),
       last_ended_frame_(0),
-      disable_schedule_composite_(false) {
+      disable_schedule_composite_(false),
+      compositor_lock_(NULL) {
   WebKit::WebCompositorSupport* compositor_support =
       WebKit::Platform::current()->compositorSupport();
   root_web_layer_.reset(compositor_support->createLayer());
   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;
 
   root_web_layer_->setAnchorPoint(WebKit::WebFloatPoint(0.f, 0.f));
   host_.reset(compositor_support->createLayerTreeView(this, *root_web_layer_,
                                                       settings));
   host_->setSurfaceReady();
 }
 
 Compositor::~Compositor() {
+  if (compositor_lock_) {
+    compositor_lock_->CancelLock();
+    DCHECK(!compositor_lock_);
+  }
+
   // Don't call |CompositorDelegate::ScheduleDraw| from this point.
   delegate_ = NULL;
   if (root_layer_)
     root_layer_->SetCompositor(NULL);
 
   // Stop all outstanding draws before telling the ContextFactory to tear
   // down any contexts that the |host_| may rely upon.
   host_.reset();
 
   if (!test_compositor_enabled)
@@ skipping 49 matching lines @@
 void Compositor::SetHostHasTransparentBackground(
     bool host_has_transparent_background) {
   host_->setHasTransparentBackground(host_has_transparent_background);
 }
 
 void Compositor::Draw(bool force_clear) {
   if (!root_layer_)
     return;
 
   last_started_frame_++;
-  if (!g_compositor_thread)
-    FOR_EACH_OBSERVER(CompositorObserver,
-                      observer_list_,
-                      OnCompositingWillStart(this));
-
-  // TODO(nduca): Temporary while compositor calls
-  // compositeImmediately() directly.
-  layout();
-  host_->composite();
+  if (!g_compositor_thread && !IsLocked()) {
+    // 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) {
   if (bounds_in_pixel.right() > size().width() ||
       bounds_in_pixel.bottom() > size().height())
     return false;
   bitmap->setConfig(SkBitmap::kARGB_8888_Config,
                     bounds_in_pixel.width(), bounds_in_pixel.height());
   bitmap->allocPixels();
   SkAutoLockPixels lock_image(*bitmap);
   unsigned char* pixels = static_cast<unsigned char*>(bitmap->getPixels());
+  Unlock();

[piman, 2012/10/25 00:14:08]

I think this should really be:
if (compositor_lock_)
  compositor_lock_->CancelLock();

Several reasons:
- calling Unlock will result in a DCHECK if it wasn't locked
- the CompositorLock object would still be live, but we wouldn't reset its back
pointer on Compositor destruction, possibly resulting in a use-after-free.

Maybe putting this into a (private?) Compositor::CancelLock wrapper, and calling
that from Compositor::~Compositor too.

[jonathan.backer, 2012/10/25 15:19:04]

Done.

   return host_->compositeAndReadback(pixels, bounds_in_pixel);
 }
 
 void Compositor::SetScaleAndSize(float scale, const gfx::Size& size_in_pixel) {
   DCHECK_GT(scale, 0);
   if (size_in_pixel.IsEmpty() || scale <= 0)
     return;
   size_ = size_in_pixel;
   host_->setViewportSize(size_in_pixel);
   root_web_layer_->setBounds(size_in_pixel);
@@ skipping 10 matching lines @@
 }
 
 void Compositor::RemoveObserver(CompositorObserver* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 bool Compositor::HasObserver(CompositorObserver* observer) {
   return observer_list_.HasObserver(observer);
 }
 
-bool Compositor::IsThreaded() const {
-  return g_compositor_thread != NULL;
-}
-
 void Compositor::OnSwapBuffersPosted() {
   swap_posted_ = true;
 }
 
 void Compositor::OnSwapBuffersComplete() {
   DCHECK(swap_posted_);
   swap_posted_ = false;
   NotifyEnd();
 }
 
@@ skipping 73 matching lines @@
     return new WebGraphicsContextToOutputSurfaceAdapter(test_context);
   } else {
     return new WebGraphicsContextToOutputSurfaceAdapter(
         ContextFactory::GetInstance()->CreateContext(this));
   }
 }
 
 void Compositor::didRecreateOutputSurface(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 (!IsLocked()) {

[piman, 2012/10/25 00:14:08]

Is there actually a case where we'll call didCommit with IsLocked() == true?
Can we assert instead?

[jonathan.backer, 2012/10/25 15:19:04]

You are right. This is should no longer happen. I'm going to leave a DCHECK here
to cover the single threaded case (we are covered by DCHECKs in cc::ThreadProxy
for forced commits).

+    FOR_EACH_OBSERVER(CompositorObserver,
+                      observer_list_,
+                      OnCompositingDidCommit(this));
+  }
 }
 
 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 (g_compositor_thread)
+      host_->setDeferCommits(true);
+    FOR_EACH_OBSERVER(CompositorObserver,
+                      observer_list_,
+                      OnCompositingLockStateChanged(this));
+  }
+  return compositor_lock_;
+}
+
+void Compositor::Unlock() {
+  DCHECK(compositor_lock_);
+  compositor_lock_ = NULL;
+  if (g_compositor_thread)
+    host_->setDeferCommits(false);
+  FOR_EACH_OBSERVER(CompositorObserver,
+                    observer_list_,
+                    OnCompositingLockStateChanged(this));
+}
+
 void Compositor::NotifyEnd() {
   last_ended_frame_++;
   FOR_EACH_OBSERVER(CompositorObserver,
                     observer_list_,
                     OnCompositingEnded(this));
 }
 
 COMPOSITOR_EXPORT void SetupTestCompositor() {
   if (!CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableTestCompositor)) {
     test_compositor_enabled = true;
   }
 #if defined(OS_CHROMEOS)
   // If the test is running on the chromeos envrionment (such as
   // device or vm bots), use the real compositor.
   if (base::chromeos::IsRunningOnChromeOS())
     test_compositor_enabled = false;
 #endif
 }
 
 COMPOSITOR_EXPORT void DisableTestCompositor() {
   test_compositor_enabled = false;
 }
 
 COMPOSITOR_EXPORT bool IsTestCompositorEnabled() {
   return test_compositor_enabled;
 }
 
 }  // namespace ui