C++ bindings: support using a callback as connection error handler.

C++ bindings: support using a callback as connection error handler.

R=qsr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/b39682ef66710fb9842540708f64f9a5c4564a24

[yzshen1, 2015-06-10 17:00:14]

yzshen@chromium.org changed reviewers:
+ qsr@chromium.org

[yzshen1, 2015-06-10 17:00:14]

Hi, Benjamin.

Would you please take a look? Thanks!

[qsr, 2015-06-11 09:37:53]

LGTM with nits.

https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
File mojo/public/cpp/bindings/binding.h (right):

https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
mojo/public/cpp/bindings/binding.h:125: [this]() {
connection_error_handler_.Run(); });
I'm missing something. Why do you need to do this, why not just having
set_connection_error_handler delegate to the router?

[yzshen1, 2015-06-11 16:51:38]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2015-06-11 16:51:45]

https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
File mojo/public/cpp/bindings/binding.h (right):

https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
mojo/public/cpp/bindings/binding.h:125: [this]() {
connection_error_handler_.Run(); });
On 2015/06/11 09:37:53, qsr wrote:
> I'm missing something. Why do you need to do this, why not just having
> set_connection_error_handler delegate to the router?

The current code matches the previous behavior:
* set_connection_error_handler() can be called at any time, before or after
|internal_router_| is created;
* the error handler is preserved after Close() and Unbind().

I understand that this is not consistent with what we do with InterfacePtr. But
I think it is beyond the scope of this CL to try to unify their behavior.

What do you think? Thanks!

[commit-bot: I haz the power, 2015-06-11 16:55:03]

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

[commit-bot: I haz the power, 2015-06-11 16:55:04]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[yzshen1, 2015-06-11 16:58:22]

Committed patchset #1 (id:1) manually as
b39682ef66710fb9842540708f64f9a5c4564a24 (presubmit successful).

[qsr, 2015-06-12 09:52:35]

On 2015/06/11 16:51:45, yzshen1 wrote:
>
https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
> File mojo/public/cpp/bindings/binding.h (right):
> 
>
https://codereview.chromium.org/1174073002/diff/1/mojo/public/cpp/bindings/bi...
> mojo/public/cpp/bindings/binding.h:125: [this]() {
> connection_error_handler_.Run(); });
> On 2015/06/11 09:37:53, qsr wrote:
> > I'm missing something. Why do you need to do this, why not just having
> > set_connection_error_handler delegate to the router?
> 
> The current code matches the previous behavior:
> * set_connection_error_handler() can be called at any time, before or after
> |internal_router_| is created;
> * the error handler is preserved after Close() and Unbind().
> 
> I understand that this is not consistent with what we do with InterfacePtr.
But
> I think it is beyond the scope of this CL to try to unify their behavior.
> 
> What do you think? Thanks!

I think we should unify all of this, but I agree this was not the right CL.