Fix race condition in flaky GamepadProvider tests

device/gamepad/gamepad_provider_unittest.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 "device/gamepad/gamepad_provider.h"
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
+#include "base/threading/platform_thread.h"
 #include "build/build_config.h"
 #include "device/gamepad/gamepad_data_fetcher.h"
 #include "device/gamepad/gamepad_test_helpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace device {
 
 namespace {
 
 using blink::WebGamepads;
@@ skipping 20 matching lines @@
 // Main test fixture
 class GamepadProviderTest : public testing::Test, public GamepadTestHelper {
  public:
   GamepadProvider* CreateProvider(const WebGamepads& test_data) {
     mock_data_fetcher_ = new MockGamepadDataFetcher(test_data);
     provider_.reset(new GamepadProvider(
         nullptr, std::unique_ptr<GamepadDataFetcher>(mock_data_fetcher_)));
     return provider_.get();
   }
 
+  // Sleep until the shared memory buffer's seqlock advances the buffer version,
+  // indicating that the gamepad provider has written to it after polling the
+  // gamepad fetchers. The buffer will report an odd value for the version if
+  // the buffer is not in a consistent state, so we also require that the value
+  // is even before continuing.
+  void WaitForData(GamepadHardwareBuffer* buffer) {
+    const base::subtle::Atomic32 initial_version = buffer->seqlock.ReadBegin();
+    base::subtle::Atomic32 current_version;
+    do {
+      base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(10));
+      current_version = buffer->seqlock.ReadBegin();
+    } while (current_version % 2 || current_version == initial_version);
+  }
+
   void ReadGamepadHardwareBuffer(GamepadHardwareBuffer* buffer,
                                  WebGamepads* output) {
     memset(output, 0, sizeof(WebGamepads));
     base::subtle::Atomic32 version;
     do {
       version = buffer->seqlock.ReadBegin();
       memcpy(output, &buffer->data, sizeof(WebGamepads));
     } while (buffer->seqlock.ReadRetry(version));
   }
 
  protected:
   GamepadProviderTest() {}
 
   std::unique_ptr<GamepadProvider> provider_;
 
   // Pointer owned by the provider.
   MockGamepadDataFetcher* mock_data_fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(GamepadProviderTest);
 };
 
-// Test is flaky. crbug.com/705367
-TEST_F(GamepadProviderTest, DISABLED_PollingAccess) {
+TEST_F(GamepadProviderTest, PollingAccess) {
   WebGamepads test_data;
   memset(&test_data, 0, sizeof(WebGamepads));
   test_data.items[0].connected = true;
   test_data.items[0].timestamp = 0;
   test_data.items[0].buttons_length = 1;
   test_data.items[0].axes_length = 2;
   test_data.items[0].buttons[0].value = 1.f;
   test_data.items[0].buttons[0].pressed = true;
   test_data.items[0].axes[0] = -1.f;
   test_data.items[0].axes[1] = .5f;
 
   GamepadProvider* provider = CreateProvider(test_data);
   provider->SetSanitizationEnabled(false);
   provider->Resume();
 
   base::RunLoop().RunUntilIdle();
 
-  mock_data_fetcher_->WaitForDataRead();
-
   // Renderer-side, pull data out of poll buffer.
   base::SharedMemoryHandle handle = provider->GetSharedMemoryHandleForProcess(
       base::GetCurrentProcessHandle());
   std::unique_ptr<base::SharedMemory> shared_memory(
       new base::SharedMemory(handle, true));
   EXPECT_TRUE(shared_memory->Map(sizeof(GamepadHardwareBuffer)));
 
   GamepadHardwareBuffer* buffer =
       static_cast<GamepadHardwareBuffer*>(shared_memory->memory());
+
+  // Wait until the shared memory buffer has been written at least once.
+  WaitForData(buffer);
+
   WebGamepads output;
   ReadGamepadHardwareBuffer(buffer, &output);
 
   EXPECT_EQ(1u, output.items[0].buttons_length);
   EXPECT_EQ(1.f, output.items[0].buttons[0].value);
   EXPECT_EQ(true, output.items[0].buttons[0].pressed);
   EXPECT_EQ(2u, output.items[0].axes_length);
   EXPECT_EQ(-1.f, output.items[0].axes[0]);
   EXPECT_EQ(0.5f, output.items[0].axes[1]);
 }
 
-// Flaky on all platforms: http://crbug.com/692219
-TEST_F(GamepadProviderTest, DISABLED_ConnectDisconnectMultiple) {
+TEST_F(GamepadProviderTest, ConnectDisconnectMultiple) {
   WebGamepads test_data;
   test_data.items[0].connected = true;
   test_data.items[0].timestamp = 0;
   test_data.items[0].axes_length = 2;
   test_data.items[0].axes[0] = -1.f;
   test_data.items[0].axes[1] = .5f;
 
   test_data.items[1].connected = true;
   test_data.items[1].timestamp = 0;
   test_data.items[1].axes_length = 2;
   test_data.items[1].axes[0] = 1.f;
   test_data.items[1].axes[1] = -.5f;
 
   WebGamepads test_data_onedisconnected;
   test_data_onedisconnected.items[1].connected = true;
   test_data_onedisconnected.items[1].timestamp = 0;
   test_data_onedisconnected.items[1].axes_length = 2;
   test_data_onedisconnected.items[1].axes[0] = 1.f;
   test_data_onedisconnected.items[1].axes[1] = -.5f;
 
   GamepadProvider* provider = CreateProvider(test_data);
   provider->SetSanitizationEnabled(false);
   provider->Resume();
 
   base::RunLoop().RunUntilIdle();
 
-  mock_data_fetcher_->WaitForDataRead();
-
   // Renderer-side, pull data out of poll buffer.
   base::SharedMemoryHandle handle = provider->GetSharedMemoryHandleForProcess(
       base::GetCurrentProcessHandle());
   std::unique_ptr<base::SharedMemory> shared_memory(
       new base::SharedMemory(handle, true));
   EXPECT_TRUE(shared_memory->Map(sizeof(GamepadHardwareBuffer)));
 
   GamepadHardwareBuffer* buffer =
       static_cast<GamepadHardwareBuffer*>(shared_memory->memory());
+
+  // Wait until the shared memory buffer has been written at least once.
+  WaitForData(buffer);
+
   WebGamepads output;
   ReadGamepadHardwareBuffer(buffer, &output);
 
   EXPECT_EQ(2u, output.items[0].axes_length);
   EXPECT_EQ(-1.f, output.items[0].axes[0]);
   EXPECT_EQ(0.5f, output.items[0].axes[1]);
   EXPECT_EQ(2u, output.items[1].axes_length);
   EXPECT_EQ(1.f, output.items[1].axes[0]);
   EXPECT_EQ(-0.5f, output.items[1].axes[1]);
 
   mock_data_fetcher_->SetTestData(test_data_onedisconnected);
-  mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
+
+  // The provider polls the data on the background thread and then issues
+  // the callback on the client thread. Waiting for it to poll twice ensures
+  // that it was able to issue callbacks for the first poll.
+  WaitForData(buffer);
+  WaitForData(buffer);

[bajones, 2017/04/14 03:06:18]

This sequence and comment is repeated often enough that it's probably worth
putting in a function up top with the comment and an explanatory name (like,
say, WaitForDataAndCallbacksIssued?)

[mattreynolds, 2017/04/14 18:05:48]

Done.

+
   ReadGamepadHardwareBuffer(buffer, &output);
 
   EXPECT_EQ(0u, output.items[0].axes_length);
   EXPECT_EQ(2u, output.items[1].axes_length);
   EXPECT_EQ(1.f, output.items[1].axes[0]);
   EXPECT_EQ(-0.5f, output.items[1].axes[1]);
 }
 
 // Tests that waiting for a user gesture works properly.
 TEST_F(GamepadProviderTest, UserGesture) {
@@ skipping 10 matching lines @@
   WebGamepads button_down_data = no_button_data;
   button_down_data.items[0].buttons[0].value = 1.f;
   button_down_data.items[0].buttons[0].pressed = true;
 
   UserGestureListener listener;
   GamepadProvider* provider = CreateProvider(no_button_data);
   provider->SetSanitizationEnabled(false);
   provider->Resume();
 
   provider->RegisterForUserGesture(listener.GetClosure());
-  mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
+
+  base::RunLoop().RunUntilIdle();
+
+  // Renderer-side, pull data out of poll buffer.
+  base::SharedMemoryHandle handle = provider->GetSharedMemoryHandleForProcess(
+      base::GetCurrentProcessHandle());
+  std::unique_ptr<base::SharedMemory> shared_memory(
+      new base::SharedMemory(handle, true));
+  EXPECT_TRUE(shared_memory->Map(sizeof(GamepadHardwareBuffer)));
+
+  GamepadHardwareBuffer* buffer =
+      static_cast<GamepadHardwareBuffer*>(shared_memory->memory());
+
+  // Wait until the shared memory buffer has been written at least once.
+  WaitForData(buffer);
 
   // It should not have issued our callback.
-  base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(listener.has_user_gesture());
 
-  // Set a button down and wait for it to be read twice.
+  // Set a button down.
   mock_data_fetcher_->SetTestData(button_down_data);
-  mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
+
+  // The provider polls the data on the background thread and then issues
+  // the callback on the client thread. Waiting for it to poll twice ensures
+  // that it was able to issue callbacks for the first poll.
+  WaitForData(buffer);
+  WaitForData(buffer);
 
   // It should have issued our callback.
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(listener.has_user_gesture());
 }
 
-// Flaky on CrOS and Linux: http://crbug.com/640086, https://crbug.com/702712
-#if defined(OS_LINUX) || defined(OS_CHROMEOS)
-#define MAYBE_Sanitization DISABLED_Sanitization
-#else
-#define MAYBE_Sanitization Sanitization
-#endif
 // Tests that waiting for a user gesture works properly.
-TEST_F(GamepadProviderTest, MAYBE_Sanitization) {
+TEST_F(GamepadProviderTest, Sanitization) {
   WebGamepads active_data;
   active_data.items[0].connected = true;
   active_data.items[0].timestamp = 0;
   active_data.items[0].buttons_length = 1;
   active_data.items[0].axes_length = 1;
   active_data.items[0].buttons[0].value = 1.f;
   active_data.items[0].buttons[0].pressed = true;
   active_data.items[0].axes[0] = -1.f;
 
   WebGamepads zero_data;
   zero_data.items[0].connected = true;
   zero_data.items[0].timestamp = 0;
   zero_data.items[0].buttons_length = 1;
   zero_data.items[0].axes_length = 1;
   zero_data.items[0].buttons[0].value = 0.f;
   zero_data.items[0].buttons[0].pressed = false;
   zero_data.items[0].axes[0] = 0.f;
 
   UserGestureListener listener;
   GamepadProvider* provider = CreateProvider(active_data);
   provider->SetSanitizationEnabled(true);
   provider->Resume();
 
   base::RunLoop().RunUntilIdle();
 
-  mock_data_fetcher_->WaitForDataRead();
-
   // Renderer-side, pull data out of poll buffer.
   base::SharedMemoryHandle handle = provider->GetSharedMemoryHandleForProcess(
       base::GetCurrentProcessHandle());
   std::unique_ptr<base::SharedMemory> shared_memory(
       new base::SharedMemory(handle, true));
   EXPECT_TRUE(shared_memory->Map(sizeof(GamepadHardwareBuffer)));
 
   GamepadHardwareBuffer* buffer =
       static_cast<GamepadHardwareBuffer*>(shared_memory->memory());
+
+  // Wait until the shared memory buffer has been written at least once.
+  WaitForData(buffer);
+
   WebGamepads output;
   ReadGamepadHardwareBuffer(buffer, &output);
 
   // Initial data should all be zeroed out due to sanitization, even though the
   // gamepad reported input
   EXPECT_EQ(1u, output.items[0].buttons_length);
   EXPECT_EQ(0.f, output.items[0].buttons[0].value);
   EXPECT_FALSE(output.items[0].buttons[0].pressed);
   EXPECT_EQ(1u, output.items[0].axes_length);
   EXPECT_EQ(0.f, output.items[0].axes[0]);
 
   // Zero out the inputs
   mock_data_fetcher_->SetTestData(zero_data);
-  mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
+
+  // The provider polls the data on the background thread and then issues
+  // the callback on the client thread. Waiting for it to poll twice ensures
+  // that it was able to issue callbacks for the first poll.
+  WaitForData(buffer);
+  WaitForData(buffer);
 
   // Read updated data from shared memory
   ReadGamepadHardwareBuffer(buffer, &output);
 
   // Should still read zero, which is now an accurate reflection of the data
   EXPECT_EQ(1u, output.items[0].buttons_length);
   EXPECT_EQ(0.f, output.items[0].buttons[0].value);
   EXPECT_FALSE(output.items[0].buttons[0].pressed);
   EXPECT_EQ(1u, output.items[0].axes_length);
   EXPECT_EQ(0.f, output.items[0].axes[0]);
 
   // Re-set the active inputs
   mock_data_fetcher_->SetTestData(active_data);
-  mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
+
+  // Wait for two more polls.
+  WaitForData(buffer);
+  WaitForData(buffer);
 
   // Read updated data from shared memory
   ReadGamepadHardwareBuffer(buffer, &output);
 
   // Should now accurately reflect the reported data.
   EXPECT_EQ(1u, output.items[0].buttons_length);
   EXPECT_EQ(1.f, output.items[0].buttons[0].value);
   EXPECT_TRUE(output.items[0].buttons[0].pressed);
   EXPECT_EQ(1u, output.items[0].axes_length);
   EXPECT_EQ(-1.f, output.items[0].axes[0]);
 }
 
 }  // namespace
 
 }  // namespace device