Fix policy invalidator lifecycle bugs for Android.

chrome/browser/policy/cloud/cloud_policy_invalidator.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_POLICY_CLOUD_CLOUD_POLICY_INVALIDATOR_H_
 #define CHROME_BROWSER_POLICY_CLOUD_CLOUD_POLICY_INVALIDATOR_H_
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "chrome/browser/policy/cloud/cloud_policy_store.h"
 #include "sync/notifier/invalidation_handler.h"
 
-class Profile;
-
 namespace base {
 class SequencedTaskRunner;
 }
 
 namespace invalidation {
 class InvalidationService;
 }
 
 namespace policy {
 
 class CloudPolicyInvalidationHandler;
 
 // Listens for and provides policy invalidations.
 class CloudPolicyInvalidator : public syncer::InvalidationHandler,
                                public CloudPolicyStore::Observer {
  public:
   // The number of minutes to delay a policy refresh after receiving an
   // invalidation with no payload.
   static const int kMissingPayloadDelay;
 
   // The default, min and max values for max_fetch_delay_.
   static const int kMaxFetchDelayDefault;
   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
   // called.
   // |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);
+  // 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();
 
-  // 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);
+  // 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 and disables invalidations. It must be called before the object
-  // is destroyed.
+  // 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.
   bool invalidations_enabled() {
     return invalidations_enabled_;
   }
 
   // syncer::InvalidationHandler:
   virtual void OnInvalidatorStateChange(
       syncer::InvalidatorState state) OVERRIDE;
   virtual void OnIncomingInvalidation(
       const syncer::ObjectIdInvalidationMap& invalidation_map) OVERRIDE;
 
   // CloudPolicyStore::Observer:
   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);
 
   // Update object registration with the invalidation service based on the
   // given policy data.
   void UpdateRegistration(const enterprise_management::PolicyData* policy);
 
   // Registers the given object with the invalidation service.
   void Register(int64 timestamp, const invalidation::ObjectId& object_id);
 
@@ skipping 13 matching lines @@
   // invalidation with a missing payload.
   void RunInvalidateCallback(bool is_missing_payload);
 
   // Acknowledge the latest invalidation.
   void AcknowledgeInvalidation();
 
   // Get the kMetricPolicyRefresh histogram metric which should be incremented
   // 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.
   CloudPolicyStore* store_;
 
   // 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_;
 
   // Whether the invalidator currently has the ability to receive invalidations.
   // This is true if the invalidation service is enabled and the invalidator
   // has registered for a policy object.
   bool invalidations_enabled_;
 
   // Whether the invalidation service is currently enabled.
   bool invalidation_service_enabled_;
@@ skipping 55 matching lines @@
   // This method is called when the invalidator determines that the ability to
   // receive policy invalidations becomes enabled or disabled. The invalidator
   // starts in a disabled state, so the first call to this method is always when
   // the invalidator becomes enabled.
   virtual void OnInvalidatorStateChanged(bool invalidations_enabled) = 0;
 };
 
 }  // namespace policy
 
 #endif  // CHROME_BROWSER_POLICY_CLOUD_CLOUD_POLICY_INVALIDATOR_H_