Skip to content

Commit

Permalink
sync: Finish the SyncScheduler refactor
Browse files Browse the repository at this point in the history
This change removes SyncSessionJob entirely.  The first step towards
this goal was to refactor all functions that depended on SyncSessionJob.
All these functions have either been inlined or modified to take
different parameters instead.  DoSyncSessionJob was split into
two separate functions, one for configure jobs and one for nudge jobs,
which removes the need for SyncSessionJob's "purpose" member.

The SyncScheduler's pending_configure_job_ has been replaced with
a ConfigParams member, since that was the only part of the job still
being referenced.  The pending_nudge_job_ has been replaced with
scheduled_nudge_time_ (which is similar to the old scheduled_start_
member of SyncSessionJob) and a new object called a NudgeTracker.

The NudgeTracker inherits the SyncSessionJob's responsibilities with
respect to coalescing nudge sources.  The plan is to extend this class
to support more sophisticated nudge coalescing in the future.  For now
it tries to emulate the old SyncSessionJob behaviour as closely as
possible.

Some of the refactoring does change behaviour.  In particular, the
decision-making logic has been updated to fix issues 179515 and 155296.

BUG=175024,179515,155296

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@194766 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Apr 18, 2013
1 parent 9df2d09 commit 0248d92
Show file tree
Hide file tree
Showing 15 changed files with 422 additions and 1,053 deletions.
8 changes: 4 additions & 4 deletions chrome/browser/sync/profile_sync_service_harness.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ class ProfileSyncServiceHarness
// Gets the |auto_start_enabled_| variable from the |service_|.
bool AutoStartEnabled();

// Returns true if a status change took place, false on timeout.
bool AwaitStatusChangeWithTimeout(int timeout_milliseconds,
const std::string& reason);

private:
friend class StateChangeTimeoutEvent;

Expand Down Expand Up @@ -286,10 +290,6 @@ class ProfileSyncServiceHarness
// change has taken place.
bool RunStateChangeMachine();

// Returns true if a status change took place, false on timeout.
bool AwaitStatusChangeWithTimeout(int timeout_milliseconds,
const std::string& reason);

// A helper for implementing IsDataSynced() and IsFullySynced().
bool IsDataSyncedImpl(
const syncer::sessions::SyncSessionSnapshot& snapshot);
Expand Down
15 changes: 4 additions & 11 deletions chrome/browser/sync/test/integration/sync_errors_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,20 +140,13 @@ IN_PROC_BROWSER_TEST_F(SyncErrorTest,
protocol_error.error_description);
}

// Trigger an auth error and make sure the sync client detects it when
// trying to commit.
// Trigger an auth error and make sure the sync client displays a warning in the
// UI.
IN_PROC_BROWSER_TEST_F(SyncErrorTest, AuthErrorTest) {
ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";

const BookmarkNode* node1 = AddFolder(0, 0, L"title1");
SetTitle(0, node1, L"new_title1");
ASSERT_TRUE(GetClient(0)->AwaitFullSyncCompletion("Sync."));

ASSERT_TRUE(SetupClients());
TriggerAuthError();

const BookmarkNode* node2 = AddFolder(0, 0, L"title2");
SetTitle(0, node2, L"new_title2");
ASSERT_TRUE(GetClient(0)->AwaitExponentialBackoffVerification());
ASSERT_FALSE(GetClient(0)->SetupSync());
ASSERT_EQ(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS,
GetClient(0)->service()->GetAuthError().state());
}
Expand Down
Loading

0 comments on commit 0248d92

Please sign in to comment.