Fix CompositorResizeLock to do something.

ui/compositor/compositor.h

 // 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.
 
 #ifndef UI_COMPOSITOR_COMPOSITOR_H_
 #define UI_COMPOSITOR_COMPOSITOR_H_
 
 #include <stdint.h>
 
 #include <memory>
 #include <string>
 
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "cc/output/begin_frame_args.h"
 #include "cc/surfaces/surface_sequence.h"
 #include "cc/trees/layer_tree_host_client.h"
 #include "cc/trees/layer_tree_host_single_thread_client.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/compositor/compositor_animation_observer.h"
 #include "ui/compositor/compositor_export.h"
+#include "ui/compositor/compositor_lock.h"
 #include "ui/compositor/compositor_observer.h"
 #include "ui/compositor/layer_animator_collection.h"
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 #include "ui/gfx/native_widget_types.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ skipping 24 matching lines @@
 
 namespace ui {
 
 class Compositor;
 class CompositorVSyncManager;
 class LatencyInfo;
 class Layer;
 class Reflector;
 class ScopedAnimationDurationScaleMode;
 
-const int kCompositorLockTimeoutMs = 67;
+constexpr int kCompositorLockTimeoutMs = 67;
 
 class COMPOSITOR_EXPORT ContextFactoryObserver {
  public:
   virtual ~ContextFactoryObserver() {}
 
   // Notifies that the ContextProvider returned from
   // ui::ContextFactory::SharedMainThreadContextProvider was lost.  When this
   // is called, the old resources (e.g. shared context, GL helper) still
   // exist, but are about to be destroyed. Getting a reference to those
   // resources from the ContextFactory (e.g. through
@@ skipping 78 matching lines @@
   virtual gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() = 0;
 
   // Gets the task graph runner.
   virtual cc::TaskGraphRunner* GetTaskGraphRunner() = 0;
 
   virtual void AddObserver(ContextFactoryObserver* observer) = 0;
 
   virtual void RemoveObserver(ContextFactoryObserver* observer) = 0;
 };
 
-// This class represents a lock on the compositor, that can be used to prevent
-// commits to the compositor tree while we're waiting for an asynchronous
-// event. The typical use case is when waiting for a renderer to produce a frame
-// at the right size. The caller keeps a reference on this object, and drops the
-// reference once it desires to release the lock.
-// By default, the lock will be cancelled after a short timeout to ensure
-// responsiveness of the UI, so the compositor tree should be kept in a
-// "reasonable" state while the lock is held. If the compositor sets
-// locks to not time out, then the lock will remain in effect until destroyed.
-// Don't instantiate this class directly, use Compositor::GetCompositorLock.
-class COMPOSITOR_EXPORT CompositorLock
-    : public base::RefCounted<CompositorLock>,
-      public base::SupportsWeakPtr<CompositorLock> {
- private:
-  friend class base::RefCounted<CompositorLock>;
-  friend class Compositor;
-
-  explicit CompositorLock(Compositor* compositor);
-  ~CompositorLock();
-
-  void CancelLock();
-
-  Compositor* compositor_;
-  DISALLOW_COPY_AND_ASSIGN(CompositorLock);
-};
-
 // Compositor object to take care of GPU painting.
 // A Browser compositor object is responsible for generating the final
 // displayable form of pixels comprising a single widget's contents. It draws an
 // appropriately transformed texture for each transformed view in the widget's
 // view hierarchy.
 class COMPOSITOR_EXPORT Compositor
     : NON_EXPORTED_BASE(public cc::LayerTreeHostClient),
-      NON_EXPORTED_BASE(public cc::LayerTreeHostSingleThreadClient) {
+      NON_EXPORTED_BASE(public cc::LayerTreeHostSingleThreadClient),
+      NON_EXPORTED_BASE(public CompositorLockDelegate) {
  public:
   Compositor(const cc::FrameSinkId& frame_sink_id,
              ui::ContextFactory* context_factory,
              ui::ContextFactoryPrivate* context_factory_private,
              scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~Compositor() override;
 
   ui::ContextFactory* context_factory() { return context_factory_; }
 
   ui::ContextFactoryPrivate* context_factory_private() {
@@ skipping 104 matching lines @@
   // Compositor does not own observers. It is the responsibility of the
   // observer to remove itself when it is done observing.
   void AddObserver(CompositorObserver* observer);
   void RemoveObserver(CompositorObserver* observer);
   bool HasObserver(const CompositorObserver* observer) const;
 
   void AddAnimationObserver(CompositorAnimationObserver* observer);
   void RemoveAnimationObserver(CompositorAnimationObserver* observer);
   bool HasAnimationObserver(const CompositorAnimationObserver* observer) const;
 
-  // Change the timeout behavior for all future locks that are created. Locks
-  // should time out if there is an expectation that the compositor will be
-  // responsive.
-  void SetLocksWillTimeOut(bool locks_will_time_out) {
-    locks_will_time_out_ = locks_will_time_out;
-  }
-
   // 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();
+  // this time (i.e. we're waiting to complete a previous unlock). If the
+  // timeout is null, then no timeout is used.
+  scoped_refptr<CompositorLock> GetCompositorLock(
+      CompositorLockClient* client,

[piman, 2017/03/24 04:05:31]

2 things.
1- I assume we want the client to outlive the CompositorLock. However, because
the CompositorLock is refcounted, and arbitrary unrelated things may want to
grab one, which will extend the lifetime of the CompositorLock, unbeknownst to
|client|.
2- relatedly, the second caller that tries to grab it will get a CompositorLock
that will not call into |client|

IOW I think the client needs to be put in an ObserverList (so that we can have
more than one and so that they can remove themselves).

[danakj, 2017/03/24 14:49:26]

Thank you, good point.

[danakj, 2017/03/24 16:54:56]

I've made CompositorLock a unique_ptr instead, but the timeout is shared and
implemented in ui::Compositor. Added a test to verify this.

+      base::TimeDelta timeout =
+          base::TimeDelta::FromMilliseconds(kCompositorLockTimeoutMs));
 
   // Internal functions, called back by command-buffer contexts on swap buffer
   // events.
 
   // Signals swap has been posted.
   void OnSwapBuffersPosted();
 
   // Signals swap has completed.
   void OnSwapBuffersComplete();
 
@@ skipping 35 matching lines @@
   LayerAnimatorCollection* layer_animator_collection() {
     return &layer_animator_collection_;
   }
 
   const cc::FrameSinkId& frame_sink_id() const { return frame_sink_id_; }
   int committed_frame_number() const { return committed_frame_number_; }
   float refresh_rate() const { return refresh_rate_; }
 
  private:
   friend class base::RefCounted<Compositor>;
-  friend class CompositorLock;
 
-  // Called by CompositorLock.
-  void UnlockCompositor();
-
-  // Called to release any pending CompositorLock
-  void CancelCompositorLock();
+  // CompositorLockDelegate implementation.
+  void DoUnlockCompositor() override;
 
   gfx::Size size_;
 
   ui::ContextFactory* context_factory_;
   ui::ContextFactoryPrivate* context_factory_private_;
 
   // The root of the Layer tree drawn by this compositor.
   Layer* root_layer_;
 
   base::ObserverList<CompositorObserver, true> observer_list_;
@@ skipping 16 matching lines @@
   std::unique_ptr<cc::LayerTreeHost> host_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   // The manager of vsync parameters for this compositor.
   scoped_refptr<CompositorVSyncManager> vsync_manager_;
 
   // The device scale factor of the monitor that this compositor is compositing
   // layers on.
   float device_scale_factor_;
 
-  bool locks_will_time_out_;
   CompositorLock* compositor_lock_;
 
   LayerAnimatorCollection layer_animator_collection_;
   scoped_refptr<cc::AnimationTimeline> animation_timeline_;
   std::unique_ptr<ScopedAnimationDurationScaleMode> slow_animations_;
 
   gfx::ColorSpace output_color_space_;
   gfx::ColorSpace blending_color_space_;
 
   base::WeakPtrFactory<Compositor> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Compositor);
 };
 
 }  // namespace ui
 
 #endif  // UI_COMPOSITOR_COMPOSITOR_H_