Skip to content

Commit

Permalink
sync: SyncableService support for starting sync.
Browse files Browse the repository at this point in the history
Still has no actual effect as operational bits are behind the --sync-enable-deferred-startup flag.  If you pass that flag, autofill will now be permitted to kick off sync init (in fact, it will be the only thing that can kick off sync init, so you probably don't want to pass the flag yet!).

Next I'll add a fallback timer and plumb the flare to more datatypes; non Sync API types will have to call ProfileSyncService directly.

TBR=dhollowa@chromium.org
^ For web_data_service_factory as changes are just basic consequence of the parameter change to AutocompleteSyncableService already LGTM'd.

BUG=80194

Review URL: https://chromiumcodereview.appspot.com/14018026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@198309 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tim@chromium.org committed May 4, 2013
1 parent bc1d60e commit 67c7e0a
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 19 deletions.
51 changes: 51 additions & 0 deletions chrome/browser/sync/glue/sync_start_util.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// 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/sync/glue/sync_start_util.h"

#include "base/bind.h"
#include "base/files/file_path.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "content/public/browser/browser_thread.h"

namespace {

void StartSyncOnUIThread(const base::FilePath& profile,
syncer::ModelType type) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* p = profile_manager->GetProfileByPath(profile);
if (!p) {
DVLOG(2) << "Profile not found, can't start sync.";
return;
}

ProfileSyncService* service = ProfileSyncServiceFactory::GetForProfile(p);
if (!service) {
DVLOG(2) << "No ProfileSyncService for profile, can't start sync.";
return;
}
service->OnDataTypeRequestsSyncStartup(type);
}

void StartSyncProxy(const base::FilePath& profile,
syncer::ModelType type) {
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::Bind(&StartSyncOnUIThread, profile, type));
}

} // namespace

namespace sync_start_util {

syncer::SyncableService::StartSyncFlare GetFlareForSyncableService(
const base::FilePath& profile_path) {
return base::Bind(&StartSyncProxy, profile_path);
}

} // namespace sync_start_util
33 changes: 33 additions & 0 deletions chrome/browser/sync/glue/sync_start_util.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.
//
// Various utilities for kicking off sync initialization from data types or
// other services.

#ifndef CHROME_BROWSER_SYNC_GLUE_SYNC_START_UTIL_H_
#define CHROME_BROWSER_SYNC_GLUE_SYNC_START_UTIL_H_

#include "sync/api/syncable_service.h"

namespace base {
class FilePath;
}

namespace sync_start_util {

// Creates a StartSyncFlare that a SyncableService can use to tell
// ProfileSyncService it needs sync to start ASAP. Typically this would be
// given to the SyncableService on construction.
//
// The flare built by this function is designed to be Run()able from any thread
// so that non-UI types don't have to deal with posting tasks.
//
// |profile_path| is used to get a hold of the actual Profile* once the
// request to start sync is safely in UI Thread land.
syncer::SyncableService::StartSyncFlare GetFlareForSyncableService(
const base::FilePath& profile_path);

} // namespace sync_start_util

#endif // CHROME_BROWSER_SYNC_GLUE_SYNC_START_UTIL_H_
61 changes: 51 additions & 10 deletions chrome/browser/sync/profile_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ ProfileSyncService::ProfileSyncService(ProfileSyncComponentsFactory* factory,
sync_prefs_(profile_ ? profile_->GetPrefs() : NULL),
invalidator_storage_(profile_ ? profile_->GetPrefs(): NULL),
sync_service_url_(kDevServerUrl),
data_type_requested_sync_startup_(false),
is_first_time_sync_configure_(false),
backend_initialized_(false),
is_auth_in_progress_(false),
Expand Down Expand Up @@ -305,7 +306,12 @@ void ProfileSyncService::TryStart() {
// for performance reasons and maximizing parallelism at chrome startup, we
// defer the heavy lifting for sync init until things have calmed down.
if (HasSyncSetupCompleted()) {
StartUp(STARTUP_BACKEND_DEFERRED);
if (!data_type_requested_sync_startup_)
StartUp(STARTUP_BACKEND_DEFERRED);
else if (start_up_time_.is_null())
StartUp(STARTUP_IMMEDIATE);
else
StartUpSlowBackendComponents();
} else if (setup_in_progress_ || auto_start_enabled_) {
// We haven't completed sync setup. Start immediately if the user explicitly
// kicked this off or we're supposed to automatically start syncing.
Expand Down Expand Up @@ -527,23 +533,58 @@ void ProfileSyncService::StartUp(StartUpDeferredOption deferred_option) {
return;
}

StartUpSlowBackendComponents(STARTUP_IMMEDIATE);
StartUpSlowBackendComponents();
}

void ProfileSyncService::StartUpSlowBackendComponents(
StartUpDeferredOption deferred_option) {
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)) {
// 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;
}

if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kSyncEnableDeferredStartup)) {
DVLOG(2) << "Data type requesting sync startup: "
<< syncer::ModelTypeToString(type);
// Measure the time spent waiting for init and the type that triggered it.
// We could measure the time spent deferred on a per-datatype basis, but
// for now this is probably sufficient.
if (!start_up_time_.is_null()) {
// TODO(tim): Cache |type| and move this tracking to StartUp. I'd like
// to pull all the complicated init logic and state out of
// ProfileSyncService and have only a StartUp method, though. One step
// at a time. Bug 80149.
base::TimeDelta time_deferred = base::Time::Now() - start_up_time_;
UMA_HISTOGRAM_TIMES("Sync.Startup.TimeDeferred", time_deferred);
UMA_HISTOGRAM_ENUMERATION("Sync.Startup.TypeTriggeringInit",
ModelTypeToHistogramInt(type),
syncer::MODEL_TYPE_COUNT);
}
data_type_requested_sync_startup_ = true;
TryStart();
}
DVLOG(2) << "Ignoring data type request for sync startup: "
<< syncer::ModelTypeToString(type);
}

void ProfileSyncService::StartUpSlowBackendComponents() {
// Don't start up multiple times.
if (backend_) {
DVLOG(1) << "Skipping bringing up backend host.";
return;
}

DCHECK(!start_up_time_.is_null());
if (deferred_option == STARTUP_BACKEND_DEFERRED) {
base::TimeDelta time_deferred = base::Time::Now() - start_up_time_;
UMA_HISTOGRAM_TIMES("Sync.Startup.TimeDeferred", time_deferred);
}

DCHECK(IsSyncEnabledAndLoggedIn());
CreateBackend();

Expand Down
18 changes: 14 additions & 4 deletions chrome/browser/sync/profile_sync_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,12 @@ class ProfileSyncService : public ProfileSyncServiceBase,
const invalidation::ObjectId& id,
const std::string& payload);

// Called when a datatype (SyncableService) has a need for sync to start
// ASAP, presumably because a local change event has occurred but we're
// still in deferred start mode, meaning the SyncableService hasn't been
// told to MergeDataAndStartSyncing yet.
void OnDataTypeRequestsSyncStartup(syncer::ModelType type);

protected:
// Used by test classes that derive from ProfileSyncService.
virtual browser_sync::SyncBackendHost* GetBackendForTest();
Expand Down Expand Up @@ -742,10 +748,8 @@ class ProfileSyncService : public ProfileSyncServiceBase,
};
void StartUp(StartUpDeferredOption deferred_option);

// Starts up the backend sync components. |deferred_option| specifies whether
// this is being called as part of an immediate startup or startup was
// originally deferred and we're finally getting around to finishing.
void StartUpSlowBackendComponents(StartUpDeferredOption deferred_option);
// Starts up the backend sync components.
void StartUpSlowBackendComponents();

// About-flags experiment names for datatypes that aren't enabled by default
// yet.
Expand Down Expand Up @@ -825,6 +829,12 @@ class ProfileSyncService : public ProfileSyncServiceBase,
// called.
base::Time start_up_time_;

// Whether we have received a signal from a SyncableService requesting that
// sync starts as soon as possible.
// TODO(tim): Move this and other TryStart related logic + state to separate
// class. Bug 80149.
bool data_type_requested_sync_startup_;

// The time that OnConfigureStart is called. This member is zero if
// OnConfigureStart has not yet been called, and is reset to zero once
// OnConfigureDone is called.
Expand Down
19 changes: 14 additions & 5 deletions chrome/browser/webdata/autocomplete_syncable_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ AutocompleteSyncableService::AutocompleteSyncableService()
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
}

void AutocompleteSyncableService::InjectStartSyncFlare(
const syncer::SyncableService::StartSyncFlare& flare) {
flare_ = flare;
}

syncer::SyncMergeResult AutocompleteSyncableService::MergeDataAndStartSyncing(
syncer::ModelType type,
const syncer::SyncDataList& initial_sync_data,
Expand Down Expand Up @@ -304,12 +309,16 @@ syncer::SyncError AutocompleteSyncableService::ProcessSyncChanges(

void AutocompleteSyncableService::AutofillEntriesChanged(
const AutofillChangeList& changes) {
// Check if sync is on. If we receive notification prior to the sync being set
// up we are going to process all when MergeData..() is called. If we receive
// notification after the sync exited, it will be sinced next time Chrome
// starts.
if (sync_processor_.get())
// Check if sync is on. If we recieve this notification prior to sync being
// started, we'll notify sync to start as soon as it can and later process
// all entries when MergeData..() is called. If we receive this notification
// sync has exited, it will be synced next time Chrome starts.
if (sync_processor_.get()) {
ActOnChanges(changes);
} else if (!flare_.is_null()) {
flare_.Run(syncer::AUTOFILL);
flare_.Reset();
}
}

bool AutocompleteSyncableService::LoadAutofillData(
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/webdata/autocomplete_syncable_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ class AutocompleteSyncableService
void UpdateCullSetting(bool cull_expired_entries);
bool cull_expired_entries() const { return cull_expired_entries_; }

// Provides a StartSyncFlare to the SyncableService. See
// sync_start_util for more.
void InjectStartSyncFlare(
const syncer::SyncableService::StartSyncFlare& flare);

protected:
explicit AutocompleteSyncableService(
autofill::AutofillWebDataService* web_data_service);
Expand Down Expand Up @@ -165,6 +170,8 @@ class AutocompleteSyncableService
// via UpdateCullingSetting.
bool cull_expired_entries_;

syncer::SyncableService::StartSyncFlare flare_;

DISALLOW_COPY_AND_ASSIGN(AutocompleteSyncableService);
};

Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/webdata/web_data_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile_dependency_manager.h"
#include "chrome/browser/sync/glue/sync_start_util.h"
#include "chrome/browser/ui/profile_error_dialog.h"
#include "chrome/browser/webdata/autocomplete_syncable_service.h"
#include "chrome/browser/webdata/autofill_profile_syncable_service.h"
Expand Down Expand Up @@ -40,12 +41,15 @@ void ProfileErrorCallback(sql::InitStatus status) {

void InitSyncableServicesOnDBThread(
scoped_refptr<AutofillWebDataService> autofill_web_data,
const syncer::SyncableService::StartSyncFlare& flare,
const std::string& app_locale) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));

// Currently only Autocomplete and Autofill profiles use the new Sync API, but
// all the database data should migrate to this API over time.
AutocompleteSyncableService::CreateForWebDataService(autofill_web_data);
AutocompleteSyncableService::FromWebDataService(
autofill_web_data)->InjectStartSyncFlare(flare);
AutofillProfileSyncableService::CreateForWebDataService(
autofill_web_data, app_locale);
}
Expand Down Expand Up @@ -95,6 +99,7 @@ WebDataServiceWrapper::WebDataServiceWrapper(Profile* profile) {
BrowserThread::DB, FROM_HERE,
base::Bind(&InitSyncableServicesOnDBThread,
autofill_web_data_,
sync_start_util::GetFlareForSyncableService(path),
g_browser_process->GetApplicationLocale()));
}

Expand Down
2 changes: 2 additions & 0 deletions chrome/chrome_browser.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,8 @@
'browser/sync/glue/sync_backend_host.h',
'browser/sync/glue/sync_backend_registrar.cc',
'browser/sync/glue/sync_backend_registrar.h',
'browser/sync/glue/sync_start_util.cc',
'browser/sync/glue/sync_start_util.h',
'browser/sync/glue/synced_device_tracker.cc',
'browser/sync/glue/synced_device_tracker.h',
'browser/sync/glue/synced_session.cc',
Expand Down
10 changes: 10 additions & 0 deletions sync/api/syncable_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <vector>

#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
Expand All @@ -28,6 +29,15 @@ class SYNC_EXPORT SyncableService
: public SyncChangeProcessor,
public base::SupportsWeakPtr<SyncableService> {
public:
// A StartSyncFlare is useful when your SyncableService has a need for sync
// to start ASAP, typically because a local change event has occurred but
// MergeDataAndStartSyncing hasn't been called yet, meaning you don't have a
// SyncChangeProcessor. The sync subsystem will respond soon after invoking
// Run() on your flare by calling MergeDataAndStartSyncing. The ModelType
// parameter is included so that the recieving end can track usage and timing
// statistics, make optimizations or tradeoffs by type, etc.
typedef base::Callback<void(ModelType)> StartSyncFlare;

// Informs the service to begin syncing the specified synced datatype |type|.
// The service should then merge |initial_sync_data| into it's local data,
// calling |sync_processor|'s ProcessSyncChanges as necessary to reconcile the
Expand Down

0 comments on commit 67c7e0a

Please sign in to comment.