Reland of Mojo C++ Bindings: Additional support for associated binding sets...

Reland of Mojo C++ Bindings: Additional support for associated binding sets (patchset #1 id:1 of https://codereview.chromium.org/2324623003/ )

Reason for revert:
It seems like linker just occasionally ran out of memory on the buildbot. The CL seems to be innocent.

Original issue's description:
> Revert of Mojo C++ Bindings: Additional support for associated binding sets (patchset #2 id:20001 of https://codereview.chromium.org/2309513002/ )
>
> Reason for revert:
> This broke compilation on Win x64 builder:
>
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20x64%20Builder/builds/94511
>
> Original issue's description:
> > Mojo C++ Bindings: Additional support for associated binding sets
> >
> > Implements three orthogonal changes to the public bindings API:
> >
> >  1. Adds the ability to remove bindings from a binding set. Uses
> >     a new mojo::BindingId integer alias to identify added bindings.
> >
> >  2. Adds the ability to set a pre-dispatch hook on binding sets.
> >     In some cases simply making the dispatch_context() available
> >     is insufficient, for example when a consumer owns multiple
> >     binding sets and wants a common path for establishing context
> >     among any of them.
> >
> >  3. Adds mojo::GetDummyProxy, a helper that can bind an
> >     AssociatedInterfacePtr<T> so that it has some valid internal
> >     state and silently drops all outgoing messages. Unlike
> >     regular message pipe interfaces, without this feature
> >     associated interfaces are difficult to stub out in test
> >     environments.
> >
> > Part a series of CLs to enable and demonstrate WebContents associated
> > interfaces:
> >
> >   1. https://codereview.chromium.org/2301123004
> >   2. This CL
> >   3. https://codereview.chromium.org/2310563002
> >   4. https://codereview.chromium.org/2310583002
> >
> > BUG=612500
> > R=yzshen@chromium.org
> >
> > Committed: https://crrev.com/4b8bca190cdc55bae81974aa09cc10f422f022fb
> > Cr-Commit-Position: refs/heads/master@{#417126}
>
> TBR=yzshen@chromium.org,rockot@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=612500
>
> Committed: https://crrev.com/c08a3abcbc9d79e2a7031b116d0af9ee92d17459
> Cr-Commit-Position: refs/heads/master@{#417135}

TBR=yzshen@chromium.org,rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612500

Review-Url: https://codereview.chromium.org/2320723003
Cr-Commit-Position: refs/heads/master@{#417142}

mojo/public/cpp/bindings/associated_interface_ptr.h

@@ -18,6 +18,8 @@
 #include "mojo/public/cpp/bindings/associated_interface_ptr_info.h"
 #include "mojo/public/cpp/bindings/associated_interface_request.h"
 #include "mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h"
+#include "mojo/public/cpp/bindings/lib/multiplex_router.h"
+#include "mojo/public/cpp/system/message_pipe.h"
 
 namespace mojo {
 
@@ -209,6 +211,17 @@ AssociatedInterfaceRequest<Interface> GetProxy(
   return request;
 }
 
+// Creates an associated interface proxy which casts its messages into the void.
+template <typename Interface>
+void GetDummyProxyForTesting(AssociatedInterfacePtr<Interface>* proxy) {
+  MessagePipe pipe;
+  scoped_refptr<internal::MultiplexRouter> router =
+      new internal::MultiplexRouter(false, std::move(pipe.handle0),
+                                    base::ThreadTaskRunnerHandle::Get());
+  std::unique_ptr<AssociatedGroup> group = router->CreateAssociatedGroup();
+  GetProxy(proxy, group.get());
+}
+
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_ASSOCIATED_INTERFACE_PTR_H_

mojo/public/cpp/bindings/binding_set.h

@@ -35,11 +35,14 @@ enum class BindingSetDispatchMode {
   WITH_CONTEXT,
 };
 
+using BindingId = size_t;
+
 // Use this class to manage a set of bindings, which are automatically destroyed
 // and removed from the set when the pipe they are bound to is disconnected.
 template <typename Interface, typename BindingType = Binding<Interface>>
 class BindingSet {
  public:
+  using PreDispatchCallback = base::Callback<void(void*)>;
   using Traits = BindingSetTraits<BindingType>;
   using ProxyType = typename Traits::ProxyType;
   using RequestType = typename Traits::RequestType;
@@ -57,25 +60,54 @@ class BindingSet {
     error_handler_ = error_handler;
   }
 
+  // Sets a callback to be invoked immediately before dispatching any message or
+  // error received by any of the bindings in the set. This may only be used
+  // if the set was constructed with |BindingSetDispatchMode::WITH_CONTEXT|.
+  // |handler| is passed the context associated with the binding which received
+  // the message or event about to be dispatched.
+  void set_pre_dispatch_handler(const PreDispatchCallback& handler) {
+    DCHECK(SupportsContext());
+    pre_dispatch_handler_ = handler;
+  }
+
   // Adds a new binding to the set which binds |request| to |impl|. If |context|
   // is non-null, dispatch_context() will reflect this value during the extent
   // of any message or error dispatch targeting this specific binding. Note that
   // |context| may only be non-null if the BindingSet was constructed with
   // |BindingSetDispatchMode::WITH_CONTEXT|.
-  void AddBinding(Interface* impl,
-                  RequestType request,
-                  void* context = nullptr) {
+  BindingId AddBinding(Interface* impl,
+                     RequestType request,
+                     void* context = nullptr) {
     DCHECK(!context || SupportsContext());
+    BindingId id = next_binding_id_++;
+    DCHECK_GE(next_binding_id_, 0u);
     std::unique_ptr<Entry> entry =
-        base::MakeUnique<Entry>(impl, std::move(request), this, context);
-    bindings_.insert(std::make_pair(entry.get(), std::move(entry)));
+        base::MakeUnique<Entry>(impl, std::move(request), this, id, context);
+    bindings_.insert(std::make_pair(id, std::move(entry)));
+    return id;
+  }
+
+  // Removes a binding from the set. Note that this is safe to call even if the
+  // binding corresponding to |id| has already been removed.
+  //
+  // Returns |true| if the binding was removed and |false| if it didn't exist.
+  bool RemoveBinding(BindingId id) {
+    auto it = bindings_.find(id);
+    if (it == bindings_.end())
+      return false;
+    bindings_.erase(it);
+    return true;
   }
 
   // Returns a proxy bound to one end of a pipe whose other end is bound to
-  // |this|.
-  ProxyType CreateInterfacePtrAndBind(Interface* impl) {
+  // |this|. If |id_storage| is not null, |*id_storage| will be set to the ID
+  // of the added binding.
+  ProxyType CreateInterfacePtrAndBind(Interface* impl,
+                                      BindingId* id_storage = nullptr) {
     ProxyType proxy;
-    AddBinding(impl, Traits::GetProxy(&proxy));
+    BindingId id = AddBinding(impl, Traits::GetProxy(&proxy));
+    if (id_storage)
+      *id_storage = id;
     return proxy;
   }
 
@@ -109,9 +141,11 @@ class BindingSet {
     Entry(Interface* impl,
           RequestType request,
           BindingSet* binding_set,
+          BindingId binding_id,
           void* context)
         : binding_(impl, std::move(request)),
           binding_set_(binding_set),
+          binding_id_(binding_id),
           context_(context) {
       if (binding_set->SupportsContext())
         binding_.AddFilter(base::MakeUnique<DispatchFilter>(this));
@@ -147,11 +181,12 @@ class BindingSet {
     void OnConnectionError() {
       if (binding_set_->SupportsContext())
         WillDispatch();
-      binding_set_->OnConnectionError(this);
+      binding_set_->OnConnectionError(binding_id_);
     }
 
     BindingType binding_;
     BindingSet* const binding_set_;
+    const BindingId binding_id_;
     void* const context_;
 
     DISALLOW_COPY_AND_ASSIGN(Entry);
@@ -160,14 +195,16 @@ class BindingSet {
   void SetDispatchContext(void* context) {
     DCHECK(SupportsContext());
     dispatch_context_ = context;
+    if (!pre_dispatch_handler_.is_null())
+      pre_dispatch_handler_.Run(context);
   }
 
   bool SupportsContext() const {
     return dispatch_mode_ == BindingSetDispatchMode::WITH_CONTEXT;
   }
 
-  void OnConnectionError(Entry* entry) {
-    auto it = bindings_.find(entry);
+  void OnConnectionError(BindingId id) {
+    auto it = bindings_.find(id);
     DCHECK(it != bindings_.end());
     bindings_.erase(it);
 
@@ -177,7 +214,9 @@ class BindingSet {
 
   BindingSetDispatchMode dispatch_mode_;
   base::Closure error_handler_;
-  std::map<Entry*, std::unique_ptr<Entry>> bindings_;
+  PreDispatchCallback pre_dispatch_handler_;
+  BindingId next_binding_id_ = 0;
+  std::map<BindingId, std::unique_ptr<Entry>> bindings_;
   void* dispatch_context_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(BindingSet);