Mojo C++ Bindings: Introduce mojo::SupportsStrongBinding

mojo/public/cpp/bindings/strong_binding.h

Index: mojo/public/cpp/bindings/strong_binding.h
diff --git a/mojo/public/cpp/bindings/strong_binding.h b/mojo/public/cpp/bindings/strong_binding.h
index 99d10956c6f040e2835ff9f705f713f38486a641..f2efeb293da39ca1f739429e8644718cda50bb88 100644
--- a/mojo/public/cpp/bindings/strong_binding.h
+++ b/mojo/public/cpp/bindings/strong_binding.h
@@ -31,6 +31,33 @@ class StrongBinding;
 template <typename Interface>
 using StrongBindingPtr = base::WeakPtr<StrongBinding<Interface>>;
 
+// Interface implementations must inherit this in order to support being
+// strongly bound to a message pipe.
+template <typename Interface>
+class SupportsStrongBinding : public Interface {
+ public:
+  virtual ~SupportsStrongBinding() {
+    if (binding_) {
+      // Dissociate |this| from the binding before closing it.
+      ignore_result(binding_->Release().release());
+
+      // Deletes |*binding_|.
+      binding_->Close();
+    }
+  }
+
+ private:
+  friend class StrongBinding<Interface>;
+
+  void set_binding(const StrongBindingPtr<Interface>& binding) {
+    binding_ = binding;
+  }
+
+  StrongBindingPtr<Interface> binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(SupportsStrongBinding);
+};
+
 // This connects an interface implementation strongly to a pipe. When a
 // connection error is detected the implementation is deleted.
 //
@@ -46,12 +73,19 @@ class StrongBinding {
   // Create a new StrongBinding instance. The instance owns itself, cleaning up
   // only in the event of a pipe connection error. Returns a WeakPtr to the new
   // StrongBinding instance.
+  //
+  // Note that it's also safe for |impl| to delete itself at any time.
+  template <typename Impl>
   static StrongBindingPtr<Interface> Create(

[dcheng, 2016/11/21 10:42:12]

Would this still need to return a weak pointer to the strong binding once we fix
up existing usage of this return value?

I think we wouldn't need it with this model?

-      std::unique_ptr<Interface> impl,
+      std::unique_ptr<Impl> impl,
       InterfaceRequest<Interface> request) {
+    SupportsStrongBinding<Interface>* raw_impl = impl.get();
     StrongBinding* binding =
         new StrongBinding(std::move(impl), std::move(request));
-    return binding->weak_factory_.GetWeakPtr();
+    StrongBindingPtr<Interface> weak_binding =
+        binding->weak_factory_.GetWeakPtr();
+    raw_impl->set_binding(weak_binding);
+    return weak_binding;
   }
 
   // Note: The error handler must not delete the interface implementation.
@@ -74,6 +108,10 @@ class StrongBinding {
   // Forces the binding to close. This destroys the StrongBinding instance.
   void Close() { delete this; }
 
+  std::unique_ptr<Interface> Release() WARN_UNUSED_RESULT {
+    return std::move(impl_);
+  }
+
   Interface* impl() { return impl_.get(); }
 
   // Exposed for testing, should not generally be used.
@@ -95,7 +133,12 @@ class StrongBinding {
         base::Bind(&StrongBinding::OnConnectionError, base::Unretained(this)));
   }
 
-  ~StrongBinding() {}
+  ~StrongBinding() {
+    // Ensure any WeakPtrs are invalidated *before* |impl_| is destroyed,
+    // otherwise the |impl_|'s destructor will attempt to reenter this object to
+    // Close it.
+    weak_factory_.InvalidateWeakPtrs();
+  }
 
   void OnConnectionError(uint32_t custom_reason,
                          const std::string& description) {
@@ -115,6 +158,7 @@ class StrongBinding {
   DISALLOW_COPY_AND_ASSIGN(StrongBinding);
 };
 
+// Convenient helper wrapping StrongBinding::Create. See above.
 template <typename Interface, typename Impl>
 StrongBindingPtr<Interface> MakeStrongBinding(
     std::unique_ptr<Impl> impl,