Fix policy invalidator lifecycle bugs for Android.

chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc

 // 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.
 
 #include "chrome/browser/policy/cloud/user_cloud_policy_invalidator.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/policy/cloud/cloud_policy_core.h"
+#include "chrome/browser/invalidation/invalidation_service_factory.h"
 #include "chrome/browser/policy/cloud/cloud_policy_manager.h"
 #include "content/public/browser/notification_source.h"
 
 namespace policy {
 
 UserCloudPolicyInvalidator::UserCloudPolicyInvalidator(
     Profile* profile,
     CloudPolicyManager* policy_manager)
     : CloudPolicyInvalidator(
-          policy_manager,
-          policy_manager->core()->store(),
+          policy_manager->core(),
           base::MessageLoopProxy::current()),
-      profile_(profile),
-      policy_manager_(policy_manager) {
+      profile_(profile) {
   DCHECK(profile);
 
-  // Register for notification that profile creation is complete.
+  // Register for notification that profile creation is complete. The
+  // invalidator must not be initialized before then because the invalidation
+  // service cannot be started because it depends on components initialized
+  // after this object is instantiated.
+  // TODO(stepco): Investigate if we can update the invalidation service such

[Mattias Nissler (ping if slow), 2013/09/05 14:54:04]

OK, thanks for the stack trace. It turns out that there's already a bug on file
that complains about GetRequestContext() not working during profile
initialization: https://code.google.com/p/chromium/issues/detail?id=171406

Can you please file a new bug that you reference here which tracks cleaning this
up and block the new bug on 171406?

[Steve Condie, 2013/09/06 06:23:05]

Thanks for looking into that and finding the existing bug. I created bug 286209
to track the work.

+  // that it can be used immediately.
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_ADDED,
                  content::Source<Profile>(profile));
 }
 
 void UserCloudPolicyInvalidator::Shutdown() {
   CloudPolicyInvalidator::Shutdown();
 }
 
 void UserCloudPolicyInvalidator::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  // Enable invalidations now that profile creation is complete.
+  // Initialize now that profile creation is complete and the invalidation
+  // service can safely be initialized.
   DCHECK(type == chrome::NOTIFICATION_PROFILE_ADDED);
-  policy_manager_->EnableInvalidations(
-      base::Bind(
-          &CloudPolicyInvalidator::InitializeWithProfile,
-          GetWeakPtr(),
-          base::Unretained(profile_)));
+  Initialize(base::Bind(
+      &invalidation::InvalidationServiceFactory::GetForProfile,

[Mattias Nissler (ping if slow), 2013/09/05 14:54:04]

I guess you could just call GetForProfile here and pass the the
InvalidationService pointer?

[Steve Condie, 2013/09/06 06:23:05]

Since the invalidation service is initialized when GetForProfile is called, and
we're not sure at this point if it will be needed (or if cloud policy will even
be loaded), I thought it would be better to only call GetForProfile once we know
we need the service.

[Mattias Nissler (ping if slow), 2013/09/06 08:51:54]

Other consumers (such as the push messaging extension API) grab the
InvalidationService pointer immediately at startup as well, so grabbing the
pointer here directly doesn't make a difference (at least on desktop
platfrorms). Do you have any indication that instantiating InvalidationService
when we don't actually use it causes significant overhead that should be
avoided?

Even if so, I'd again prefer to not work around peculiarities of the
dependencies in consuming code (i.e. InvalidationService should make sure it
keeps its footprint within reasonable limits as long as it is unused), because
that adds noise to all consumers.

If there are no known issues with grabbing the pointer immediately, I prefer
doing it for code clarity.

[Steve Condie, 2013/09/06 18:01:40]

Done.

+      base::Unretained(profile_)));
 }
 
 }  // namespace policy