Fix CompositorResizeLock to do something.

ui/compositor/compositor_unittest.cc

 // Copyright 2015 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 <stdint.h>
 
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "cc/output/begin_frame_args.h"
@@ skipping 33 matching lines @@
     compositor_.reset(new ui::Compositor(
         context_factory_private->AllocateFrameSinkId(), context_factory,
         context_factory_private, task_runner_));
     compositor_->SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
   }
   void TearDown() override {
     compositor_.reset();
     ui::TerminateContextFactoryForTests();
   }
 
+  void DestroyCompositor() { compositor_.reset(); }
+
  protected:
   base::SingleThreadTaskRunner* task_runner() { return task_runner_.get(); }
   ui::Compositor* compositor() { return compositor_.get(); }
 
  private:
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   std::unique_ptr<ui::Compositor> compositor_;
 
   DISALLOW_COPY_AND_ASSIGN(CompositorTest);
 };
 
+class MockCompositorLockClient
+    : NON_EXPORTED_BASE(public ui::CompositorLockClient) {
+ public:
+  MOCK_METHOD0(CompositorLockTimedOut, void());
+};
+
 }  // namespace
 
 TEST_F(CompositorTest, LocksTimeOut) {
-  scoped_refptr<ui::CompositorLock> lock;
+  std::unique_ptr<CompositorLock> lock;
+
+  base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(100);
+
   {
+    testing::StrictMock<MockCompositorLockClient> lock_client;
     base::RunLoop run_loop;
-    // Ensure that the lock times out by default.
-    lock = compositor()->GetCompositorLock();
+    // This lock has a timeout.
+    lock = compositor()->GetCompositorLock(&lock_client, timeout);
     EXPECT_TRUE(compositor()->IsLocked());
-    task_runner()->PostDelayedTask(
-        FROM_HERE, run_loop.QuitClosure(),
-        base::TimeDelta::FromMilliseconds(kCompositorLockTimeoutMs));
+    task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout);
+    EXPECT_CALL(lock_client, CompositorLockTimedOut()).Times(1);
     run_loop.Run();
     EXPECT_FALSE(compositor()->IsLocked());
   }
 
   {
+    testing::StrictMock<MockCompositorLockClient> lock_client;
     base::RunLoop run_loop;
-    // Ensure that the lock does not time out when set.
-    compositor()->SetLocksWillTimeOut(false);
-    lock = compositor()->GetCompositorLock();
+    // This lock has no timeout.
+    lock = compositor()->GetCompositorLock(&lock_client, base::TimeDelta());
     EXPECT_TRUE(compositor()->IsLocked());
-    task_runner()->PostDelayedTask(
-        FROM_HERE, run_loop.QuitClosure(),
-        base::TimeDelta::FromMilliseconds(kCompositorLockTimeoutMs));
+    task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout);
+    EXPECT_CALL(lock_client, CompositorLockTimedOut()).Times(0);
     run_loop.Run();
     EXPECT_TRUE(compositor()->IsLocked());
   }
 }
 
+TEST_F(CompositorTest, MultipleLockClients) {
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+  testing::StrictMock<MockCompositorLockClient> lock_client2;
+  std::unique_ptr<CompositorLock> lock2;
+
+  base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(1);
+  base::RunLoop run_loop;
+  // Both locks are grabbed from the Compositor with a separate client.
+  lock1 = compositor()->GetCompositorLock(&lock_client1, timeout);
+  lock2 = compositor()->GetCompositorLock(&lock_client2, timeout);
+  EXPECT_TRUE(compositor()->IsLocked());
+  task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout);
+  // Both clients get notified of timeout.
+  EXPECT_CALL(lock_client1, CompositorLockTimedOut()).Times(1);
+  EXPECT_CALL(lock_client2, CompositorLockTimedOut()).Times(1);
+  run_loop.Run();
+  EXPECT_FALSE(compositor()->IsLocked());
+}
+
+TEST_F(CompositorTest, ExtendingLifeOfLockDoesntUseDeadClient) {
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+  testing::StrictMock<MockCompositorLockClient> lock_client2;
+  std::unique_ptr<CompositorLock> lock2;
+
+  base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(1);
+  base::RunLoop run_loop;
+
+  // One lock is grabbed from the compositor with a client. The other
+  // extends its lifetime past that of the first.
+  lock1 = compositor()->GetCompositorLock(&lock_client1, timeout);
+  EXPECT_TRUE(compositor()->IsLocked());
+
+  // This also locks the compositor and will do so past |lock1| ending.
+  lock2 = compositor()->GetCompositorLock(&lock_client2, timeout);
+  // |lock1| is destroyed, so it won't timeout but |lock2| will.
+  lock1 = nullptr;
+
+  task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout);
+
+  EXPECT_CALL(lock_client1, CompositorLockTimedOut()).Times(0);
+  EXPECT_CALL(lock_client2, CompositorLockTimedOut()).Times(1);
+  run_loop.Run();
+  EXPECT_FALSE(compositor()->IsLocked());
+}
+
+TEST_F(CompositorTest, AddingLocksDoesNotExtendTimeout) {
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+  testing::StrictMock<MockCompositorLockClient> lock_client2;
+  std::unique_ptr<CompositorLock> lock2;
+
+  base::TimeDelta timeout1 = base::TimeDelta::FromMilliseconds(1);
+  base::TimeDelta timeout2 = base::TimeDelta::FromMilliseconds(10);
+  base::RunLoop run_loop;
+
+  // The first lock has a short timeout.
+  lock1 = compositor()->GetCompositorLock(&lock_client1, timeout1);
+  EXPECT_TRUE(compositor()->IsLocked());
+
+  // The second lock has a longer timeout, but since a lock is active,
+  // the first one is used for both.
+  lock2 = compositor()->GetCompositorLock(&lock_client2, timeout2);
+
+  task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout1);
+
+  EXPECT_CALL(lock_client1, CompositorLockTimedOut()).Times(1);
+  EXPECT_CALL(lock_client2, CompositorLockTimedOut()).Times(1);
+  run_loop.Run();
+  EXPECT_FALSE(compositor()->IsLocked());
+}
+
+TEST_F(CompositorTest, LockIsDestroyedDoesntTimeout) {
+  base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(1);
+  base::RunLoop run_loop;
+
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+  lock1 = compositor()->GetCompositorLock(&lock_client1, timeout);
+  EXPECT_TRUE(compositor()->IsLocked());
+  task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout);
+  // The CompositorLockClient is destroyed when |lock1| is released.
+  lock1 = nullptr;
+  // The client isn't called as a result.
+  EXPECT_CALL(lock_client1, CompositorLockTimedOut()).Times(0);
+  run_loop.Run();
+  EXPECT_FALSE(compositor()->IsLocked());
+}
+
+TEST_F(CompositorTest, TimeoutEndsWhenLockEnds) {
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+  testing::StrictMock<MockCompositorLockClient> lock_client2;
+  std::unique_ptr<CompositorLock> lock2;
+
+  base::TimeDelta timeout1 = base::TimeDelta::FromMilliseconds(1);
+  base::TimeDelta timeout2 = base::TimeDelta::FromMilliseconds(10);
+
+  // The first lock has a short timeout.
+  lock1 = compositor()->GetCompositorLock(&lock_client1, timeout1);
+  EXPECT_TRUE(compositor()->IsLocked());
+  // But the first lock is ended before timeout.
+  lock1 = nullptr;
+  EXPECT_FALSE(compositor()->IsLocked());
+
+  // The second lock has a longer timeout, and it should use that timeout,
+  // since the first lock is done.
+  lock2 = compositor()->GetCompositorLock(&lock_client2, timeout2);
+  EXPECT_TRUE(compositor()->IsLocked());
+
+  {
+    // The second lock doesn't timeout from the first lock which has ended.
+    base::RunLoop run_loop;
+    task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout1);

[piman, 2017/03/24 19:57:45]

This one could in theory be racy causing a flake, if for some reason l.214 or
l.218 take >9ms to execute, causing this callback to run after the timeout2. Is
it possible to inject fake time, or is it hard?

[danakj, 2017/03/24 20:04:48]

Good point, let me look around in base/test/

+
+    EXPECT_CALL(lock_client2, CompositorLockTimedOut()).Times(0);
+    run_loop.Run();
+  }
+
+  {
+    // The second lock can still timeout on its own though.
+    base::RunLoop run_loop;
+    task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), timeout2);
+
+    EXPECT_CALL(lock_client2, CompositorLockTimedOut()).Times(1);
+    run_loop.Run();
+  }
+
+  EXPECT_FALSE(compositor()->IsLocked());
+}
+
+TEST_F(CompositorTest, CompositorLockOutlivesCompositor) {
+  testing::StrictMock<MockCompositorLockClient> lock_client1;
+  std::unique_ptr<CompositorLock> lock1;
+
+  lock1 = compositor()->GetCompositorLock(&lock_client1, base::TimeDelta());
+  // The compositor is destroyed before the lock.
+  DestroyCompositor();
+  // This doesn't crash.
+  lock1 = nullptr;
+}
+
 TEST_F(CompositorTest, ReleaseWidgetWithOutputSurfaceNeverCreated) {
   compositor()->SetVisible(false);
   EXPECT_EQ(gfx::kNullAcceleratedWidget,
             compositor()->ReleaseAcceleratedWidget());
   compositor()->SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
   compositor()->SetVisible(true);
 }
 
 #if defined(OS_WIN)
 // TODO(crbug.com/608436): Flaky on windows trybots
@@ skipping 14 matching lines @@
   EXPECT_EQ(gfx::kNullAcceleratedWidget,
             compositor()->ReleaseAcceleratedWidget());
   compositor()->SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
   compositor()->SetVisible(true);
   compositor()->ScheduleDraw();
   DrawWaiterForTest::WaitForCompositingEnded(compositor());
   compositor()->SetRootLayer(nullptr);
 }
 
 }  // namespace ui