Fix CompositorResizeLock to do something.

ui/compositor/compositor_unittest.cc

Index: ui/compositor/compositor_unittest.cc
diff --git a/ui/compositor/compositor_unittest.cc b/ui/compositor/compositor_unittest.cc
index e62e0b8c430a84131a25aeb4a6c3ee619a3eab11..0cad16edf04423f82529a0f361b98c9cb3e6cb03 100644
--- a/ui/compositor/compositor_unittest.cc
+++ b/ui/compositor/compositor_unittest.cc
@@ -51,6 +51,8 @@ class CompositorTest : public testing::Test {
     ui::TerminateContextFactoryForTests();
   }
 
+  void DestroyCompositor() { compositor_.reset(); }
+
  protected:
   base::SingleThreadTaskRunner* task_runner() { return task_runner_.get(); }
   ui::Compositor* compositor() { return compositor_.get(); }
@@ -62,36 +64,187 @@ class CompositorTest : public testing::Test {
   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,