cc: Acquire worker context lock be calling BindToCurrentThread.

cc: Acquire worker context lock be calling BindToCurrentThread.

BUG=523411
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

[reveman, 2015-08-24 10:29:25]

The CQ bit was checked by reveman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-24 10:29:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311093002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311093002/1

[commit-bot: I haz the power, 2015-08-24 11:24:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-24 11:24:41]

Dry run: This issue passed the CQ dry run.

[reveman, 2015-08-24 15:23:25]

reveman@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2015-08-24 20:47:38]

LGTM but is this testable?

[ericrk, 2015-08-24 21:00:10]

ericrk@chromium.org changed reviewers:
+ ericrk@chromium.org

[ericrk, 2015-08-24 21:00:10]

https://codereview.chromium.org/1311093002/diff/1/cc/output/output_surface.cc
File cc/output/output_surface.cc (right):

https://codereview.chromium.org/1311093002/diff/1/cc/output/output_surface.cc...
cc/output/output_surface.cc:132: ContextProvider::ScopedContextLock
scoped_context(
I think this may just be a naming issue and actually safe, but it seems weird
that we take the lock here before calling "SetupLock" - is SetupLock a misnomer?
comment says it "Sets up a lock so this context can be used from multiple
threads.", but it seems that it just passes the existing lock on to the command
buffer, rather than "setting up" anything.

Out of curiosity, were we hitting an issue that this fixes? if so, does that
mean there were other uses of the lock before we call "setuplock"?

[reveman, 2015-08-25 22:59:59]

https://codereview.chromium.org/1311093002/diff/1/cc/output/output_surface.cc
File cc/output/output_surface.cc (right):

https://codereview.chromium.org/1311093002/diff/1/cc/output/output_surface.cc...
cc/output/output_surface.cc:132: ContextProvider::ScopedContextLock
scoped_context(
On 2015/08/24 at 21:00:10, ericrk wrote:
> I think this may just be a naming issue and actually safe, but it seems weird
that we take the lock here before calling "SetupLock" - is SetupLock a misnomer?
comment says it "Sets up a lock so this context can be used from multiple
threads.", but it seems that it just passes the existing lock on to the command
buffer, rather than "setting up" anything.
> 
> Out of curiosity, were we hitting an issue that this fixes? if so, does that
mean there were other uses of the lock before we call "setuplock"?

Yes, the name of this function is a bit confusing. As I mentioned on the bug, we
might want to move this code out of cc::OutputSurface. Here's a patch that does
that instead:

https://codereview.chromium.org/1317743002

I think that's preferred unless there turns out to be some issues with that
approach.