Privatize StrongBinding lifetime management

Privatize StrongBinding lifetime management

Makes StrongBinding's ctor and dtor private and
simplifies ownership semantics. Now StrongBinding
explicitly owns its bound impl and nobody owns
a StrongBinding except itself.

Adds a helper mojo::MakeStrongBinding for binding
requests to new impls.

BUG=None
R=yzshen@chromium.org
TBR=ben@chromium.org

Committed: https://crrev.com/8e66a08d9a81cac06ebe4e8933cd02f751fb65fd
Cr-Commit-Position: refs/heads/master@{#418124}

[Ken Rockot(use gerrit already), 2016-09-09 13:51:19]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2016-09-09 13:53:33]

PTAL - I've only updated bindings tests for now but can go through and update
all other uses if we're OK with the interface.

[yzshen1, 2016-09-09 16:21:30]

LGTM! Thanks!

https://codereview.chromium.org/2326913003/diff/20001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/tests/binding_unittest.cc (left):

https://codereview.chromium.org/2326913003/diff/20001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/tests/binding_unittest.cc:494: // Tests that even when
the implementation object owns the StrongBinding, that
This pattern seems pretty weird. +1 that StrongBinding no longer supports this!

If some code needs this, I think it should using Binding and does the lifetime
management explicitly by itself.

[Ken Rockot(use gerrit already), 2016-09-09 21:49:34]

Description was changed from

==========
Privatize StrongBinding lifetime management

Makes StrongBinding's ctor and dtor private and
simplifies ownership semantics. Now StrongBinding
explicitly owns its bound impl and nobody owns
a StrongBinding except itself.

Adds a helper mojo::MakeStrongBinding for binding
requests to new impls.

BUG=None
R=yzshen@chromium.org
==========

to

==========
Privatize StrongBinding lifetime management

Makes StrongBinding's ctor and dtor private and
simplifies ownership semantics. Now StrongBinding
explicitly owns its bound impl and nobody owns
a StrongBinding except itself.

Adds a helper mojo::MakeStrongBinding for binding
requests to new impls.

BUG=None
R=yzshen@chromium.org
TBR=ben@chromium.org
==========

[Ken Rockot(use gerrit already), 2016-09-09 21:52:23]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 21:52:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 22:14:57]

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

[commit-bot: I haz the power, 2016-09-09 22:14:58]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Ken Rockot(use gerrit already), 2016-09-09 23:04:42]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 23:05:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 23:09:12]

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

[commit-bot: I haz the power, 2016-09-09 23:09:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Rockot(use gerrit already), 2016-09-09 23:27:41]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 23:28:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 23:31:29]

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

[commit-bot: I haz the power, 2016-09-09 23:31:31]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Rockot(use gerrit already), 2016-09-09 23:39:39]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 23:40:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-10 00:46:08]

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

[commit-bot: I haz the power, 2016-09-10 00:46:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-09-12 15:45:27]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 15:45:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-09-12 16:24:43]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 16:24:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 16:51:12]

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

[commit-bot: I haz the power, 2016-09-12 16:51:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-09-12 16:56:35]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 16:57:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 17:02:54]

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

[commit-bot: I haz the power, 2016-09-12 17:02:56]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-09-12 19:18:25]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 19:18:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-09-12 20:45:37]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-12 20:45:39]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-12 20:45:40]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2326913003/#ps130001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-12 20:46:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 20:57:16]

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

[commit-bot: I haz the power, 2016-09-12 20:57:18]

Failed to apply patch for
components/contextual_search/renderer/overlay_js_render_frame_observer.cc:
While running git apply --index -3 -p1;
  error: patch failed:
components/contextual_search/renderer/overlay_js_render_frame_observer.cc:35
  Falling back to three-way merge...
  Applied patch to
'components/contextual_search/renderer/overlay_js_render_frame_observer.cc' with
conflicts.
  U components/contextual_search/renderer/overlay_js_render_frame_observer.cc

Patch:      
components/contextual_search/renderer/overlay_js_render_frame_observer.cc
Index: components/contextual_search/renderer/overlay_js_render_frame_observer.cc
diff --git
a/components/contextual_search/renderer/overlay_js_render_frame_observer.cc
b/components/contextual_search/renderer/overlay_js_render_frame_observer.cc
index
2991d2f1d54316c0a1bba02266cced0f5f595f34..2eabd6a0bef80bb103ed390cff4b96ade2552022
100644
--- a/components/contextual_search/renderer/overlay_js_render_frame_observer.cc
+++ b/components/contextual_search/renderer/overlay_js_render_frame_observer.cc
@@ -10,6 +10,7 @@
 #include "components/contextual_search/renderer/contextual_search_wrapper.h"
 #include
"components/contextual_search/renderer/overlay_page_notifier_service_impl.h"
 #include "content/public/renderer/render_frame.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/shell/public/cpp/interface_registry.h"
 #include "v8/include/v8.h"
 
@@ -35,8 +36,9 @@ void OverlayJsRenderFrameObserver::RegisterMojoInterface() {
 
 void OverlayJsRenderFrameObserver::CreateOverlayPageNotifierService(
     mojo::InterfaceRequest<mojom::OverlayPageNotifierService> request) {
-  // This is strongly bound to and owned by the pipe.
-  new OverlayPageNotifierServiceImpl(this, std::move(request));
+  mojo::MakeStrongBinding(
+      base::MakeUnique<OverlayPageNotifierServiceImpl>(this),
+      std::move(request));
 }
 
 void OverlayJsRenderFrameObserver::SetIsContextualSearchOverlay() {

[Ken Rockot(use gerrit already), 2016-09-12 21:01:23]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 21:02:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-09-12 21:33:22]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-12 21:33:25]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-12 21:33:26]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2326913003/#ps150001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-12 21:33:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 00:48:44]

Committed patchset #8 (id:150001)

[commit-bot: I haz the power, 2016-09-13 00:50:05]

Description was changed from

==========
Privatize StrongBinding lifetime management

Makes StrongBinding's ctor and dtor private and
simplifies ownership semantics. Now StrongBinding
explicitly owns its bound impl and nobody owns
a StrongBinding except itself.

Adds a helper mojo::MakeStrongBinding for binding
requests to new impls.

BUG=None
R=yzshen@chromium.org
TBR=ben@chromium.org
==========

to

==========
Privatize StrongBinding lifetime management

Makes StrongBinding's ctor and dtor private and
simplifies ownership semantics. Now StrongBinding
explicitly owns its bound impl and nobody owns
a StrongBinding except itself.

Adds a helper mojo::MakeStrongBinding for binding
requests to new impls.

BUG=None
R=yzshen@chromium.org
TBR=ben@chromium.org

Committed: https://crrev.com/8e66a08d9a81cac06ebe4e8933cd02f751fb65fd
Cr-Commit-Position: refs/heads/master@{#418124}
==========

[commit-bot: I haz the power, 2016-09-13 00:50:07]

Patchset 8 (id:??) landed as
https://crrev.com/8e66a08d9a81cac06ebe4e8933cd02f751fb65fd
Cr-Commit-Position: refs/heads/master@{#418124}