GpuVSyncProvider with unit test

gpu/ipc/service/gpu_vsync_provider_unittest_win.cc

Index: gpu/ipc/service/gpu_vsync_provider_unittest_win.cc
diff --git a/gpu/ipc/service/gpu_vsync_provider_unittest_win.cc b/gpu/ipc/service/gpu_vsync_provider_unittest_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..25d8c684f76a60ae890cbc5b21292aa8687080e5
--- /dev/null
+++ b/gpu/ipc/service/gpu_vsync_provider_unittest_win.cc
@@ -0,0 +1,70 @@
+// Copyright (c) 2016 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 "gpu/ipc/service/gpu_vsync_provider.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/synchronization/waitable_event.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/win/hidden_window.h"
+
+#include <windows.h>
+
+namespace gpu {
+
+class GpuVSyncProviderTest : public testing::Test {
+ public:
+  GpuVSyncProviderTest()
+      : vsync_event_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
+                     base::WaitableEvent::InitialState::NOT_SIGNALED) {}
+  ~GpuVSyncProviderTest() override {}
+
+  void SetUp() override {}
+
+  void TearDown() override {}
+
+  void OnVSync(base::TimeTicks timestamp) {
+    // This is called on VSync worker thread.
+    if (InterlockedIncrement(&vsync_count_) == 3)
+      vsync_event_.Signal();
+  }
+
+  base::WaitableEvent vsync_event_;
+  volatile LONG vsync_count_ = 0;

[brucedawson, 2016/12/22 21:49:16]

Volatile isn't needed here. What makes this safe to access from multiple threads
is the use of Interlocked operations. The non-interlocked accesses to this
variable are theoretically dangerous.

Safer thing to do is to use a lock to protect vsync_count_. Yes, it feels like
overkill, but it is the only way to avoid a host of subtle and undefined
behavior.

TL;DR - use a lock to protect all accesses to vsync_count_ and get rid of
volatile and Interlocked*. Safer, and more portable.

[stanisc, 2017/01/04 22:33:04]

Done.

+};
+
+TEST_F(GpuVSyncProviderTest, VSyncSignalTest) {
+  SurfaceHandle window = ui::GetHiddenWindow();
+
+  std::unique_ptr<GpuVSyncProvider> provider = GpuVSyncProvider::Create(
+      base::Bind(&GpuVSyncProviderTest::OnVSync, base::Unretained(this)),
+      window);
+
+  constexpr base::TimeDelta wait_timeout =
+      base::TimeDelta::FromMilliseconds(300);
+
+  // Verify that there are no VSync signals before provider is enabled
+  bool wait_result = vsync_event_.TimedWait(wait_timeout);
+  EXPECT_FALSE(wait_result);
+  EXPECT_EQ(0, vsync_count_);
+
+  provider->EnableVSync(true);
+
+  vsync_event_.Wait();
+
+  provider->EnableVSync(false);
+
+  // Verify that VSync callbacks stop coming after disabling.
+  // Please note that it might still be possible for one
+  // callback to be in flight on VSync worker thread, so |vsync_count_|
+  // could still be incremented once, but not enough times to trigger
+  // |vsync_event_|.
+  InterlockedExchange(&vsync_count_, 0);
+  wait_result = vsync_event_.TimedWait(wait_timeout);
+  EXPECT_FALSE(wait_result);
+}
+
+}  // namespace gpu