Decoupled the SchemaRegistryService from SchemaRegistry.

components/policy/core/common/schema_registry.h

Index: components/policy/core/common/schema_registry.h
diff --git a/components/policy/core/common/schema_registry.h b/components/policy/core/common/schema_registry.h
index b9f7fa8777e02f181d70d6441be6c52755a40589..6df4db2b19a7565337e8ce2a619990584e37afcc 100644
--- a/components/policy/core/common/schema_registry.h
+++ b/components/policy/core/common/schema_registry.h
@@ -43,6 +43,16 @@ class POLICY_EXPORT SchemaRegistry : public base::NonThreadSafe {
     virtual ~Observer();
   };
 
+  // This observer is only meant to be used by subclasses.

[bartfab (slow), 2014/06/20 14:48:01]

Why make it public then?

[Joao da Silva, 2014/06/20 15:30:14]

Ha :-) It was protected initially, but it doesn't work as you'd expect.

Basically ForwardingSchemaRegistry can't inherit from this if this is protected,
even if it inherits from SchemaRegistry.

Same for the AddInternalObserver() method: you can call that method on |this|
but you can't call it on other objects of the superclass (you can call it on
objects of the same class though).

[bartfab (slow), 2014/06/20 15:44:18]

Makes sense, when you take the meaning of "protected" absolutely literally (as I
guess compilers do). LGTM as-is then. I wonder though whether a cryptic name
like InternalObserver is really necessary/useful here. It could be called
something like LifetimeObserver. It is mostly useful for subclasses but if some
other class ever decides it wants to track this class' lifetime, why not let it?

[Joao da Silva, 2014/06/20 15:47:43]

Sounds reasonable. The current name hints at its current usage; let's rename it
when we get an external observer then.

+  class POLICY_EXPORT InternalObserver {
+   public:
+    // Invoked when |registry| is about to be destroyed.
+    virtual void OnSchemaRegistryShuttingDown(SchemaRegistry* registry) = 0;
+
+   protected:
+    virtual ~InternalObserver();
+  };
+
   SchemaRegistry();
   virtual ~SchemaRegistry();
 
@@ -69,7 +79,8 @@ class POLICY_EXPORT SchemaRegistry : public base::NonThreadSafe {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  bool HasObservers() const;
+  void AddInternalObserver(InternalObserver* observer);

[bartfab (slow), 2014/06/20 14:48:01]

Why are these public if they are meant for subclasses only?

[Joao da Silva, 2014/06/20 15:30:14]

See above

+  void RemoveInternalObserver(InternalObserver* observer);
 
  protected:
   void Notify(bool has_new_schemas);
@@ -78,30 +89,35 @@ class POLICY_EXPORT SchemaRegistry : public base::NonThreadSafe {
 
  private:
   ObserverList<Observer, true> observers_;
+  ObserverList<InternalObserver, true> internal_observers_;
   bool domains_ready_[POLICY_DOMAIN_SIZE];
 
   DISALLOW_COPY_AND_ASSIGN(SchemaRegistry);
 };
 
 // A registry that combines the maps of other registries.
-class POLICY_EXPORT CombinedSchemaRegistry : public SchemaRegistry,
-                                             public SchemaRegistry::Observer {
+class POLICY_EXPORT CombinedSchemaRegistry
+    : public SchemaRegistry,
+      public SchemaRegistry::Observer,
+      public SchemaRegistry::InternalObserver {
  public:
   CombinedSchemaRegistry();
   virtual ~CombinedSchemaRegistry();
 
   void Track(SchemaRegistry* registry);
-  void Untrack(SchemaRegistry* registry);
 
+  // SchemaRegistry:
   virtual void RegisterComponents(PolicyDomain domain,
                                   const ComponentMap& components) OVERRIDE;
-
   virtual void UnregisterComponent(const PolicyNamespace& ns) OVERRIDE;
 
+  // SchemaRegistry::Observer:
   virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE;
-
   virtual void OnSchemaRegistryReady() OVERRIDE;
 
+  // SchemaRegistry::InternalObserver:
+  virtual void OnSchemaRegistryShuttingDown(SchemaRegistry* registry) OVERRIDE;
+
  private:
   void Combine(bool has_new_schemas);
 
@@ -111,6 +127,35 @@ class POLICY_EXPORT CombinedSchemaRegistry : public SchemaRegistry,
   DISALLOW_COPY_AND_ASSIGN(CombinedSchemaRegistry);
 };
 
+// A registry that wraps another schema registry.
+class POLICY_EXPORT ForwardingSchemaRegistry
+    : public SchemaRegistry,
+      public SchemaRegistry::Observer,
+      public SchemaRegistry::InternalObserver {
+ public:
+  // This registry will stop updating its SchemaMap when |wrapped| is
+  // destroyed.
+  explicit ForwardingSchemaRegistry(SchemaRegistry* wrapped);
+  virtual ~ForwardingSchemaRegistry();
+
+  // SchemaRegistry:
+  virtual void RegisterComponents(PolicyDomain domain,
+                                  const ComponentMap& components) OVERRIDE;
+  virtual void UnregisterComponent(const PolicyNamespace& ns) OVERRIDE;
+
+  // SchemaRegistry::Observer:
+  virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE;
+  virtual void OnSchemaRegistryReady() OVERRIDE;
+
+  // SchemaRegistry::InternalObserver:
+  virtual void OnSchemaRegistryShuttingDown(SchemaRegistry* registry) OVERRIDE;
+
+ private:
+  SchemaRegistry* wrapped_;
+
+  DISALLOW_COPY_AND_ASSIGN(ForwardingSchemaRegistry);
+};
+
 }  // namespace policy
 
 #endif  // COMPONENTS_POLICY_CORE_COMMON_SCHEMA_REGISTRY_H_