base::Bind() conversion for content/common/gpu and content/gpu

base::Bind() conversion for content/common/gpu and content/gpu

There will be a followup patch to finish the conversion for GpuCommandBufferStub.

BUG=none
TEST=compiles and passes try bots

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110607

[dcheng, 2011-11-16 02:07:38]

Standard base::Bind() conversion, but I noticed two strange things:
- GpuCommandBufferStub seems to have a completely unused
ScopedRunnableMethodFactory. I removed it.
- GpuWatchdogThread wrapped ScopedRunnableMethodFactory in a scoped_ptr. I'm
hoping Albert will know if base::WeakPtrFactory can be created on one thread and
GetWeakPtr()/InvalidateWeakPtrs() called on another. If it's safe to use in that
manner, it seems like the scoped_ptr can be eliminated.

[awong, 2011-11-16 02:20:59]

On 2011/11/16 02:07:38, dcheng wrote:
> Standard base::Bind() conversion, but I noticed two strange things:
> - GpuCommandBufferStub seems to have a completely unused
> ScopedRunnableMethodFactory. I removed it.
> - GpuWatchdogThread wrapped ScopedRunnableMethodFactory in a scoped_ptr. I'm
> hoping Albert will know if base::WeakPtrFactory can be created on one thread
and
> GetWeakPtr()/InvalidateWeakPtrs() called on another. If it's safe to use in
that
> manner, it seems like the scoped_ptr can be eliminated.

I'm almost certain that WeakPtrFactory only becomes bound to a thread on the
first call to GetWeakPtr<>.

A WeakPtrFactory has a WeakReferenceOwner which has a WeakRefernece::Flag.  The
Flag is what owns the ThreadChecker.

When you first call GetWeakPtr<>, the Flag is created/initialized and thus you
get bound to the current thread.  When you call InvalidateWeakPtrs(), you
invalidate the weak_reference_owner_, which also drops the reference to the
Flag, and thus unbinds from the current thread.

So, if you can guarantee that the containing object never deletes the factory
until all pointers have been invalidated on the appropraite thread, it should be
safe to not use a scoped_ptr.

You'll find out fast if we're wrong I think. :)

[Ken Russell (switch to Gerrit), 2011-11-16 03:10:39]

apatrick should really review this instead of me.

[dcheng, 2011-11-16 20:09:12]

Converted scoped_ptr<base::WeakPtrFactory> -> base::WeakPtrFactory. Per ajwong's
comments, I don't think the scoped_ptr was doing anything except adding
complexity.

[awong, 2011-11-16 20:14:51]

LGTM

[apatrick_chromium, 2011-11-17 22:27:18]

Per GpuWatchdogThread, maybe things have changed but it was not possible before
for ScopedMethodFactory to be created on the main thread and then used on
another thread. If that is no longer the case then LGTM.

[awong, 2011-11-17 22:28:43]

IIRC, someone went and messed with that a few months back.  The code
*reads* like it's safe right now so I think it's good.

-Albert


On Thu, Nov 17, 2011 at 2:27 PM, <apatrick@chromium.org> wrote:

> Per GpuWatchdogThread, maybe things have changed but it was not possible
> before
> for ScopedMethodFactory to be created on the main thread and then used on
> another thread. If that is no longer the case then LGTM.
>
>
http://codereview.chromium.**org/8572047/<http://codereview.chromium.org/8572...
>

[dcheng, 2011-11-17 22:35:07]

On 2011/11/17 22:28:43, awong wrote:
> IIRC, someone went and messed with that a few months back.  The code
> *reads* like it's safe right now so I think it's good.
> 
> -Albert
> 
> 
> On Thu, Nov 17, 2011 at 2:27 PM, <mailto:apatrick@chromium.org> wrote:
> 
> > Per GpuWatchdogThread, maybe things have changed but it was not possible
> > before
> > for ScopedMethodFactory to be created on the main thread and then used on
> > another thread. If that is no longer the case then LGTM.
> >
> >
>
http://codereview.chromium.**org/8572047/%3Chttp://codereview.chromium.org/85...>
> >

If something explodes, I will revert and convert it back to
scoped_ptr<base::WeakPtrFactory>.

[commit-bot: I haz the power, 2011-11-17 22:37:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/8572047/6001

[commit-bot: I haz the power, 2011-11-17 23:44:00]

Try job failure for 8572047-6001 (retry) on linux_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...