mojo: Make InterfacePtr<> testable in if() statements without .get().

mojo: Make InterfacePtr<> testable in if() statements without .get().

This also goes and changes a lot of "if (obj.get())" to "if (obj)".

BUG=394639
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284269

[darin (slow to review), 2014-07-18 18:13:18]

LGTM!!

[yzshen1, 2014-07-18 19:17:44]

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
File mojo/public/cpp/bindings/interface_ptr.h (right):

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
mojo/public/cpp/bindings/interface_ptr.h:107: // Allow InterfacePtr<> to be used
in boolean expressions, but not
One thing that may worth considering:
I just landed a CL to make InterfacePtr<> do the setup work lazily: Bind() only
records the pipe and waiter; the internal ProxyWithStub/Rounter are constructed
only when the pointer is actually used. (Please see
https://code.google.com/p/chromium/issues/detail?id=394883 for why this is
needed.)

Calling get() is one way of "actually using" the InterfacePtr<> and doing all
the setup.

Therefore, it seems nice if we can avoid get() in Testable(), instead we test
whether it has been bound. What do you think?

[Elliot Glaysher, 2014-07-18 19:32:15]

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
File mojo/public/cpp/bindings/interface_ptr.h (right):

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
mojo/public/cpp/bindings/interface_ptr.h:107: // Allow InterfacePtr<> to be used
in boolean expressions, but not
On 2014/07/18 19:17:43, yzshen1 wrote:
> One thing that may worth considering:
> I just landed a CL to make InterfacePtr<> do the setup work lazily: Bind()
only
> records the pipe and waiter; the internal ProxyWithStub/Rounter are
constructed
> only when the pointer is actually used. (Please see
> https://code.google.com/p/chromium/issues/detail?id=394883 for why this is
> needed.)
> 
> Calling get() is one way of "actually using" the InterfacePtr<> and doing all
> the setup.
> 
> Therefore, it seems nice if we can avoid get() in Testable(), instead we test
> whether it has been bound. What do you think?

What do you propose replacing get() with?

[yzshen1, 2014-07-18 19:38:45]

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
File mojo/public/cpp/bindings/interface_ptr.h (right):

https://codereview.chromium.org/405653003/diff/20001/mojo/public/cpp/bindings...
mojo/public/cpp/bindings/interface_ptr.h:107: // Allow InterfacePtr<> to be used
in boolean expressions, but not
On 2014/07/18 19:32:15, Elliot Glaysher wrote:
> On 2014/07/18 19:17:43, yzshen1 wrote:
> > One thing that may worth considering:
> > I just landed a CL to make InterfacePtr<> do the setup work lazily: Bind()
> only
> > records the pipe and waiter; the internal ProxyWithStub/Rounter are
> constructed
> > only when the pointer is actually used. (Please see
> > https://code.google.com/p/chromium/issues/detail?id=394883 for why this is
> > needed.)
> > 
> > Calling get() is one way of "actually using" the InterfacePtr<> and doing
all
> > the setup.
> > 
> > Therefore, it seems nice if we can avoid get() in Testable(), instead we
test
> > whether it has been bound. What do you think?
> 
> What do you propose replacing get() with?

Maybe an IsBound() method on the internal state, which checks
(handle_.is_valid() || router).

[yzshen1, 2014-07-18 21:18:01]

LGTM Thanks!

[Elliot Glaysher, 2014-07-18 22:11:04]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2014-07-18 22:12:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erg@chromium.org/405653003/40001

[commit-bot: I haz the power, 2014-07-19 00:58:01]

Change committed as 284269