Mojo: Make overriding OnConnectionError optional

Mojo: Make overriding OnConnectionError optional

Using mojo::Application::AddService<ServiceImpl> results in the system taking care of error handling for you. There's no need to force ServiceImpl to override OnConnectionError in this case.

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

[darin (slow to review), 2014-05-16 22:28:11]

This is something Dave and I agreed to earlier in the week.

[sky, 2014-05-16 22:38:52]

LGTM

https://codereview.chromium.org/289873008/diff/1/mojo/public/cpp/bindings/int...
File mojo/public/cpp/bindings/interface_impl.h (right):

https://codereview.chromium.org/289873008/diff/1/mojo/public/cpp/bindings/int...
mojo/public/cpp/bindings/interface_impl.h:35: // will be silently ignored.
I'm surprised you're going for the silently ignored here. It seems akin to
allowing calling functions on a NULL object.

[darin (slow to review), 2014-05-17 01:48:26]

https://codereview.chromium.org/289873008/diff/1/mojo/public/cpp/bindings/int...
File mojo/public/cpp/bindings/interface_impl.h (right):

https://codereview.chromium.org/289873008/diff/1/mojo/public/cpp/bindings/int...
mojo/public/cpp/bindings/interface_impl.h:35: // will be silently ignored.
On 2014/05/16 22:38:52, sky wrote:
> I'm surprised you're going for the silently ignored here. It seems akin to
> allowing calling functions on a NULL object.

Yeah. I'm torn on this one. I think the idea we talked about of making the
client() and FooPtr auto-null on error is quite interesting. It would sort of be
like using WeakPtr<T>. The risk is that it will lead to crashes as folks might
not realize they need to null check client() and FooPtr instance prior to
calling methods. I'm going to experiment with this idea in a separate CL.

[darin (slow to review), 2014-05-17 01:48:31]

The CQ bit was checked by darin@chromium.org

[commit-bot: I haz the power, 2014-05-17 01:51:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/darin@chromium.org/289873008/1

[commit-bot: I haz the power, 2014-05-17 07:55:01]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-05-17 07:58:42]

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

[commit-bot: I haz the power, 2014-05-17 07:58:42]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)

[darin (slow to review), 2014-05-17 20:50:02]

The CQ bit was checked by darin@chromium.org

[commit-bot: I haz the power, 2014-05-17 20:50:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/darin@chromium.org/289873008/20001

[commit-bot: I haz the power, 2014-05-17 22:39:09]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-18 14:02:45]

Change committed as 271287