Fix policy invalidator lifecycle bugs for Android.

chrome/browser/policy/cloud/cloud_policy_invalidator.h

Index: chrome/browser/policy/cloud/cloud_policy_invalidator.h
diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator.h b/chrome/browser/policy/cloud/cloud_policy_invalidator.h
index 2575d1be371d67a6b97a3248e6e7dcc59eec0960..76e19b9912f9bdb72c0e77f26c669b5a2611cb6d 100644
--- a/chrome/browser/policy/cloud/cloud_policy_invalidator.h
+++ b/chrome/browser/policy/cloud/cloud_policy_invalidator.h
@@ -15,8 +15,6 @@
 #include "chrome/browser/policy/cloud/cloud_policy_store.h"
 #include "sync/notifier/invalidation_handler.h"
 
-class Profile;
-
 namespace base {
 class SequencedTaskRunner;
 }
@@ -42,6 +40,16 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   static const int kMaxFetchDelayMin;
   static const int kMaxFetchDelayMax;
 
+  // A callback used to get a pointer to an invalidation service.
+  typedef base::Callback<invalidation::InvalidationService*()>
+      GetInvalidationService;
+
+  // |get_invalidation_service| is a callback which gets a pointer to the
+  // invalidation service. The pointer returned is not owned by this object and
+  // must remain valid until Shutdown is called. A callback is passed instead of
+  // of the raw pointer so that the InvalidationService can be lazily
+  // initialized. This prevents attempting to access the service before it is
+  // ready or if it is not needed.

[Mattias Nissler (ping if slow), 2013/08/29 16:02:30]

Can you tell me more about the not-ready case? It seems that should be covered
by declaring dependencies properly?

[Steve Condie, 2013/08/30 00:16:35]

The requirements to turn on the invalidator are
1) The invalidation service can be created (which needs to be after profile
creation is complete; cannot happen at profile-keyed-service creation time).
2) The cloud policy refresh scheduler is running (alternatively, we could have
some way of storing an invalidation until the refresh scheduler is running, but
I think that would be more complex).

I think in the latest patch set waiting for these dependencies is implemented in
a fairly clean way. 1) is fulfilled by waiting until the profile creation
notification to call the Initialize method and 2) is fulfilled by registering a
CloudPolicyCore observer and starting/stopping the invalidator when the refresh
scheduler is started/stopped.

[Mattias Nissler (ping if slow), 2013/09/02 11:54:52]

Why?

   // |invalidation_handler| handles invalidations provided by this object and
   // must remain valid until Shutdown is called.
   // |store| is cloud policy store. It must remain valid until Shutdown is
@@ -49,25 +57,29 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   // |task_runner| is used for scheduling delayed tasks. It must post tasks to
   // the main policy thread.
   CloudPolicyInvalidator(
+      const GetInvalidationService& get_invalidation_service,
       CloudPolicyInvalidationHandler* invalidation_handler,
       CloudPolicyStore* store,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
   virtual ~CloudPolicyInvalidator();
 
-  // Initializes the invalidator with the given profile. The invalidator uses
-  // the profile to get a reference to the profile's invalidation service if
-  // needed. Both the profile and the invalidation service must remain valid
-  // until Shutdown is called. An Initialize method must only be called once.
-  void InitializeWithProfile(Profile* profile);
-
-  // Initializes the invalidator with the invalidation service. It must remain
-  // valid until Shutdown is called. An Initialize method must only be called
-  // once.
-  void InitializeWithService(
-      invalidation::InvalidationService* invalidation_service);
-
-  // Shuts down and disables invalidations. It must be called before the object
-  // is destroyed.
+  // Starts the invalidator. This method should be called after the invalidation
+  // handler is ready to receive invalidations and the invalidation service is
+  // ready for use. This method must only be called when the object is in the
+  // stopped state (the initial state) and moves the object into the started
+  // state.
+  void Start();
+
+  // Stops the invalidator. This method should be only called when the cloud
+  // policy is being discarded because the user explicitly signed out; it should
+  // not be called for a normal session shutdown. This method must only be
+  // called when the object is in the started state and moves the object into
+  // the stopped state.
+  void Stop();
+
+  // Shuts down the invalidator. This method must be called before the object
+  // is destroyed. It can be called when the object is in either the started or
+  // stopped state and moves the object into the shut-down state.
   void Shutdown();
 
   // Whether the invalidator currently has the ability to receive invalidations.
@@ -85,19 +97,7 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE;
   virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE;
 
- protected:
-  // Allows subclasses to create a weak pointer to the object. The pointer
-  // should only be used to call one of the Initialize methods, as after the
-  // object is initialized weak pointers may be invalidated at any time.
-  base::WeakPtr<CloudPolicyInvalidator> GetWeakPtr();
-
  private:
-  // Initialize the invalidator.
-  void Initialize();
-
-  // Returns whether an Initialize method has been called.
-  bool IsInitialized();
-
   // Handle an invalidation to the policy.
   void HandleInvalidation(const syncer::Invalidation& invalidation);
 
@@ -131,7 +131,18 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   // when a policy is stored.
   int GetPolicyRefreshMetric();
 
-  // The handler for invalidations provded by this object.
+  // The state of the object.
+  enum State {
+    STOPPED,
+    STARTED,
+    SHUT_DOWN
+  };
+  State state_;
+
+  // The callback invoked to lazily retrieve the invalidation service.
+  GetInvalidationService get_invalidation_service_;
+
+  // The handler for invalidations provided by this object.
   CloudPolicyInvalidationHandler* invalidation_handler_;
 
   // The cloud policy store.
@@ -140,10 +151,6 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   // Schedules delayed tasks.
   const scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
-  // The profile which will be used to get a reference to an invalidation
-  // service.
-  Profile* profile_;
-
   // The invalidation service.
   invalidation::InvalidationService* invalidation_service_;