Allow cross dependencies between BCKSF and BSKSF

components/keyed_service/core/keyed_service_base_factory.h

Index: components/keyed_service/core/keyed_service_base_factory.h
diff --git a/components/keyed_service/core/keyed_service_base_factory.h b/components/keyed_service/core/keyed_service_base_factory.h
index a22e9405808504cf2e20744f310cc5717b84af0c..b277fff50cbd416dfccdf9333d6f772d79b04f54 100644
--- a/components/keyed_service/core/keyed_service_base_factory.h
+++ b/components/keyed_service/core/keyed_service_base_factory.h
@@ -57,14 +57,14 @@ class KEYED_SERVICE_EXPORT KeyedServiceBaseFactory
 #ifndef NDEBUG
   // Debugging assertion that will NOTREACHED() is |context| is considered
   // stale. Should be used by subclasses when accessing |context|.
-  void AssertContextWasntDestroyed(const base::SupportsUserData* context) const;
+  void AssertContextWasntDestroyed(base::SupportsUserData* context) const;
 
   // Marks |context| as live (i.e., not stale). This method can be called as a
   // safeguard against |AssertContextWasntDestroyed()| checks going off due to
   // |context| aliasing am instance from a prior test (i.e., 0xWhatever might
   // be created, be destroyed, and then a new object might be created at
   // 0xWhatever).
-  void MarkContextLiveForTesting(const base::SupportsUserData* context);
+  void MarkContextLiveForTesting(base::SupportsUserData* context);
 #endif
 
   // Calls RegisterProfilePrefs() after doing house keeping required to work
@@ -72,13 +72,13 @@ class KEYED_SERVICE_EXPORT KeyedServiceBaseFactory
   // TODO(gab): This method can be replaced by RegisterProfilePrefs() directly
   // once RegisterUserPrefsOnContextForTest() is phased out.
   void RegisterPrefsIfNecessaryForContext(
-      const base::SupportsUserData* context,
+      base::SupportsUserData* context,
       user_prefs::PrefRegistrySyncable* registry);
 
   // Returns the |user_pref::PrefRegistrySyncable| associated with |context|.
   // The way they are associated is controlled by the embedder.
-  virtual user_prefs::PrefRegistrySyncable* GetAssociatedPrefRegistry(
-      base::SupportsUserData* context) const = 0;
+  user_prefs::PrefRegistrySyncable* GetAssociatedPrefRegistry(
+      base::SupportsUserData* context) const;
 
   // Finds which context (if any) to use.
   virtual base::SupportsUserData* GetContextToUse(
@@ -109,10 +109,30 @@ class KEYED_SERVICE_EXPORT KeyedServiceBaseFactory
   virtual void ContextDestroyed(base::SupportsUserData* context);
 
   // Returns whether the preferences have been registered on this context.
-  bool ArePreferencesSetOn(const base::SupportsUserData* context) const;
+  bool ArePreferencesSetOn(base::SupportsUserData* context) const;
 
   // Mark context has having preferences registered.
-  void MarkPreferencesSetOn(const base::SupportsUserData* context);
+  void MarkPreferencesSetOn(base::SupportsUserData* context);
+
+  // Returns the underlying context corresponding to |context|. Used to allow
+  // cross dependencies between BrowserContextKeyedServiceFactories and
+  // BrowserStateKeyedServiceFactories during the upstreaming of iOS. Wrapper
+  // around GetUnderlyingContextInternal() that assert the returned |context|
+  // is not null.
+  // TODO(ios): remove this method and all usage of the methods once iOS code
+  // only use BrowserStateKeyedServiceFactory, http://crbug.com/478763
+  base::SupportsUserData* GetUnderlyingContext(
+      base::SupportsUserData* context) const;
+
+  // Returns the original context corresponding to |context|. Used to allow
+  // cross dependencies between BrowserContextKeyedServiceFactories and
+  // BrowserStateKeyedServiceFactories during the upstreaming of iOS. Wrapper
+  // around GetOriginalContextInternal() that assert the returned |context|
+  // is not null.
+  // TODO(ios): remove this method and all usage of the methods once iOS code

[droger, 2015/04/21 14:19:17]

Can we merge these comments and give a bit more details?
I suggest:

Return the underlying context and the original context corresponding to
|context|.
Used to allow cross dependencies between BrowserContextKeyedServiceFactories and
BrowserStateKeyedServiceFactories during the upstreaming of iOS.
The original context is the context used to compute the dependencies, and the
underlying context is the context that is exposed in the factories public APIs.
These methods do nothing by default (i.e. they return the context that was
passed in), but iOS provides
hooks to override this behavior temporarily while the upstreaming is in
progress.
TODO(ios): remove this method and all usage of the methods once iOS code
only use BrowserStateKeyedServiceFactory, http://crbug.com/478763

+  // only use BrowserStateKeyedServiceFactory, http://crbug.com/478763
+  base::SupportsUserData* GetOriginalContext(

[droger, 2015/04/21 14:19:17]

I don't really like "Original" and "Underlying".
Also if I am not mistaken, the "Original" context is the one used by the
dependency manager, and since the dependency manager sits at the bottom of the
class hierarchy, I wonder if it would actually make more sense to swap the two
names.

Some other suggestions, maybe less confusing, but maybe not:
- "InternalContext" / "PublicContext"
- "DependencyContext" / "FactorySpecificContext"
- "BaseContext" / ???

+      base::SupportsUserData* context) const;
 
  private:
   friend class DependencyManager;
@@ -121,6 +141,22 @@ class KEYED_SERVICE_EXPORT KeyedServiceBaseFactory
   // by all the factories of a given type. Unit tests will use their own copy.
   DependencyManager* dependency_manager_;
 
+  // Returns the underlying context corresponding to |context|. Used to allow
+  // cross dependencies between BrowserContextKeyedServiceFactories and
+  // BrowserStateKeyedServiceFactories during the upstreaming of iOS.
+  // TODO(ios): remove this method and all usage of the methods once iOS code
+  // only use BrowserStateKeyedServiceFactory, http://crbug.com/478763

[droger, 2015/04/21 14:19:17]

Add something like:
Subclasses must implement these methods to convert between the context used for
dependencies and the context exposed by the concrete factories.
It is expected that all platforms expect iOS simply return the context that is
passed in.

+  virtual base::SupportsUserData* GetUnderlyingContextInternal(
+      base::SupportsUserData* context) const = 0;
+
+  // Returns the original context corresponding to |context|. Used to allow
+  // cross dependencies between BrowserContextKeyedServiceFactories and
+  // BrowserStateKeyedServiceFactories during the upstreaming of iOS.
+  // TODO(ios): remove this method and all usage of the methods once iOS code
+  // only use BrowserStateKeyedServiceFactory, http://crbug.com/478763
+  virtual base::SupportsUserData* GetOriginalContextInternal(
+      base::SupportsUserData* context) const = 0;
+
   // Registers any preferences used by this service.
   virtual void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry) {}
 
@@ -135,7 +171,7 @@ class KEYED_SERVICE_EXPORT KeyedServiceBaseFactory
   virtual void CreateServiceNow(base::SupportsUserData* context) = 0;
 
   // Contexts that have this service's preferences registered on them.
-  std::set<const base::SupportsUserData*> registered_preferences_;
+  std::set<base::SupportsUserData*> registered_preferences_;
 
 #if !defined(NDEBUG)
   // A static string passed in to the constructor. Should be unique across all