Add non-blocking sync code to ProfileSyncService

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 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/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 61 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "grit/generated_resources.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "sync/api/sync_error.h"
 #include "sync/internal_api/public/configure_reason.h"
 #include "sync/internal_api/public/http_bridge_network_resources.h"
 #include "sync/internal_api/public/network_resources.h"
+#include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/util/sync_string_conversions.h"
 #include "sync/js/js_arg_list.h"
 #include "sync/js/js_event_details.h"
 #include "sync/util/cryptographer.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/time_format.h"
 
 #if defined(ENABLE_MANAGED_USERS)
@@ skipping 248 matching lines @@
 
 void ProfileSyncService::UnregisterAuthNotifications() {
   if (signin())
     signin()->RemoveObserver(this);
   oauth2_token_service_->RemoveObserver(this);
 }
 
 void ProfileSyncService::RegisterDataTypeController(
     DataTypeController* data_type_controller) {
   DCHECK_EQ(data_type_controllers_.count(data_type_controller->type()), 0U);
+  DCHECK(!GetRegisteredNonBlockingDataTypes().Has(
+      data_type_controller->type()));
   data_type_controllers_[data_type_controller->type()] =
       data_type_controller;
 }
 
+void ProfileSyncService::RegisterNonBlockingType(syncer::ModelType type) {
+  DCHECK_EQ(data_type_controllers_.count(type), 0U);
+  DCHECK(!GetRegisteredNonBlockingDataTypes().Has(type));
+  off_thread_types_.Put(type);

[Nicolas Zea, 2014/04/02 23:19:05]

Presumably this is where we could kick off the startup for this type if it's
preferred? Add a comment?

[rlarocque, 2014/04/03 01:03:46]

Nope, not here.  Registration is something that should happen with every init of
the ProfileSyncService.  This function is equivalent to
RegisterDataTypeController.

Startup will be much more complicated.  We might need to add an extra class to
track all the state involved.  

We can't start up until both the type and the backend are ready.  These are two
independent events that could happen in any order.  We'll also need some
edge-triggered logic to handle types being preferred or un-preferred.  So there
will be at least three different code paths that can trigger a startup.  

+}
+
 browser_sync::SessionModelAssociator*
     ProfileSyncService::GetSessionModelAssociatorDeprecated() {
   if (!IsSessionsDataTypeControllerRunning())
     return NULL;
 
   // If we're using sessions V2, there's no model associator.
   if (sessions_sync_manager_.get())
     return NULL;
 
   return static_cast<browser_sync::SessionDataTypeController*>(
@@ skipping 172 matching lines @@
       scoped_ptr<syncer::SyncManagerFactory>(
           new syncer::SyncManagerFactory).Pass(),
       backend_unrecoverable_error_handler.Pass(),
       &browser_sync::ChromeReportUnrecoverableError,
       network_resources_.get());
 }
 
 bool ProfileSyncService::IsEncryptedDatatypeEnabled() const {
   if (encryption_pending())
     return true;
-  const syncer::ModelTypeSet preferred_types = GetPreferredDataTypes();
+  const syncer::ModelTypeSet preferred_types =
+      GetPreferredDirectoryDataTypes();

[Nicolas Zea, 2014/04/02 23:19:05]

Why not GetPreferredDataTypes? Encryption isn't something tied to directory vs
nonblocking, right?

[rlarocque, 2014/04/03 01:03:46]

Good point.  Fixed.

   const syncer::ModelTypeSet encrypted_types = GetEncryptedDataTypes();
   DCHECK(encrypted_types.Has(syncer::PASSWORDS));
   return !Intersection(preferred_types, encrypted_types).Empty();
 }
 
 void ProfileSyncService::OnSyncConfigureRetry() {
   // Note: in order to handle auth failures that arise before the backend is
   // initialized (e.g. from invalidation notifier, or downloading new control
   // types), we have to gracefully handle configuration retries at all times.
   // At this point an auth error badge should be shown, which once resolved
@@ skipping 10 matching lines @@
 
 void ProfileSyncService::OnDataTypeRequestsSyncStartup(
     syncer::ModelType type) {
   DCHECK(syncer::UserTypes().Has(type));
   if (backend_.get()) {
     DVLOG(1) << "A data type requested sync startup, but it looks like "
                 "something else beat it to the punch.";
     return;
   }
 
-  if (!GetPreferredDataTypes().Has(type)) {
+  if (!GetPreferredDirectoryDataTypes().Has(type)) {

[Nicolas Zea, 2014/04/02 23:19:05]

Here too. If a nonblocking type requests sync startup, shouldn't we start up?

[rlarocque, 2014/04/03 01:03:46]

Maybe.  The non-blocking types won't be as badly affected by deferred init as
directory types.  I don't foresee them needing to request sync startup.

I guess it does not harm to support it, though.  I've updated it to
GetPreferredTypes() for now.

     // We can get here as datatype SyncableServices are typically wired up
     // to the native datatype even if sync isn't enabled.
     DVLOG(1) << "Dropping sync startup request because type "
              << syncer::ModelTypeToString(type) << "not enabled.";
     return;
   }
 
   startup_controller_.OnDataTypeRequestsSyncStartup(type);
 }
 
@@ skipping 341 matching lines @@
     // directory.  It would be no big deal if we tried to delete it again.
     OnInternalUnrecoverableError(FROM_HERE,
                                  "BackendInitialize failure",
                                  false,
                                  ERROR_REASON_BACKEND_INIT_FAILURE);
     return;
   }
 
   backend_initialized_ = true;
 
+  backend_->SetPreferredNonBlockingTypes(GetPreferredNonBlockingDataTypes());

[Nicolas Zea, 2014/04/02 23:19:05]

It's not clear to me why the backend needs to know about the preferred
nonblocking types

[rlarocque, 2014/04/03 01:03:46]

I was surprised to learn about this, too.

We want non-blocking types to receive invalidations like any other type.  But
the set of registered ObjectIds lives in the SyncBackendHost.  This call lets
the SyncBackendHost know that it should register these types to receive
invalidations.

There's probably a better solution than this.  Maybe invalidations should be
separated from the SyncBackendHost.  But that would be problematic, since the
SyncBackendHost manages the SyncBackendHostCore, which is where those
invalidations need to be forwarded eventually.  

I'm interested in fixing this, but I can't think of any simple way to do it.

[Nicolas Zea, 2014/04/03 17:52:15]

But why is this different from directory types? Do we care about registering for
invalidations before we actually start syncing the non-blocking type? Otherwise,
can't we wait to register for invalidations for that type until we're actually
syncing it?

[rlarocque, 2014/04/03 19:57:03]

Directory types register for invalidations once they're configured.  Now that we
have reliable invaliations, it's debatable whether or not it's necessary to wait
until configuration is done before registering them.  However, that's the way
it's always been, and there is some logic to it.  

Non-blocking types would have some difficulty registering only after they've
configured.  Non-blocking types' concept of configuration is less well-defined. 
There's currently no plan post a task to the SyncBackendHost / UI thread to
signal that their configuration is complete, like
FinishConfigureDataTypesOnFrontendLoop does now for directory types.  

I guess it would make more sense to delay the registration of the non-blocking
types until all the conditions for sync are met (ie. type is preferred, backend
is initialized, and model type code has indicated it is ready).  We'd still need
a separate hook into the SyncBackendHost, though.

+
   sync_js_controller_.AttachJsBackend(js_backend);
   debug_info_listener_ = debug_info_listener;
 
   if (protocol_event_observers_.might_have_observers()) {
     backend_->SetForwardProtocolEvents(true);
   }
 
+  syncer::SyncCoreProxy sync_core_proxy_ = backend_->GetSyncCoreProxy();
+
   // If we have a cached passphrase use it to decrypt/encrypt data now that the
   // backend is initialized. We want to call this before notifying observers in
   // case this operation affects the "passphrase required" status.
   ConsumeCachedPassphraseIfPossible();
 
   // The very first time the backend initializes is effectively the first time
   // we can say we successfully "synced".  last_synced_time_ will only be null
   // in this case, because the pref wasn't restored on StartUp.
   if (last_synced_time_.is_null()) {
     UpdateLastSyncedTime();
@@ skipping 254 matching lines @@
   if (HasUnrecoverableError()) {
     // When unrecoverable error is detected we post a task to shutdown the
     // backend. The task might not have executed yet.
     return;
   }
 
   DVLOG(1) << "Passphrase required with reason: "
            << syncer::PassphraseRequiredReasonToString(reason);
   passphrase_required_reason_ = reason;
 
-  const syncer::ModelTypeSet types = GetPreferredDataTypes();
+  const syncer::ModelTypeSet types = GetPreferredDirectoryDataTypes();
   if (data_type_manager_) {
     // Reconfigure without the encrypted types (excluded implicitly via the
     // failed datatypes handler).
     data_type_manager_->Configure(types,
                                   syncer::CONFIGURE_REASON_CRYPTO);
   }
 
   // Notify observers that the passphrase status may have changed.
   NotifyObservers();
 }
 
 void ProfileSyncService::OnPassphraseAccepted() {
   DVLOG(1) << "Received OnPassphraseAccepted.";
 
   // If the pending keys were resolved via keystore, it's possible we never
   // consumed our cached passphrase. Clear it now.
   if (!cached_passphrase_.empty())
     cached_passphrase_.clear();
 
   // Reset passphrase_required_reason_ since we know we no longer require the
   // passphrase. We do this here rather than down in ResolvePassphraseRequired()
   // because that can be called by OnPassphraseRequired() if no encrypted data
   // types are enabled, and we don't want to clobber the true passphrase error.
   passphrase_required_reason_ = syncer::REASON_PASSPHRASE_NOT_REQUIRED;
 
   // Make sure the data types that depend on the passphrase are started at
   // this time.
-  const syncer::ModelTypeSet types = GetPreferredDataTypes();
+  const syncer::ModelTypeSet types = GetPreferredDirectoryDataTypes();
   if (data_type_manager_) {
     // Re-enable any encrypted types if necessary.
     data_type_manager_->Configure(types,
                                   syncer::CONFIGURE_REASON_CRYPTO);
   }
 
   NotifyObservers();
 }
 
 void ProfileSyncService::OnEncryptedTypesChanged(
@@ skipping 394 matching lines @@
 void ProfileSyncService::ChangePreferredDataTypes(
     syncer::ModelTypeSet preferred_types) {
 
   DVLOG(1) << "ChangePreferredDataTypes invoked";
   const syncer::ModelTypeSet registered_types = GetRegisteredDataTypes();
   const syncer::ModelTypeSet registered_preferred_types =
       Intersection(registered_types, preferred_types);
   sync_prefs_.SetPreferredDataTypes(registered_types,
                                     registered_preferred_types);
 
+  if (backend_.get() && backend_initialized_)
+    backend_->SetPreferredNonBlockingTypes(GetPreferredNonBlockingDataTypes());
+
   // Now reconfigure the DTM.
   ReconfigureDatatypeManager();
 }
 
 syncer::ModelTypeSet ProfileSyncService::GetActiveDataTypes() const {
-  const syncer::ModelTypeSet preferred_types = GetPreferredDataTypes();
+  const syncer::ModelTypeSet preferred_types = GetPreferredDirectoryDataTypes();

[Nicolas Zea, 2014/04/02 23:19:05]

ActiveTypes only cares about whether they're running or not, not whether they're
directory or nonblocking. Why change this?

[rlarocque, 2014/04/03 01:03:46]

I thought this was used only for construction of the about:sync status, and that
it was related to DTC status.  Looks like that was wrong.

I've changed it back.

By the way, what is this actually used for?

[Nicolas Zea, 2014/04/03 17:52:15]

Anything that cares about what types are actually running. This affects sync's
internal error handling as well as UI that cares of sync types (like those using
favicons or tab data).

   const syncer::ModelTypeSet failed_types =
       failed_data_types_handler_.GetFailedTypes();
   return Difference(preferred_types, failed_types);
 }
 
 syncer::ModelTypeSet ProfileSyncService::GetPreferredDataTypes() const {
   const syncer::ModelTypeSet registered_types = GetRegisteredDataTypes();
   const syncer::ModelTypeSet preferred_types =
       sync_prefs_.GetPreferredDataTypes(registered_types);
   return preferred_types;
 }
 
+syncer::ModelTypeSet
+ProfileSyncService::GetPreferredDirectoryDataTypes() const {
+  const syncer::ModelTypeSet registered_directory_types =
+      GetRegisteredDirectoryDataTypes();
+  const syncer::ModelTypeSet preferred_types =
+      sync_prefs_.GetPreferredDataTypes(registered_directory_types);
+  return preferred_types;
+}
+
+syncer::ModelTypeSet
+ProfileSyncService::GetPreferredNonBlockingDataTypes() const {
+  return sync_prefs_.GetPreferredDataTypes(GetRegisteredNonBlockingDataTypes());
+}
+
 syncer::ModelTypeSet ProfileSyncService::GetRegisteredDataTypes() const {
+  return Union(GetRegisteredDirectoryDataTypes(),
+               GetRegisteredNonBlockingDataTypes());
+}
+
+syncer::ModelTypeSet
+ProfileSyncService::GetRegisteredDirectoryDataTypes() const {
   syncer::ModelTypeSet registered_types;
   // The data_type_controllers_ are determined by command-line flags; that's
   // effectively what controls the values returned here.
   for (DataTypeController::TypeMap::const_iterator it =
        data_type_controllers_.begin();
        it != data_type_controllers_.end(); ++it) {
     registered_types.Put(it->first);
   }
   return registered_types;
 }
 
+syncer::ModelTypeSet
+ProfileSyncService::GetRegisteredNonBlockingDataTypes() const {
+  return off_thread_types_;
+}
+
 bool ProfileSyncService::IsUsingSecondaryPassphrase() const {
   syncer::PassphraseType passphrase_type = GetPassphraseType();
   return passphrase_type == syncer::FROZEN_IMPLICIT_PASSPHRASE ||
          passphrase_type == syncer::CUSTOM_PASSPHRASE;
 }
 
 syncer::PassphraseType ProfileSyncService::GetPassphraseType() const {
   return backend_->GetPassphraseType();
 }
 
 base::Time ProfileSyncService::GetExplicitPassphraseTime() const {
   return backend_->GetExplicitPassphraseTime();
 }
 
 bool ProfileSyncService::IsCryptographerReady(
     const syncer::BaseTransaction* trans) const {
   return backend_.get() && backend_->IsCryptographerReady(trans);
 }
 
 void ProfileSyncService::ConfigurePriorityDataTypes() {
   const syncer::ModelTypeSet priority_types =
-      Intersection(GetPreferredDataTypes(), syncer::PriorityUserTypes());
+      Intersection(GetPreferredDirectoryDataTypes(),
+                   syncer::PriorityUserTypes());
   if (!priority_types.Empty()) {
     const syncer::ConfigureReason reason = HasSyncSetupCompleted() ?
         syncer::CONFIGURE_REASON_RECONFIGURATION :
         syncer::CONFIGURE_REASON_NEW_CLIENT;
     data_type_manager_->Configure(priority_types, reason);
   }
 }
 
 void ProfileSyncService::ConfigureDataTypeManager() {
   // Don't configure datatypes if the setup UI is still on the screen - this
@@ skipping 17 matching lines @@
 
     // We create the migrator at the same time.
     migrator_.reset(
         new browser_sync::BackendMigrator(
             profile_->GetDebugName(), GetUserShare(),
             this, data_type_manager_.get(),
             base::Bind(&ProfileSyncService::StartSyncingWithServer,
                        base::Unretained(this))));
   }
 
-  const syncer::ModelTypeSet types = GetPreferredDataTypes();
+  const syncer::ModelTypeSet types = GetPreferredDirectoryDataTypes();
   syncer::ConfigureReason reason = syncer::CONFIGURE_REASON_UNKNOWN;
   if (!HasSyncSetupCompleted()) {
     reason = syncer::CONFIGURE_REASON_NEW_CLIENT;
   } else if (restart) {
     // Datatype downloads on restart are generally due to newly supported
     // datatypes (although it's also possible we're picking up where a failed
     // previous configuration left off).
     // TODO(sync): consider detecting configuration recovery and setting
     // the reason here appropriately.
     reason = syncer::CONFIGURE_REASON_NEWLY_ENABLED_DATA_TYPE;
@@ skipping 96 matching lines @@
       type_status->SetString("value", error_text);
     } else if (throttled_types.Has(type) && passive_types.Has(type)) {
       type_status->SetString("status", "warning");
       type_status->SetString("value", "Passive, Throttled");
     } else if (passive_types.Has(type)) {
       type_status->SetString("status", "warning");
       type_status->SetString("value", "Passive");
     } else if (throttled_types.Has(type)) {
       type_status->SetString("status", "warning");
       type_status->SetString("value", "Throttled");
+    } else if (GetRegisteredNonBlockingDataTypes().Has(type)) {
+      type_status->SetString("status", "ok");
+      type_status->SetString("value", "Non-Blocking");
     } else if (active_types.Has(type)) {
       type_status->SetString("status", "ok");
       type_status->SetString("value", "Active: " +
                              ModelSafeGroupToString(routing_info[type]));
     } else {
       type_status->SetString("status", "warning");
       type_status->SetString("value", "Disabled by User");
     }
 
     int live_count = detailed_status.num_entries_by_type[type] -
@@ skipping 342 matching lines @@
   status.last_get_token_error = last_get_token_error_;
   if (request_access_token_retry_timer_.IsRunning())
     status.next_token_request_time = next_token_request_time_;
   return status;
 }
 
 void ProfileSyncService::OverrideNetworkResourcesForTest(
     scoped_ptr<syncer::NetworkResources> network_resources) {
   network_resources_ = network_resources.Pass();
 }