| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change refactors the related structs ErrorCounters, SyncerStatus, and
SyncCycleControlParameters. Their values have all been merged into
AllModelTypeState, which has been renamed to ModelNeutralState. All the
functions which depend on this data have been updated to use the new struct.
This change also removes the DirtyOnWrite template class, the is_dirty flag,
and the SyncerCommand logic to send change events if it detects a change in the
syncer's status. The changes are so frequent and predictable that it's easier
to just send the snapshots manually after any major syncer steps.
Finally, this change removes the 'invalid_store' status member, which was never
set nor read outside of unit tests.
BUG=132630
TEST=
Review URL: https://chromiumcodereview.appspot.com/10636010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143675 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remove all references to content in HttpBridge.
Clean up handling of HTTP user agent in HttpBridge.
BUG=133791
TEST=
Review URL: https://chromiumcodereview.appspot.com/10645004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143575 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
| |
BUG=128060
TEST=
Review URL: https://chromiumcodereview.appspot.com/10601002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143557 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Update all references from chrome.
Leave possibly-extraneous csync:: qualifications in sync/ for now.
(This will be cleaned up once everything in sync/ is in csync::.)
BUG=128060
TEST=
TBR=jhawkins@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10600002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143449 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only functional change was to remove the dirkernel_ member from
BaseTransaction, which was to reduce the number of includes required.
Everything else is just moving code around, and updating includes, forward
declarations, and 'using' statements.
BUG=103332
TEST=
Review URL: https://chromiumcodereview.appspot.com/10579036
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143218 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
| |
BUG=131336
TEST=none
Review URL: https://chromiumcodereview.appspot.com/10584019
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143088 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
CleanupDisabledTypes and GetKey commands.
This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.
BUG=129665,133061,129825
TEST=unit/integration tests
Review URL: https://chromiumcodereview.appspot.com/10483015
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10583002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142814 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
CleanupDisabledTypes and GetKey commands.
This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.
BUG=129665,133061,129825
TEST=unit/integration tests
Review URL: https://chromiumcodereview.appspot.com/10483015
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10536194
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10581005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142798 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
CleanupDisabledTypes and GetKey commands.
This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.
BUG=129665,133061,129825
TEST=unit/integration tests
Review URL: https://chromiumcodereview.appspot.com/10483015
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10536194
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142794 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
internal_api/public
TBR=jhawkins@chromium.org
BUG=131130
TEST=
Review URL: https://chromiumcodereview.appspot.com/10534080
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142626 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Old clients would sometimes end up with items that were deleted before
their existence was ever reported to the server. For a while, we let
these entries accumulate in the database, and just filtered them out of
the list of items to commit when we created that list in GetCommitIds.
Then we learned that a subset of these entries could be deleted in such
a manner that they would be detected as database corruption. The client
was udpated so it would delete these entries in a timely manner. The
filter in GetCommitIds was removed. See crbug.com/125381 for details.
It turns out that a lot of clients have some deleted local items in
their sync database created by older clients. When they update to the
new client which doesn't have the GetCommitIds filter, they will try
to commit the local tombstones. This is bad.
This commit addresses the problem in two ways:
1. Drop any server-unknown deleted items when we open the database.
2. As a fallback, put the filter back into GetCommitIds. The filter
should not be required, so it will trigger a NOTREACHED() if it is
executed.
BUG=132905,125381
TEST=SyncableDirectoryTest.OldClientLeftUnsyncedDeletedLocalItem
Review URL: https://chromiumcodereview.appspot.com/10559018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142555 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We remove all the pending download/configure state in SBH, in addition
to the split transaction nature of configurations themselves. This allows
us to have a single SyncScheduler::ScheduleConfiguration command that is
both synchronous (assuming it doesn't fail) and can handle
CleanupDisabledTypes and GetKey commands.
This also now keys which datatypes need downloading by checking the
initial sync ended bits directly. This allows us to recover from a new
sync db gracefully.
BUG=129665,133061,129825
TEST=unit/integration tests
Review URL: https://chromiumcodereview.appspot.com/10483015
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142517 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This CL pulls the code to track throttled data types out of the sync
session context and into a class meant for that purpose. This new
class, ThrottledDataTypeTracker, also implements code to notify the
AllStatus object whenever the set of throttled datatypes is changed.
The fact that ThrottledDataTypeTracker, which lives in sync/engine,
references AllStatus caused some problems with DEPS checks. After a few
iterations during code review, this commit now includes the following
additional changes:
- Move all_status.{cc,h} from sync/internal_api to sync/engine.
- Move the SyncManager::Status inner class out of SyncManager and into
sync/internal_api/public/engine/sync_status.{cc,h}. The class has
been renamed to SyncStatus.
This CL does not include code to expose the throttled status on the
chrome://sync page, though it does contain functionality we will use to
implement it in a future commit.
BUG=125065
TEST=
Review URL: https://chromiumcodereview.appspot.com/10454105
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141992 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Configuration, Clear User Data, Cleanup Disabled Types, and StartMode are all
now synchronous methods. Nudge and Stop are the only methods that do posting.
BUG=129665, 103327
TEST=unit_tests
Review URL: https://chromiumcodereview.appspot.com/10542044
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141541 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change includes some cleanups of the code introduced in r139519.
They have been kept separate from that CL in the hopes of making both
CLs easiser to read.
This commit moves some error-detection functionality from
ProcessCommitResponse's ModelNeutralExecuteImpl() into
BuildAndPostCommits(). This simplifies some of the error handling and
allows us to remove ModelChangingSyncerCommand's
ModelNeutralExecuteImpl().
This CL also combines both commit error indicators into a single
variable.
BUG=91696,36594
TEST=
Review URL: https://chromiumcodereview.appspot.com/10523003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141321 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
chrome/browser/sync.
Note on sync.gyp changes and .cc file moves: most files in /public have .h and their .cc side by side, as they are simple implementations. In some cases like model_type.cc (and others in a follow up patch, like sync_manager.cc) have only their header exposed in /public while the impl stays behind, because it needs to include things from within sync/, and /public has a strict include DEPS policy. This is in accordance with other /public folders (like content/).
Cleans up DEPS files in sync + c/b/sync. Adds sync/{engine, sessions, syncable} to public/. There is more to come (moving things in internal_api/ into public).
Not touching /notifier as that is in flux at the moment.
BUG=131130
TEST=
Review URL: https://chromiumcodereview.appspot.com/10532019
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141038 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We now have Succeeded(), which is true iff the command ran successfully, and
SuccessfullyReachedServer(), which is true iff we actually contacted the server
without errors.
BUG=131414
TEST=unit_tests
Review URL: https://chromiumcodereview.appspot.com/10537032
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140885 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SyncSourceInfo out of session_state.
This is in preparation for moving the snapshot et al to internal_api/public.
BUG=131130
TEST=none
Review URL: https://chromiumcodereview.appspot.com/10545010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140687 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
| |
That CL should have updated some unit tests to account for the change in
ownership of SyncSessionContext.
BUG=103326
TEST=Ran valgrind locally on sync_unit_tests.
Review URL: https://chromiumcodereview.appspot.com/10507002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140135 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The ModelSafeWorkerRegistrar interface was a wrapper around the
SyncBackendRegistrar. When the sync thread wanted to access the
SyncBackendRegistrar's information regarding currently enabled types or
workers, it would use the ModelSafeWorkerRegistrar interface to do it.
This change removes this implicit inter-thread communication. Where
necessary, it modifies the SyncBackendHost, SyncManager, SyncScheduler
and the SyncSessionContext to ensure that these classes have access to a
fresh copy of the SyncBackendRegistrar's data whenever it is required.
The most biggest consequence of this patch is that the
SyncSessionContext now maintains a copy of the list of ModelSafeWorkers
and routing info, rather than a pointer to the ModelSafeWorkerRegistrar.
Various functions along the path to CleanupDisabledTypes, Configure and
StartSyncingNormally have been updated to ensure that the latest routing
info is made available to the session context.
Future patches may refactor this code to reduce the amount of duplicated
state. This work was intentionally left to future patches to reduce the
risk of the current change. The refactoring should be much easier once
we're convinced that the new, explicit inter-thread communication
mechanisms are no more buggy than the existing code.
BUG=103326
TEST=sync_integration_tests
Review URL: https://chromiumcodereview.appspot.com/10388187
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140122 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
| |
BUG=none
TEST=
Review URL: https://chromiumcodereview.appspot.com/10443046
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@139462 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since its inception sync has required all commits to be preceded by a
GetUpdates request. This was done to try to ensure we detect and resolve
conflicts on the client, rather than having two conflicting commits
collide at the server. It could never work perfectly, but it was likely
to work in most cases and the server would bounce the commit if it
didn't.
Now we have a new server that doesn't always give us the latest version
of a node when we ask for it, especially when the request was not
solicited by the server (ie. not triggered by notification). The update
before commit is now much less likely to detect conflicts. Even worse,
the useless update requests can be surprisingly expensive in certain
scenarios.
This change allows us to avoid fetching updates between 'batches' of
commits. This should improve performance in the case where we have lots
of items to commit (ie. first time sync) and reduce load on the server.
This CL has some far-reaching effects. This is in part because I decided
to refactor the commit code, but major changes would have been required
with or without the refactor. Highlights include:
- The commit-related SyncerCommands are now paramaterized with local
variables, allowing us to remove many members from the SyncSession
classes.
- Several syncer states have been collapsed into one COMMIT state
which redirects to the new BuildAndPostCommits() function.
- The PostCommitMessageCommand had been reduced to one line, which
has now been inlined into the BuildAndPostCommits() function.
- The unsynced_handles counter has been removed for now. Following this
change, it would have always been zero unless an error was encountered
during the commit. The functions that reference it expect different
behaviour and would have been broken by this change.
- The code to put extensions activity data back into the
ExtensionsActivityMonitor in case of failure has been moved around to
avoid double-counting if we have to post many commit messages.
- A CONFLICT response from the server is now treated as a transient
error. If we had continued to treat it as a success we might risk
going into an infinite loop. See comment in code for details.
- The "purposeless anachronism" conflicting_new_folder_ids_ has been
removed.
Review URL: https://chromiumcodereview.appspot.com/10210009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@139159 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
| |
During a sync cycle(regular or configuration) the server might send us tombstones for items that don't belong to one of the types that the client requested. If the server sent such tombstones, with this patch, the client would ignore them.
BUG=
TEST=
Review URL: https://chromiumcodereview.appspot.com/10432003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138923 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Streamline methods of PushClient and its Observer subclass.
Remove sync/protocol/service_constants.h and consolidate use of each
constant in one place. Remove duplicate constant in cloud print code.
BUG=76764
TEST=
TBR=estade@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10398051
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@137615 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Prior to this change, if we were to delete a newly-created item before
informing the sync server of its existence, it would end up in an
unusual state. We do not commit deleted items unless they were known to
sync server. However, these items were still considered to be
'unsynced' (ie. waiting to be committed to the server) and therefore
impossible to entirely remove from the sync directory. They remain
forever both IS_DEL and IS_UNSYNCED.
This had a few side-effects. The most serious is that this behaviour
could cause directory corruption. If we created a folder and a bookmark
within it, then deleted that bookmark before it had a chance to sync,
the bookmark would remain in a zombie state. When we finally got around
to committing its parent folder, the folder's ID will change. The
zombie bookmark's PARENT_ID will not be updated, leaving us with a
dangling reference that will be detected as directory corruption the
next time sync is loaded. See crbug.com/125381 for details.
Another effect is that locally deleted, never synced UNIQUE_CLIENT_TAG
could have strange effects later on. If a foreign client were to create
an item with the same UNIQUE_CLIENT_TAG and sync it, this client would
conflict-resolve the update against the zombie entry. The zombie entry
would win, the newly created item would be deleted, and the deletion
synced back to the server. This may not be an entirely bad thing, but
it is a behaviour that's changed following this patch. From now on, the
deleted item will be overwritten (if it still exists, which it probably
won't).
Finally, we now have a mechanism for permanently deleting these items.
With this patch applied, these items will no longer have the unsynced
bit set, so they will be deleted on restart by DropDeletedEntries().
This patch changes PutIsDel() to clear the IS_UNSYNCED bit of entries
which were never committed to the sync server. The remainder of the
changes either add or update existing tests.
BUG=125381
TEST=SyncerTest.ClientTagUncommittedTagMatchesUpdate,
SyncableDirectoryTest.ChangeEntryIDAndUpdateChildren_DeletedAndUnsyncedChildren
Review URL: https://chromiumcodereview.appspot.com/10389103
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@137476 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
| |
remoting, and sync.
BUG=none
TEST=none
TBR=brettw,hclam,akalin,abodenha
Review URL: https://chromiumcodereview.appspot.com/10387107
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@137376 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This test has been causing flakiness on various platforms. Although we
could try to fix this by relaxing the timing constraints, I think it's
safer to remove the time-related assertions.
The test will still be very useful even without that assertion. The
UseMockDelayProvider() line and the other assertions should catch most
of the same bugs that the timing assertions would have found. At least
it's a significant improvement over a DISABLED/FLAKY test.
BUG=118370
TEST=SyncSchedulerTest.TransientPollFailure
Review URL: https://chromiumcodereview.appspot.com/10200001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136623 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The commit r133754 introduced some errors. That change made the tests
delete the session a bit more frequently, while some of the tests
maintained more pointers to its internal structures.
This change fixes all the affected tests and adds a member function to
the test framework class to try to prevent this from happening in the
future.
BUG=122033,123270
TEST=
Review URL: https://chromiumcodereview.appspot.com/10212008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133776 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change causes the syncer to exit from its download then commit
function if the download fails. This helps prevent the creation of
server-side conflicts, which would be more likely to happen if a
download failed but the following commit succeeded.
The main changes are as follows:
- Rather than proceed to the next step when a download updates failure
is detected, the syncer will exit. This should cause the
SyncScheduler to schedule a retry at a later time.
- The definition of a download updates failure is now based on the
return code of the download attempt, rather than checking the contents
of the (possible non-existent) returned protobuf. This makes the error
detection logic used here more closely match the logic used to decide
whether or not to schedule retries.
This implementation has a side-effect on configure sync cycles. The old
behaviour was to handle failures by applying whatever updates we had
downloaded at that point. The new behaviour will leave updates
unapplied if any error occurs. This better matches a nearby comment's
assertion which states that we should not attempt to apply updates until
we have downloaded all of them. I believe the author of that comment
would approve of this change.
This change moves around some of the ExtensionActivityMonitor logic.
It's important that we not take the data from the extensions acitivity
monitor at the start of the cycle. Restoring that data is handled in
the commit building and response code, which might not be executed if we
need to break out early. This also fixes issue 123270.
Most of the diffs for this change are concentrated in the unit tests:
- Expose more of the SyncScheduler to the SyncerTest test harness.
- Add functions to SyncerTest for testing specific types of sync jobs.
- Add some functions that allow us to better control failures in
MockConnectionManager.
- Added tests for configure job success and failure.
- Added tests for update then commit job success and failure.
- Removed some redundant tests.
- Removed unused test infrastructure.
This change allows the integration tests to expose a bug that was
introduced back in r118572. That commit assumed it was OK to not
retry a job that failed due to a MIGRATION_DONE response from the
server. That is incorrect, and this error has been fixed.
BUG=122033, 123270
TEST=sync_unit_tests, specifically:
SyncerTest.UpdateThenCommit,
SyncerTest.UpdateFailsThenDontCommit,
SyncerTest.ConfigureDownloadsTwoBatchesSuccess,
SyncerTest.ConfigureFailsDontApplyUpdates
Review URL: http://codereview.chromium.org/10103017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133754 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously it was an immutable struct that was passed around by making
dynamic allocations and passing pointers. We now just have a class with
only getters and no setters, but support for default copy and assign.
This cleans up some code and makes some future work easier to pass snapshots
around.
BUG=none
TEST=sync_unit_tests
Review URL: http://codereview.chromium.org/10197004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133747 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change causes the syncer to exit from its download then commit
function if the download fails. This helps prevent the creation of
server-side conflicts, which would be more likely to happen if a
download failed but the following commit succeeded.
The main changes are as follows:
- Rather than proceed to the next step when a download updates failure
is detected, the syncer will exit. This should cause the
SyncScheduler to schedule a retry at a later time.
- The definition of a download updates failure is now based on the
return code of the download attempt, rather than checking the contents
of the (possible non-existent) returned protobuf. This makes the error
detection logic used here more closely match the logic used to decide
whether or not to schedule retries.
This implementation has a side-effect on configure sync cycles. The old
behaviour was to handle failures by applying whatever updates we had
downloaded at that point. The new behaviour will leave updates
unapplied if any error occurs. This better matches a nearby comment's
assertion which states that we should not attempt to apply updates until
we have downloaded all of them. I believe the author of that comment
would approve of this change.
This change moves around some of the ExtensionActivityMonitor logic.
It's important that we not take the data from the extensions acitivity
monitor at the start of the cycle. Restoring that data is handled in
the commit building and response code, which might not be executed if we
need to break out early. This also fixes issue 123270.
Most of the diffs for this change are concentrated in the unit tests:
- Expose more of the SyncScheduler to the SyncerTest test harness.
- Add functions to SyncerTest for testing specific types of sync jobs.
- Add some functions that allow us to better control failures in
MockConnectionManager.
- Added tests for configure job success and failure.
- Added tests for update then commit job success and failure.
- Removed some redundant tests.
BUG=122033,123270
TEST=sync_unit_tests, specifically:
SyncerTest.UpdateThenCommit,
SyncerTest.UpdateFailsThenDontCommit,
SyncerTest.ConfigureDownloadsTwoBatchesSuccess,
SyncerTest.ConfigureFailsDontApplyUpdates
Review URL: http://codereview.chromium.org/10006046
TBR=rlarocque@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10107004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132476 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change causes the syncer to exit from its download then commit
function if the download fails. This helps prevent the creation of
server-side conflicts, which would be more likely to happen if a
download failed but the following commit succeeded.
The main changes are as follows:
- Rather than proceed to the next step when a download updates failure
is detected, the syncer will exit. This should cause the
SyncScheduler to schedule a retry at a later time.
- The definition of a download updates failure is now based on the
return code of the download attempt, rather than checking the contents
of the (possible non-existent) returned protobuf. This makes the error
detection logic used here more closely match the logic used to decide
whether or not to schedule retries.
This implementation has a side-effect on configure sync cycles. The old
behaviour was to handle failures by applying whatever updates we had
downloaded at that point. The new behaviour will leave updates
unapplied if any error occurs. This better matches a nearby comment's
assertion which states that we should not attempt to apply updates until
we have downloaded all of them. I believe the author of that comment
would approve of this change.
This change moves around some of the ExtensionActivityMonitor logic.
It's important that we not take the data from the extensions acitivity
monitor at the start of the cycle. Restoring that data is handled in
the commit building and response code, which might not be executed if we
need to break out early. This also fixes issue 123270.
Most of the diffs for this change are concentrated in the unit tests:
- Expose more of the SyncScheduler to the SyncerTest test harness.
- Add functions to SyncerTest for testing specific types of sync jobs.
- Add some functions that allow us to better control failures in
MockConnectionManager.
- Added tests for configure job success and failure.
- Added tests for update then commit job success and failure.
- Removed some redundant tests.
BUG=122033,123270
TEST=sync_unit_tests, specifically:
SyncerTest.UpdateThenCommit,
SyncerTest.UpdateFailsThenDontCommit,
SyncerTest.ConfigureDownloadsTwoBatchesSuccess,
SyncerTest.ConfigureFailsDontApplyUpdates
Review URL: http://codereview.chromium.org/10006046
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132455 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
| |
BUG=118370
TBR=rlarocque
Review URL: https://chromiumcodereview.appspot.com/10079003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132170 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
| |
BUG=none
TEST=none
Review URL: https://chromiumcodereview.appspot.com/9969114
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130670 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Thanks to all my reviewers, you are legion. I hope I didn't waste too
much of your time.
BUG=None
Review URL: http://codereview.chromium.org/9854039
Review URL: http://codereview.chromium.org/9854043
Review URL: http://codereview.chromium.org/9863058
Review URL: http://codereview.chromium.org/9863059
Review URL: http://codereview.chromium.org/9887005
Review URL: http://codereview.chromium.org/9890002
Review URL: http://codereview.chromium.org/9891002
Review URL: http://codereview.chromium.org/9895003
Review URL: http://codereview.chromium.org/9896002
Review URL: http://codereview.chromium.org/9896003
Review URL: http://codereview.chromium.org/9897002
Review URL: http://codereview.chromium.org/9897003
Review URL: http://codereview.chromium.org/9903004
Review URL: http://codereview.chromium.org/9904003
Review URL: http://codereview.chromium.org/9904002
Review URL: http://codereview.chromium.org/9904004
Review URL: http://codereview.chromium.org/9906002
Review URL: http://codereview.chromium.org/9906001
Review URL: http://codereview.chromium.org/9906003
Review URL: http://codereview.chromium.org/9909001
Review URL: http://codereview.chromium.org/9909002
Review URL: http://codereview.chromium.org/9909003
Review URL: http://codereview.chromium.org/9909004
Review URL: http://codereview.chromium.org/9910001
Review URL: http://codereview.chromium.org/9910002
Review URL: http://codereview.chromium.org/9910010
Review URL: http://codereview.chromium.org/9911001
Review URL: http://codereview.chromium.org/9912001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130359 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
| |
display the traffic.
BUG=117615
TEST=
Review URL: http://codereview.chromium.org/9826035
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130081 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
| |
TBR=lipalani@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9958043
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130015 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
memory.
The next patch would address the javascript side of exposing it in the UI.
BUG=
TEST=
Review URL: http://codereview.chromium.org/9732008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130001 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
shared with
the logging proto on the server-side.
R=lipalani
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/9837021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128930 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Many of the updates a sync client receives are echoes of its own
changes. This patch attempts to count how often these updates are
received by comparing the version of downloaded updates against the
local version.
These counts are exposed locally through AllStatus/about:sync. We also
upload this information to the server through the ClientDebugInfo
mechanism.
BUG=117565
TEST=
Review URL: http://codereview.chromium.org/9702083
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128659 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We now ensure we don't change to an old default key when we receive a nigori
update. We also preserve the local explicit passphrase state if we get into
conflict with a nigori node with old encryption keys. Lastly, we write to the
node at the end of each sync cycle toensure it has the current encrypted types
and encryption keys (assuming the cryptographer is ready). This way, we'll
never lose the current encryption state due to a blank/old nigori.
BUG=119207
TEST=
Review URL: https://chromiumcodereview.appspot.com/9815018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128567 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
retail builds this code would be compiled out. To view the traffic you have to run chrome with the flag:
--vmodule=traffic_logger=1.
BUG=117615
TEST=git-cl try --email=foo
Review URL: http://codereview.chromium.org/9663023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128277 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As of today, there is just one SetPassphrase API in ProfileSyncService, SyncBackendHost, SyncManager and SyncInternal, and it is used as a general purpose API for setting a passphrase during encryption and decryption. This is bad because the UI is aware of its intent when SetPassphrase is called, and yet, we discard that intent and recompute it via internal state deep inside SyncInternal.
Ideally, we should allow the UI to declare its intent by specifying whether it is trying to encrypt or decrypt by calling the appropriate API. The end goal of this change is to make the UI more responsive to the user when a passphrase is entered for decryption, because we can now test the entered passphrase against the UI layer's local cache of the cryptographer's keys, and give the user immediate feedback without having to display a spinner while the call is sent down to the syncer thread.
This patch does a bunch of things:
- Separates the SetPassphrase API into SetEncryptionPassphrase and SetDecryptionPassphrase.
- Plumbs these two APIs all the way up the stack, from SyncInternal, SyncManager, SyncbackendHost and ProfileSyncService.
- Adds a synchronous check in SetDecryptionPassphrase to see if the user-entered passphrase works on the cached pending keys, and thereby eliminates the need to show a spinner in the UI while decrypting.
- Updates calls from the UI layer and from unit and integration tests to the correct variant of the API.
- Updates the code in sync_setup_flow.cc to immediately show an error when an incorrect passphrase is entered.
BUG=108718, 95269
TEST=unit_tests, sync_unit_tests, sync_integration_tests, manually run through sign in with / without passphrases, try incorrect passphrases (we now expect instant errors)
Review URL: https://chromiumcodereview.appspot.com/9584040
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127630 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
- Remove 'authenticated' from SyncManager::Status. This variable was
set to true iff the last attempt to contact the sync server resulted in
a 200 OK response. There are lots of problems with this approach.
See also: crbug.com/117611.
The about:sync page also displays auth errors with
MakeSyncAuthErrorText(), which is much more likely to be accurate
- Remove server_up from SyncManager::Status. It used to be set to true
iff the last server response was SERVER_CONNECTION_OK. That doesn't
make a lot of sense and isn't very useful.
- Remove server_reachable from SyncManager::Status. This was toggled
only when ServerConnectionManager::CheckServerReachable() failed, which
didn't happen very often. CheckServerReachable() has since been
removed, so I think it now gets set only at startup and on the first
successful contact with the server.
- Remove server_reachable from ServerConnectionEvent and
ServerConnectionManager, for similar reasons.
- Remove the methods CheckTime(), IsServerReachable(),
IsUserAuthenticated(), and CheckServerReachable(),
SetServerParameters(), server_reachable() from ServerConnectionManager.
These methods were not used anywhere. Also removed
IsGoodReplyFromServer(), whose last remaining callsite has been removed
in this commit.
- Remove the server_connection_ok_ flag from SyncScheduler.
- Hard-code AllStatus' online status to true. See TODO comment.
- Remove OnCredentialsUpdated() and OnConnectionStatusChange() from
SyncScheduler. Their functionality has been moved to the SyncManager.
- Rearrange some code in MockConnectionManager. Apparently there used
to be such a thing as an "AUTHENTICATE" request, which we had unit tests
for. That no longer exists, so I've rewritten some parts of this mock
class to better relfect ServerConnectionManager's current authentication
mechanisms.
- Update various unit tests in response to these changes.
- Update some integration tests to verify backoff behaviour when the
sync server is unreachable. This replaces the old behaviour of
verifying that certain flags were set in the sync snapshot.
- Reduce the number of retries required to verify backoff. This is to
help ensure that our integration tests fall within the 45s time limit.
- Disable notifications when disabling network connectivity in
integration tests. This improves the reliability of exponential backoff
verification because the notifications would sometimes interfere with
backoff logic.
BUG=110954,105739,98346,106262
TEST=
Review URL: http://codereview.chromium.org/9348036
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127310 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Adds a parameter 'client' with value 'Chromium' or 'Google Chrome'.
Also removes an obsolete parameter string, kParameterAuthToken.
BUG=
TEST=
Review URL: http://codereview.chromium.org/9694051
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127098 0039d316-1c4b-4281-b951-d872f2087c98
|
|
|
|
|
|
|
|
|
|
|
| |
BUG=
TEST=base_unittests
TBR=abodenha@chromium.org,ajwong@chromium.org,chocobo@chromium.org,mnissler@chromium.org,akalin@chromium.org,brettw@chromium.org,arv@chromium.org
Review URL: http://codereview.chromium.org/9590002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127080 0039d316-1c4b-4281-b951-d872f2087c98
|
|
Also move related test files.
Move WriteNode::UpdateEntryWithEncryption to nigori_util.h.
Clean up defines and dependencies. In particular, get rid of SYNC_ENGINE_VERSION_STRING and hard-code the string in the single place it's used.
Rename data_encryption.* to data_encryption_win.* and add a pragma for crypt32.lib.
Clean up exit-time constructor warnings in sync{able,er}_unittest.cc.
Remove some unused files.
BUG=117585
TEST=
TBR=jhawkins@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9699057
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126872 0039d316-1c4b-4281-b951-d872f2087c98
|