Fix thread checking logic in ContextProviderCommandBuffer

Fix thread checking logic in ContextProviderCommandBuffer

The thread check error addressed by https://codereview.chromium.org/1450433002
Was the result of an unnecessary thread check before thread attachment, which
was causing the thread checker to initialize itself on the wrong thread.

This CL reverts https://codereview.chromium.org/1450433002 and removes thread
checks from all the places where there were no thread checks prior to
https://codereview.chromium.org/1414683003/

BUG=556725
R=piman@chromium.org

Committed: https://crrev.com/de9af7e94dd4a3acd66b0135da4b80f4ba42ba56
Cr-Commit-Position: refs/heads/master@{#360928}

[piman, 2015-11-20 04:54:34]

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

[piman, 2015-11-20 04:54:35]

+reveman
I think the suggestion on the bug is that there is indeed an unsafe use, and so
we should identify it?
WebContext3D() should be valid almost everywhere if the ScopedContextLock does
reset the bound thread on enter & leave.

https://codereview.chromium.org/1459193003/diff/1/content/common/gpu/client/c...
File content/common/gpu/client/context_provider_command_buffer.cc (right):

https://codereview.chromium.org/1459193003/diff/1/content/common/gpu/client/c...
content/common/gpu/client/context_provider_command_buffer.cc:155:
WebContext3DNoChecks()->GetCommandBufferProxy()->SetLock(&context_lock_);
I think this one should stay as WebContext3D(), because we need to setup the
lock on the thread it was bound to (otherwise unsafe things would happen).

[reveman, 2015-11-20 05:42:56]

On 2015/11/20 at 04:54:35, piman wrote:
> +reveman
> I think the suggestion on the bug is that there is indeed an unsafe use, and
so we should identify it?
> WebContext3D() should be valid almost everywhere if the ScopedContextLock does
reset the bound thread on enter & leave.

Yes, ideally we'd have a way to do a scoped-bound-thread instead of having the
thread checker be reset to allow any thread.

[Justin Novosad, 2015-11-20 15:18:38]

On 2015/11/20 05:42:56, reveman wrote:
> On 2015/11/20 at 04:54:35, piman wrote:
> > +reveman
> > I think the suggestion on the bug is that there is indeed an unsafe use, and
> so we should identify it?
> > WebContext3D() should be valid almost everywhere if the ScopedContextLock
does
> reset the bound thread on enter & leave.
> 
> Yes, ideally we'd have a way to do a scoped-bound-thread instead of having the
> thread checker be reset to allow any thread.

That sounds like a great idea. I don't know this area of the code well enough to
tell where the scoped binder objects should be placed. I presume they would go
in several places a few stack levels up from where DetachFromThread is called?
Okay to land this CL, and you can follow-up with a scoped-bound-thread solution?

[reveman, 2015-11-20 15:23:02]

lgtm % piman's SetupLock() comment

[piman, 2015-11-20 19:27:09]

LGTM if reveman@ is happy with this.

[Justin Novosad, 2015-11-20 21:05:40]

On 2015/11/20 19:27:09, piman (slow to review) wrote:
> LGTM if reveman@ is happy with this.

and the win debug bot is okay with it too...

[Justin Novosad, 2015-11-20 21:05:44]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2015-11-20 21:05:44]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/1459193003/#ps20001
(title: "re-add check on SetupLock")

[commit-bot: I haz the power, 2015-11-20 21:06:50]

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

[commit-bot: I haz the power, 2015-11-20 22:12:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-20 23:03:32]

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