Fix race condition in flaky GamepadProvider tests

Fix race condition in flaky GamepadProvider tests

GamepadProvider polls GamepadDataFetchers on a background thread, then
copies the gamepad state into a shared memory buffer. Unit tests need to
know when this buffer has been updated to verify that the new state is
correct and check for callbacks. MockGamepadDataFetcher signals when its
state is queried, which is used as an indicator that the buffer may have
new data.

The buffer is not written until after gamepad state is read from the
fetchers, creating a small window between the MockGamepadDataFetcher signal
and the actual buffer write. Rarely, the main thread would read from the
buffer before the provider could write to it, causing test failures.

Instead of relying on MockGamepadDataFetcher, the tests will now wait for
the version number of the shared memory buffer's seqlock to advance before
continuing. This ensures that the buffer has been written to at least once.

BUG=692219
Review-Url: https://codereview.chromium.org/2820563003
Cr-Commit-Position: refs/heads/master@{#464760}
Committed: https://chromium.googlesource.com/chromium/src/+/d5427aa537bb0f921a2dda161bb3b555a0e6da6c

[mattreynolds, 2017-04-14 00:53:59]

Description was changed from

==========
Fix race condition in flaky GamepadProvider tests

GamepadProvider polls GamepadDataFetchers on a background thread, then
copies the gamepad state into a shared memory buffer. Unit tests need to
know when this buffer has been updated to verify that the new state is
correct and check for callbacks. MockGamepadDataFetcher signals when its
state is queried, which is used as an indicator that the buffer may have
new data.

The buffer is not written until after gamepad state is read from the
fetchers, creating a small window between the MockGamepadDataFetcher signal
and the actual buffer write. Rarely, the main thread would read from the
buffer before the provider could write to it, causing test failures.

Instead of relying on MockGamepadDataFetcher, the tests will now wait for
the version number of the shared memory buffer's seqlock to advance before
continuing. This ensures that the buffer has been written to at least once.

BUG=692219
==========

to

==========
Fix race condition in flaky GamepadProvider tests

GamepadProvider polls GamepadDataFetchers on a background thread, then
copies the gamepad state into a shared memory buffer. Unit tests need to
know when this buffer has been updated to verify that the new state is
correct and check for callbacks. MockGamepadDataFetcher signals when its
state is queried, which is used as an indicator that the buffer may have
new data.

The buffer is not written until after gamepad state is read from the
fetchers, creating a small window between the MockGamepadDataFetcher signal
and the actual buffer write. Rarely, the main thread would read from the
buffer before the provider could write to it, causing test failures.

Instead of relying on MockGamepadDataFetcher, the tests will now wait for
the version number of the shared memory buffer's seqlock to advance before
continuing. This ensures that the buffer has been written to at least once.

BUG=692219
==========

[mattreynolds, 2017-04-14 00:53:59]

mattreynolds@chromium.org changed reviewers:
+ bajones@chromium.org

[mattreynolds, 2017-04-14 00:54:45]

Hi Brandon, I think I figured out what was causing our gamepad provider tests to
be flaky.  PTAL!

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

Great work! Definitely sounds as if that could be the culprit. One nit, but
otherwise LGTM!

https://codereview.chromium.org/2820563003/diff/1/device/gamepad/gamepad_prov...
File device/gamepad/gamepad_provider_unittest.cc (right):

https://codereview.chromium.org/2820563003/diff/1/device/gamepad/gamepad_prov...
device/gamepad/gamepad_provider_unittest.cc:186: WaitForData(buffer);
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]

Thanks Brandon

https://codereview.chromium.org/2820563003/diff/1/device/gamepad/gamepad_prov...
File device/gamepad/gamepad_provider_unittest.cc (right):

https://codereview.chromium.org/2820563003/diff/1/device/gamepad/gamepad_prov...
device/gamepad/gamepad_provider_unittest.cc:186: WaitForData(buffer);
On 2017/04/14 03:06:18, bajones wrote:
> 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?)

Done.

[mattreynolds, 2017-04-14 18:05:53]

The CQ bit was checked by mattreynolds@chromium.org

[mattreynolds, 2017-04-14 18:05:53]

The patchset sent to the CQ was uploaded after l-g-t-m from bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/2820563003/#ps20001
(title: "add WaitForDataAndCallbacksIssued")

[commit-bot: I haz the power, 2017-04-14 18:06:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 18:30:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492193153293830,
"parent_rev": "9cbdd0bb8ed688c37d279ae367e60e8bba888d0b", "commit_rev":
"d5427aa537bb0f921a2dda161bb3b555a0e6da6c"}

[commit-bot: I haz the power, 2017-04-14 18:31:44]

Description was changed from

==========
Fix race condition in flaky GamepadProvider tests

GamepadProvider polls GamepadDataFetchers on a background thread, then
copies the gamepad state into a shared memory buffer. Unit tests need to
know when this buffer has been updated to verify that the new state is
correct and check for callbacks. MockGamepadDataFetcher signals when its
state is queried, which is used as an indicator that the buffer may have
new data.

The buffer is not written until after gamepad state is read from the
fetchers, creating a small window between the MockGamepadDataFetcher signal
and the actual buffer write. Rarely, the main thread would read from the
buffer before the provider could write to it, causing test failures.

Instead of relying on MockGamepadDataFetcher, the tests will now wait for
the version number of the shared memory buffer's seqlock to advance before
continuing. This ensures that the buffer has been written to at least once.

BUG=692219
==========

to

==========
Fix race condition in flaky GamepadProvider tests

GamepadProvider polls GamepadDataFetchers on a background thread, then
copies the gamepad state into a shared memory buffer. Unit tests need to
know when this buffer has been updated to verify that the new state is
correct and check for callbacks. MockGamepadDataFetcher signals when its
state is queried, which is used as an indicator that the buffer may have
new data.

The buffer is not written until after gamepad state is read from the
fetchers, creating a small window between the MockGamepadDataFetcher signal
and the actual buffer write. Rarely, the main thread would read from the
buffer before the provider could write to it, causing test failures.

Instead of relying on MockGamepadDataFetcher, the tests will now wait for
the version number of the shared memory buffer's seqlock to advance before
continuing. This ensures that the buffer has been written to at least once.

BUG=692219

Review-Url: https://codereview.chromium.org/2820563003
Cr-Commit-Position: refs/heads/master@{#464760}
Committed:
https://chromium.googlesource.com/chromium/src/+/d5427aa537bb0f921a2dda161bb3...
==========

[commit-bot: I haz the power, 2017-04-14 18:31:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d5427aa537bb0f921a2dda161bb3...