Support binding WeakPtr<> to methods with void return types.

Support binding WeakPtr<> to methods with void return types.

This should give functionality similar to ScopedRunnableMethodFactory.

Note that binding a WeakPtr only make sense with methods with void return types.  If the return type is not void, then it is unclear what the function should return when the pointer is invalidated.

This code adds a compile time assert to check the return type.

BUG=35223
TEST=unittests

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

[awong, 2011-05-14 01:18:22]

Here we go.  I haven't proof read the comments, but this should make things
work.

[willchan no longer on Chromium, 2011-05-14 01:25:09]

LGTM mod nits.

http://codereview.chromium.org/7015064/diff/1001/base/bind_helpers.h
File base/bind_helpers.h (right):

http://codereview.chromium.org/7015064/diff/1001/base/bind_helpers.h#newcode56
base/bind_helpers.h:56: #include "base/memory/weak_ptr.h"
sort alphabetically :)

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h
File base/bind_internal.h (right):

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h#newcode19
base/bind_internal.h:19: #include "base/memory/weak_ptr.h"
Again, sort alphabetically :)

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h#newcode556
base/bind_internal.h:556: if (!invoker->p1_.get()) {
It's non-obvious from reading the code that this is a WeakPtr. 'true' for IsWeak
isn't obvious if you're just reading this specific point. For clarity's sake, I
suggest you break this down.

const WeakPtr& ptr = invoker->p1_;
...

[darin (slow to review), 2011-05-16 04:28:25]

I don't have any additional feedback.  This looks solid.  I wish there were a
way to write unit tests that should fail to compile ;-)

[darin (slow to review), 2011-05-16 04:28:44]

On 2011/05/16 04:28:25, darin wrote:
> I don't have any additional feedback.  This looks solid.  I wish there were a
> way to write unit tests that should fail to compile ;-)

^^^ LGTM modulo nits that Will indicated

[awong, 2011-05-16 20:01:27]

http://codereview.chromium.org/7015064/diff/1001/base/bind_helpers.h
File base/bind_helpers.h (right):

http://codereview.chromium.org/7015064/diff/1001/base/bind_helpers.h#newcode56
base/bind_helpers.h:56: #include "base/memory/weak_ptr.h"
On 2011/05/14 01:25:10, willchan wrote:
> sort alphabetically :)

Done.

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h
File base/bind_internal.h (right):

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h#newcode19
base/bind_internal.h:19: #include "base/memory/weak_ptr.h"
On 2011/05/14 01:25:10, willchan wrote:
> Again, sort alphabetically :)

Done.

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h#newcode556
base/bind_internal.h:556: if (!invoker->p1_.get()) {
On 2011/05/14 01:25:10, willchan wrote:
> It's non-obvious from reading the code that this is a WeakPtr. 'true' for
IsWeak
> isn't obvious if you're just reading this specific point. For clarity's sake,
I
> suggest you break this down.
> 
> const WeakPtr& ptr = invoker->p1_;
> ...

Made a stab at it.  I need to take another pass through and rename some
variables plug break apart expressions.  In particular, this
StorageType::P1Traits::StorageType is confusing to read. :-/

But would like to do that in a different CL.

[willchan no longer on Chromium, 2011-05-16 22:01:26]

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h
File base/bind_internal.h (right):

http://codereview.chromium.org/7015064/diff/1001/base/bind_internal.h#newcode556
base/bind_internal.h:556: if (!invoker->p1_.get()) {
On 2011/05/16 20:01:27, awong wrote:
> On 2011/05/14 01:25:10, willchan wrote:
> > It's non-obvious from reading the code that this is a WeakPtr. 'true' for
> IsWeak
> > isn't obvious if you're just reading this specific point. For clarity's
sake,
> I
> > suggest you break this down.
> > 
> > const WeakPtr& ptr = invoker->p1_;
> > ...
> 
> Made a stab at it.  I need to take another pass through and rename some
> variables plug break apart expressions.  In particular, this
> StorageType::P1Traits::StorageType is confusing to read. :-/
> 
> But would like to do that in a different CL.

OK.

[awong, 2011-05-16 22:35:55]

On 2011/05/16 04:28:25, darin wrote:
> I don't have any additional feedback.  This looks solid.  I wish there were a
> way to write unit tests that should fail to compile ;-)

BTW, inside Google, there *is* actually a way to write unittests that assert
failure of compilation. :)

We could probably make something like that work for Chrome as well if you wanted
to devote a person to it. :P