cc: Fix race in SurfaceLayer unittests.

cc: Fix race in SurfaceLayer unittests.

This fixes data race in SurfaceLayerSwapPromise.
Reported by TSAN.
BUG=437454
Committed: https://crrev.com/6e8cb70336132348903ff193fb54973ea098839a
Cr-Commit-Position: refs/heads/master@{#310573}

[sohanjg, 2015-01-06 14:26:43]

sohan.jyoti@samsung.com changed reviewers:
+ danakj@chromium.org

[sohanjg, 2015-01-06 14:26:43]

Do you think function level locks will be a good way to protect from data races?
Please take a look, thanks.

[danakj, 2015-01-06 15:57:40]

https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
File cc/layers/surface_layer_unittest.cc (right):

https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
cc/layers/surface_layer_unittest.cc:182: base::AutoLock lock(lock_);
This still means ChangeTree and SwapBuffersOnThread may run in either order
which is not very desirable for a test.

Probably DidCommit should be DidCommitAndDrawFrame() so that SwapBuffersOnThread
always comes first? Or post the ChangeTree call from SwapBuffersOnThread
instead?

[sohanjg, 2015-01-08 12:05:41]

Please take a look, thanks.

https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
File cc/layers/surface_layer_unittest.cc (right):

https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
cc/layers/surface_layer_unittest.cc:182: base::AutoLock lock(lock_);
On 2015/01/06 15:57:40, danakj wrote:
> This still means ChangeTree and SwapBuffersOnThread may run in either order
> which is not very desirable for a test.
> 
> Probably DidCommit should be DidCommitAndDrawFrame() so that
SwapBuffersOnThread
> always comes first? Or post the ChangeTree call from SwapBuffersOnThread
> instead?

Yes, i have changed DidCommit to DidCommitAndDrawFrame.
I tried posting ChangeTree from SwapBuffersOnThread, but that hits an assert as
we do SetRootLayer in ChangeTree, and that will do a setneedscommit, which is
expected from Main thread.

[danakj, 2015-01-08 17:30:21]

On Jan 8, 2015 4:05 AM, <sohan.jyoti@samsung.com> wrote:
>
> Please take a look, thanks.
>
>
>
https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
> File cc/layers/surface_layer_unittest.cc (right):
>
>
https://codereview.chromium.org/834043004/diff/1/cc/layers/surface_layer_unit...
> cc/layers/surface_layer_unittest.cc:182: base::AutoLock lock(lock_);
> On 2015/01/06 15:57:40, danakj wrote:
>>
>> This still means ChangeTree and SwapBuffersOnThread may run in either
>
> order
>>
>> which is not very desirable for a test.
>
>
>> Probably DidCommit should be DidCommitAndDrawFrame() so that
>
> SwapBuffersOnThread
>>
>> always comes first? Or post the ChangeTree call from
>
> SwapBuffersOnThread
>>
>> instead?
>
>
> Yes, i have changed DidCommit to DidCommitAndDrawFrame.
> I tried posting ChangeTree from SwapBuffersOnThread, but that hits an
> assert as we do SetRootLayer in ChangeTree, and that will do a
> setneedscommit, which is expected from Main thread.

OK. (You would have to post it to the main thread from there.)

> https://codereview.chromium.org/834043004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-01-08 18:16:03]

LGTM

[sohanjg, 2015-01-08 19:15:49]

The CQ bit was checked by sohan.jyoti@samsung.com

[commit-bot: I haz the power, 2015-01-08 19:16:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/834043004/20001

[commit-bot: I haz the power, 2015-01-08 20:01:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-08 20:02:47]

Patchset 2 (id:??) landed as
https://crrev.com/6e8cb70336132348903ff193fb54973ea098839a
Cr-Commit-Position: refs/heads/master@{#310573}

[sohanjg, 2015-11-18 10:06:36]

Description was changed from

==========
cc: Fix race in SurfaceLayer unittests.

This fixes data race in SurfaceLayerSwapPromise.
Reported by TSAN.
BUG=437454

Committed: https://crrev.com/6e8cb70336132348903ff193fb54973ea098839a
Cr-Commit-Position: refs/heads/master@{#310573}
==========

to

==========
cc: Fix race in SurfaceLayer unittests.

This fixes data race in SurfaceLayerSwapPromise.
Reported by TSAN.
BUG=437454

Committed: https://crrev.com/6e8cb70336132348903ff193fb54973ea098839a
Cr-Commit-Position: refs/heads/master@{#310573}
==========