Motown: Move responsibility for binding to MediaFactoryService::Product

services/media/factory_service/factory_service.h

Index: services/media/factory_service/factory_service.h
diff --git a/services/media/factory_service/factory_service.h b/services/media/factory_service/factory_service.h
index 62ebc72c9fa01eac06b370eb470b2eecb986370f..95a3854e99ae4d0f2f615d58636627b1d3482234 100644
--- a/services/media/factory_service/factory_service.h
+++ b/services/media/factory_service/factory_service.h
@@ -20,12 +20,12 @@ class MediaFactoryService : public ApplicationDelegate,
                             public MediaFactory {
  public:
   // Provides common behavior for all objects created by the factory service.
-  class Product : public std::enable_shared_from_this<Product> {
+  class ProductBase : public std::enable_shared_from_this<ProductBase> {
    public:
-    virtual ~Product();
+    virtual ~ProductBase();
 
    protected:
-    Product(MediaFactoryService* owner);
+    ProductBase(MediaFactoryService* owner);
 
     // Returns the ApplicationImpl.
     ApplicationImpl* app() {
@@ -43,6 +43,33 @@ class MediaFactoryService : public ApplicationDelegate,
     MediaFactoryService* owner_;
   };
 
+  template <typename Interface>
+  class Product : public ProductBase {
+   public:
+    virtual ~Product() {}
+
+   protected:
+    Product(Interface* impl,
+            InterfaceRequest<Interface> request,
+            MediaFactoryService* owner)
+        : ProductBase(owner), binding_(impl, request.Pass()) {
+      DCHECK(impl);
+      binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
+    }
+
+    // Closes the binding and calls ReleaseFromOwner.
+    void Abort() {
+      if (binding_.is_bound()) {
+        binding_.Close();
+      }
+
+      ReleaseFromOwner();
+    }
+
+   private:
+    Binding<Interface> binding_;
+  };
+
   MediaFactoryService();
 
   ~MediaFactoryService() override;
@@ -80,9 +107,20 @@ class MediaFactoryService : public ApplicationDelegate,
  private:
   BindingSet<MediaFactory> bindings_;
   ApplicationImpl* app_;
-  std::unordered_set<std::shared_ptr<Product>> products_;
+  std::unordered_set<std::shared_ptr<ProductBase>> products_;
 };
 
+// For use by products when handling mojo requests.
+// Checks the condition, and, if it's false, aborts and calls return. Doesn't
+// support stream arguments.
+// TODO(dalesat): Support stream arguments.
+#define RCHECK(condition)                                         \
+  if (!(condition)) {                                             \
+    LOG(ERROR) << "request precondition failed: " #condition "."; \
+    Abort();                                                      \

[kulakowski, 2016/05/05 21:46:18]

Ah. |abort| is overloaded. I think this (and the comment above) would be much
clearer if the name were slightly more verbose. I read the comment as referring
to http://en.cppreference.com/w/cpp/utility/program/abort which would be a much
different change.

[dalesat, 2016/05/05 22:41:06]

Done.

+    return;                                                       \
+  }
+
 }  // namespace media
 }  // namespace mojo