Add AffiliatedInvalidationServiceProvider

chrome/browser/chromeos/policy/affiliated_invalidation_service_provider.cc

-// Copyright 2014 The Chromium Authors. All rights reserved.
+// Copyright 2015 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/chromeos/policy/device_cloud_policy_invalidator.h"
+#include "chrome/browser/chromeos/policy/affiliated_invalidation_service_provider.h"
 
-#include <string>
 #include <vector>
 
 #include "base/logging.h"
-#include "base/message_loop/message_loop_proxy.h"
-#include "base/time/clock.h"
-#include "base/time/default_clock.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
-#include "chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/policy/ticl_device_settings_provider.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/device_identity_provider.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
 #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h"
-#include "chrome/browser/policy/cloud/cloud_policy_invalidator.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_content_client.h"
 #include "components/invalidation/invalidation_handler.h"
 #include "components/invalidation/invalidation_service.h"
 #include "components/invalidation/invalidation_state_tracker.h"
 #include "components/invalidation/invalidator_state.h"
 #include "components/invalidation/invalidator_storage.h"
 #include "components/invalidation/profile_invalidation_provider.h"
 #include "components/invalidation/ticl_invalidation_service.h"
 #include "components/invalidation/ticl_settings_provider.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/user_manager/user.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "google_apis/gaia/identity_provider.h"
-#include "net/url_request/url_request_context_getter.h"
-#include "policy/proto/device_management_backend.pb.h"
-
-class Profile;
 
 namespace policy {
 
-class DeviceCloudPolicyInvalidator::InvalidationServiceObserver
+AffiliatedInvalidationServiceProvider::Consumer::~Consumer() {
+}
+
+class AffiliatedInvalidationServiceProvider::InvalidationServiceObserver
     : public syncer::InvalidationHandler {
  public:
   explicit InvalidationServiceObserver(
-      DeviceCloudPolicyInvalidator* parent,
+      AffiliatedInvalidationServiceProvider* parent,
       invalidation::InvalidationService* invalidation_service);
-  ~InvalidationServiceObserver() override;
+  virtual ~InvalidationServiceObserver();
 
   invalidation::InvalidationService* GetInvalidationService();
   bool IsServiceConnected() const;
 
   // public syncer::InvalidationHandler:
-  void OnInvalidatorStateChange(syncer::InvalidatorState state) override;
-  void OnIncomingInvalidation(
+  virtual void OnInvalidatorStateChange(
+      syncer::InvalidatorState state) override;
+  virtual void OnIncomingInvalidation(
       const syncer::ObjectIdInvalidationMap& invalidation_map) override;
-  std::string GetOwnerName() const override;
+  virtual std::string GetOwnerName() const override;
 
  private:
-  DeviceCloudPolicyInvalidator* parent_;
+  AffiliatedInvalidationServiceProvider* parent_;
   invalidation::InvalidationService* invalidation_service_;
   bool is_service_connected_;
 
   DISALLOW_COPY_AND_ASSIGN(InvalidationServiceObserver);
 };
 
-DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     InvalidationServiceObserver(
-        DeviceCloudPolicyInvalidator* parent,
+        AffiliatedInvalidationServiceProvider* parent,
         invalidation::InvalidationService* invalidation_service)
     : parent_(parent),
       invalidation_service_(invalidation_service),
       is_service_connected_(invalidation_service->GetInvalidatorState() ==
                                 syncer::INVALIDATIONS_ENABLED) {
   invalidation_service_->RegisterInvalidationHandler(this);
+  if (IsServiceConnected())
+    parent->OnInvalidationServiceConnected(invalidation_service_);
 }
 
-DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     ~InvalidationServiceObserver() {
   invalidation_service_->UnregisterInvalidationHandler(this);
 }
 
 invalidation::InvalidationService*
-DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     GetInvalidationService() {
   return invalidation_service_;
 }
 
-bool DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+bool AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
          IsServiceConnected() const {
   return is_service_connected_;
 }
 
-void DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+void AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     OnInvalidatorStateChange(syncer::InvalidatorState state) {
   const bool is_service_connected = (state == syncer::INVALIDATIONS_ENABLED);
   if (is_service_connected == is_service_connected_)
     return;
 
   is_service_connected_ = is_service_connected;
   if (is_service_connected_)
     parent_->OnInvalidationServiceConnected(invalidation_service_);
   else
     parent_->OnInvalidationServiceDisconnected(invalidation_service_);
 }
 
-void DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+void AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     OnIncomingInvalidation(
         const syncer::ObjectIdInvalidationMap& invalidation_map) {
 }
 
-std::string DeviceCloudPolicyInvalidator::InvalidationServiceObserver::
+std::string AffiliatedInvalidationServiceProvider::InvalidationServiceObserver::
     GetOwnerName() const {
-  return "DevicePolicy";
+  return "AffiliatedInvalidationService";
 }
 
-DeviceCloudPolicyInvalidator::DeviceCloudPolicyInvalidator()
-    : invalidation_service_(NULL),
-      highest_handled_invalidation_version_(0) {
-  // The DeviceCloudPolicyInvalidator should be created before any user
+AffiliatedInvalidationServiceProvider::AffiliatedInvalidationServiceProvider()
+    : invalidation_service_(nullptr),
+      consumer_count_(0) {
+  // The AffiliatedInvalidationServiceProvider should be created before any user
   // Profiles.
   DCHECK(g_browser_process->profile_manager()->GetLoadedProfiles().empty());
 
   // Subscribe to notification about new user profiles becoming available.
   registrar_.Add(this,
                  chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
                  content::NotificationService::AllSources());
-
-  TryToCreateInvalidator();
 }
 
-DeviceCloudPolicyInvalidator::~DeviceCloudPolicyInvalidator() {
-  DestroyInvalidator();
+AffiliatedInvalidationServiceProvider::
+~AffiliatedInvalidationServiceProvider() {
 }
 
-void DeviceCloudPolicyInvalidator::Observe(
+void AffiliatedInvalidationServiceProvider::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED, type);
   Profile* profile = content::Details<Profile>(details).ptr();
   invalidation::ProfileInvalidationProvider* invalidation_provider =
       invalidation::ProfileInvalidationProviderFactory::GetForProfile(profile);
   if (!invalidation_provider) {
     // If the Profile does not support invalidation (e.g. guest, incognito),
     // ignore it.
     return;
   }
   user_manager::User* user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
   if (!user ||
       g_browser_process->platform_part()->browser_policy_connector_chromeos()->
           GetUserAffiliation(user->email()) != USER_AFFILIATION_MANAGED) {
-    // If the Profile belongs to a user who is not affiliated with the domain
-    // the device is enrolled into, ignore it.
+    // If the Profile belongs to a user who is not affiliated with the device's
+    // enrollment domain, ignore it.
     return;
   }
-  // Create a state observer for the user's invalidation service.
+
+  // Create a state observer for the user's invalidation service. If the
+  // invalidation service is connected, the observer will trigger a check
+  // whether to switch to the service.
+  invalidation::InvalidationService* invalidation_service =
+      invalidation_provider->GetInvalidationService();
   profile_invalidation_service_observers_.push_back(
-      new InvalidationServiceObserver(
-              this,
-              invalidation_provider->GetInvalidationService()));
-
-  TryToCreateInvalidator();
+      new InvalidationServiceObserver(this, invalidation_service));
 }
 
-void DeviceCloudPolicyInvalidator::OnInvalidationServiceConnected(
+void AffiliatedInvalidationServiceProvider::RegisterConsumer(
+    Consumer* consumer) {
+  if (consumers_.HasObserver(consumer))
+    return;
+
+  consumers_.AddObserver(consumer);
+  ++consumer_count_;
+
+  if (invalidation_service_)
+    consumer->OnInvalidationServiceSet(invalidation_service_);
+  else if (consumer_count_ == 1)
+    FindConnectedInvalidationService();
+}
+
+void AffiliatedInvalidationServiceProvider::UnregisterConsumer(
+    Consumer* consumer) {
+  if (!consumers_.HasObserver(consumer))
+    return;
+
+  consumers_.RemoveObserver(consumer);
+  --consumer_count_;
+
+  if (invalidation_service_ && consumer_count_ == 0) {
+    invalidation_service_ = nullptr;
+    DestroyDeviceInvalidationService();
+  }
+}
+
+invalidation::TiclInvalidationService*
+AffiliatedInvalidationServiceProvider::
+    GetDeviceInvalidationServiceForTest() const {
+  return device_invalidation_service_.get();
+}
+
+void AffiliatedInvalidationServiceProvider::OnInvalidationServiceConnected(
     invalidation::InvalidationService* invalidation_service) {
+  if (consumer_count_ == 0) {
+    // If there are no consumers, no invalidation service is required.
+    return;
+  }
+
   if (!device_invalidation_service_) {
-    // The lack of a device-global invalidation service implies that a
-    // |CloudPolicyInvalidator| backed by another connected service exists
+    // The lack of a device-global invalidation service implies that another
+    // connected invalidation service is being made available to consumers
     // already. There is no need to switch from that to the service which just
     // connected.
     return;
   }
 
   if (invalidation_service != device_invalidation_service_.get()) {
     // If an invalidation service other than the device-global one connected,
-    // destroy the device-global service and the |CloudPolicyInvalidator| backed
-    // by it, if any.
-    DestroyInvalidator();
+    // destroy the device-global service.
+    invalidation_service_ = nullptr;
     DestroyDeviceInvalidationService();
   }
 
-  // Create a |CloudPolicyInvalidator| backed by the invalidation service which
-  // just connected.
-  CreateInvalidator(invalidation_service);
+  // Make the invalidation service that just connected available to consumers.
+  SetInvalidationService(invalidation_service);
 }
 
-void DeviceCloudPolicyInvalidator::OnInvalidationServiceDisconnected(
+void AffiliatedInvalidationServiceProvider::OnInvalidationServiceDisconnected(
     invalidation::InvalidationService* invalidation_service) {
   if (invalidation_service != invalidation_service_) {
-    // If the invalidation service which disconnected is not backing the current
-    // |CloudPolicyInvalidator|, return.
+    // If the invalidation service which disconnected was not being made
+    // available to consumers, return.
     return;
   }
 
-  // Destroy the |CloudPolicyInvalidator| backed by the invalidation service
-  // which just disconnected.
-  DestroyInvalidator();
+  // The invalidation service which disconnected was being made available to
+  // consumers. Stop making it available.
+  DCHECK(consumer_count_);
+  invalidation_service_ = nullptr;
 
-  // Try to create a |CloudPolicyInvalidator| backed by another invalidation
-  // service.
-  TryToCreateInvalidator();
+  // Try to make another invalidation service available to consumers.
+  FindConnectedInvalidationService();
+
+  // If no other connected invalidation service was found, explicitly notify
+  // consumers that the invalidation service they were using is no longer
+  // available.
+  if (!invalidation_service_)

[Mattias Nissler (ping if slow), 2015/01/21 12:57:28]

Still doing some work here after a call to the observer returns, which could
have changed internal state. Maybe it's better (and more readable?) to have
FindConnectedInvalidationService return the service it picked and then fire the
update here?

[bartfab (slow), 2015/01/21 13:38:48]

I had considered this. However, there is one code path in which
FindConnectedInvalidationService() does not explicitly set the
|invalidation_service_| itself:

If there is no |device_invalidation_service_| yet,
FindConnectedInvalidationService() creates one. If this service reports that it
is connected immediately (unlikely as a client-server handshake needs to happen
first but I need to be compatible with all possible future implementations,
including e.g. one that just latches onto a cached connection immediately), its
observer will trigger SetInvalidationService().

Thus, at the end of FindConnectedInvalidationService(), there are three options:
1) SetInvalidationService() was called by FindConnectedInvalidationService()
explicitly.
2) SetInvalidationService() was called by other code that
FindConnectedInvalidationService() triggered.
3) SetInvalidationService() was never called. |invalidation_service_| is a
nullptr.

FindConnectedInvalidationService() can report (1) reliably, but it cannot
distinguish between (2) and (3). Thus, you have to look at
|invalidation_service_| in some cases. I could move this into
FindConnectedInvalidationService() and only look at |invalidation_service_| in
the code path that needs to distinguish between (2) and (3). But I find it
cleaner, simpler and less error-prone to unconditionally look at
|invalidation_service_| here instead, allowing us to always distinguish between
(1)/(2) and (3).

[Mattias Nissler (ping if slow), 2015/01/21 14:19:26]

A simple way to get to the point where SetInvalidationService doesn't get called
for cases #2 and #3 is to remove the IsServiceConnected check in the
InvalidationServiceObserver ctor and perform the IsServiceConnected check in the
caller. That makes the block starting in lines 289 go away, and a check would
have to be added in line 166 (I think you had that initially). Do you think
that'd be clearer?

[bartfab (slow), 2015/01/21 14:45:23]

*Argh*, Rietveld ate my homework and deleted the entire comment I had typed :(.
I will start again:

I had moved IsServiceConnected() to the observer constructor at your explicit
request :), see:

https://codereview.chromium.org/828953004/diff2/20001:40001/chrome/browser/ch...

While thinking about this, I noticed another corner case in the observer
constructor that the newest patch set fixes:

1) The invalidaiton service is not connected at first.
2) The service connects while the observer is registering as an observer (line
75), calling OnInvalidatorStateChange(), which triggers
OnInvalidationServiceConnected()
3) The observer notices that the service is connected (line 76), triggering
OnInvalidationServiceConnected().

We now have two calls to OnInvalidationServiceConnected(), both of which call
SetInvalidationService(). The second call runs into
DCHECK(!invalidation_service_); (line 299).

If we move the IsServiceConnected() check out of the observer constructor, we
will still have to check somehow whether the service happens to have triggered
an SetInvalidationService() before we got to the IsServiceConnected() check. The
only way to do this that I can see is to look at |invalidation_service_| once
again. So nothing is gained.

[Mattias Nissler (ping if slow), 2015/01/21 14:59:58]

The problem you've found with RegisterInvalidationHandler() triggering
re-entrancy is even worse - what if the profile and with it the profile's
invalidation service go away in the mean time?

Tangential question: Are you handling the case of profiles going away properly
at all? I can't find a place where observers are removed fro
|profile_invalidation_service_observers_|.

Anyhow, an additional problem will be that while you call
RegisterInvalidationHandler(), the OnValidationServiceConnected() callback will
fire at a time where the newly-created observer isn't present in
profile_invalidation_service_observers_, which can only lead to trouble. Maybe
it's best to ignore callbacks until you've wired everything up correctly? Once
you arm the observer, you would then also perform the IsConnected() check.

+    SetInvalidationService(nullptr);
 }
 
-void DeviceCloudPolicyInvalidator::TryToCreateInvalidator() {
-  if (invalidator_) {
-    // If a |CloudPolicyInvalidator| exists already, return.
-    return;
-  }
+void AffiliatedInvalidationServiceProvider::FindConnectedInvalidationService() {
+  DCHECK(!invalidation_service_);
+  DCHECK(consumer_count_);
 
   for (ScopedVector<InvalidationServiceObserver>::const_iterator it =
            profile_invalidation_service_observers_.begin();
            it != profile_invalidation_service_observers_.end(); ++it) {
     if ((*it)->IsServiceConnected()) {
       // If a connected invalidation service belonging to an affiliated
-      // logged-in user is found, create a |CloudPolicyInvalidator| backed by
-      // that service and destroy the device-global service, if any.
+      // logged-in user is found, make it available to consumers.
       DestroyDeviceInvalidationService();
-      CreateInvalidator((*it)->GetInvalidationService());
+      SetInvalidationService((*it)->GetInvalidationService());
       return;
     }
   }
 
   if (!device_invalidation_service_) {
-    // If no other connected invalidation service was found, ensure that a
-    // device-global service is running.
+    // If no other connected invalidation service was found and no device-global
+    // invalidation service exists, create one. The invalidation service will be
+    // made available to consumers when it successfully connects.
     device_invalidation_service_.reset(
         new invalidation::TiclInvalidationService(
             GetUserAgent(),
             scoped_ptr<IdentityProvider>(new chromeos::DeviceIdentityProvider(
                 chromeos::DeviceOAuth2TokenServiceFactory::Get())),
             scoped_ptr<invalidation::TiclSettingsProvider>(
                 new TiclDeviceSettingsProvider),
             g_browser_process->gcm_driver(),
             g_browser_process->system_request_context()));
     device_invalidation_service_->Init(
         scoped_ptr<syncer::InvalidationStateTracker>(
             new invalidation::InvalidatorStorage(
                     g_browser_process->local_state())));
     device_invalidation_service_observer_.reset(
         new InvalidationServiceObserver(
                 this,
                 device_invalidation_service_.get()));
-  }
 
-  if (device_invalidation_service_observer_->IsServiceConnected()) {
-    // If the device-global invalidation service is connected, create a
-    // |CloudPolicyInvalidator| backed by it. Otherwise,  a
-    // |CloudPolicyInvalidator| will be created later when a connected service
-    // becomes available.
-    CreateInvalidator(device_invalidation_service_.get());
+  } else if (device_invalidation_service_observer_->IsServiceConnected()) {
+    // If a connected device-global invalidation service exists already, make it
+    // available to consumers immediately. Otherwise, the invalidation service
+    // will be made available to clients when it successfully connects.
+    SetInvalidationService(device_invalidation_service_.get());
   }
 }
 
-void DeviceCloudPolicyInvalidator::CreateInvalidator(
+void AffiliatedInvalidationServiceProvider::SetInvalidationService(
     invalidation::InvalidationService* invalidation_service) {
+  DCHECK(!invalidation_service_);
   invalidation_service_ = invalidation_service;
-  DCHECK(!invalidator_);
-  invalidator_.reset(new CloudPolicyInvalidator(
-      enterprise_management::DeviceRegisterRequest::DEVICE,
-      g_browser_process->platform_part()->browser_policy_connector_chromeos()->
-          GetDeviceCloudPolicyManager()->core(),
-      base::MessageLoopProxy::current(),
-      scoped_ptr<base::Clock>(new base::DefaultClock()),
-      highest_handled_invalidation_version_));
-  invalidator_->Initialize(invalidation_service);
+  FOR_EACH_OBSERVER(Consumer,
+                    consumers_,
+                    OnInvalidationServiceSet(invalidation_service_));
 }
 
-void DeviceCloudPolicyInvalidator::DestroyInvalidator() {
-  if (invalidator_) {
-    highest_handled_invalidation_version_ =
-        invalidator_->highest_handled_invalidation_version();
-    invalidator_->Shutdown();
-  }
-  invalidator_.reset();
-  invalidation_service_ = NULL;
-}
-
-void DeviceCloudPolicyInvalidator::DestroyDeviceInvalidationService() {
+void AffiliatedInvalidationServiceProvider::DestroyDeviceInvalidationService() {
   device_invalidation_service_observer_.reset();
   device_invalidation_service_.reset();
 }
 
 }  // namespace policy