summaryrefslogtreecommitdiffstats
path: root/sync/engine
Commit message (Collapse)AuthorAgeFilesLines
* sync: Refactor session tracking rlarocque@chromium.org2012-06-228-40/+33
| | | | | | | | | | | | | | | | | | | | | | | 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
* [Sync] Move HttpBridge to sync/akalin@chromium.org2012-06-223-11/+3
| | | | | | | | | | | | | | 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
* [Sync] Rename sync_api to csyncakalin@chromium.org2012-06-226-114/+111
| | | | | | | | | | 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
* [Sync] Rename browser_sync to csync in sync/akalin@chromium.org2012-06-2174-224/+224
| | | | | | | | | | | | | | | 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
* Split syncable.{h,cc} into several filesrlarocque@chromium.org2012-06-2027-45/+80
| | | | | | | | | | | | | | | | 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
* sync: Remove ClearUserData command.tim@chromium.org2012-06-2011-296/+1
| | | | | | | | | 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
* Revert 142517 - [Sync] Refactor sync configuration logic.zea@chromium.org2012-06-184-317/+181
| | | | | | | | | | | | | | | | | | | | | | 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
* Revert 142794 - Revert 142517 - [Sync] Refactor sync configuration logic.kkania@chromium.org2012-06-184-181/+317
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Revert 142517 - [Sync] Refactor sync configuration logic.zea@chromium.org2012-06-184-317/+181
| | | | | | | | | | | | | | | | | | | | | | 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
* sync: move internal_api components used by chrome/browser into ↵tim@chromium.org2012-06-171-1/+1
| | | | | | | | | | | | | | 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
* Protect against sync entries left by old clientsrlarocque@chromium.org2012-06-161-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [Sync] Refactor sync configuration logic.zea@chromium.org2012-06-154-181/+317
| | | | | | | | | | | | | | | | | | | 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
* sync: Refactor per-datatype throttlingrlarocque@chromium.org2012-06-1314-33/+491
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [Sync] Remove unnecessary posting of methods in sync scheduler.zea@chromium.org2012-06-113-207/+117
| | | | | | | | | | | | | 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
* Refactor following sync commit loop changerlarocque@chromium.org2012-06-088-96/+84
| | | | | | | | | | | | | | | | | | | | | | | 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
* sync: create internal_api/public to house sync/ files needed by ↵tim@chromium.org2012-06-0734-370/+41
| | | | | | | | | | | | | | | | | 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
* [Sync] Fix sync scheduler/session logic determining successful commands.zea@chromium.org2012-06-063-16/+37
| | | | | | | | | | | | | | 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
* sync: split SyncSessionSnapshot, SyncerStatus, ErrorCounters, and ↵tim@chromium.org2012-06-062-0/+2
| | | | | | | | | | | | | | 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
* Fix leak introduced in r140122rlarocque@chromium.org2012-06-022-16/+17
| | | | | | | | | | | | 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
* Remove sync's ModelSafeWorkerRegistrarrlarocque@chromium.org2012-06-016-78/+94
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [Sync] Cleanup all tab sync enabling logic now that its on by default.zea@chromium.org2012-05-302-17/+7
| | | | | | | | | | 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
* sync: Loop committing items without downloading updatesrlarocque@chromium.org2012-05-2616-357/+548
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Ignore tombstones for items belonging to unrequested types. lipalani@chromium.org2012-05-242-2/+18
| | | | | | | | | | | | 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
* [Sync] Replace TalkMediator*/MediatorThread* with PushClientakalin@chromium.org2012-05-172-2/+0
| | | | | | | | | | | | | | | 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
* Sync: Clear IS_UNSYNCED for deleted local itemsrlarocque@chromium.org2012-05-166-82/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Cleanup: Remove unneeded scoped_ptr.h includes from ppapi, printing, ↵thestig@chromium.org2012-05-161-1/+0
| | | | | | | | | | | | 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
* Remove flaky assertion from TransientPollFailurerlarocque@chromium.org2012-05-111-7/+2
| | | | | | | | | | | | | | | | | | | 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
* Fix memory errors in sync unit testsrlarocque@chromium.org2012-04-241-19/+22
| | | | | | | | | | | | | | | | | 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
* Abort sync cycles when download step failsrlarocque@chromium.org2012-04-244-233/+386
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [Sync] Convert SyncSessionSnapshot to a copy-able class.zea@chromium.org2012-04-246-119/+123
| | | | | | | | | | | | | | | | | 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
* Revert 132455 - Abort sync cycles when download step failsrlarocque@chromium.org2012-04-164-362/+221
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Abort sync cycles when download step failsrlarocque@chromium.org2012-04-164-221/+362
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Mark SyncSchedulerTest.TransientPollFailure as flakyglider@chromium.org2012-04-131-1/+2
| | | | | | | | 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
* sync: remove UnappliedUpdateMetaHandles and just use std::vector<int64>tim@chromium.org2012-04-043-4/+3
| | | | | | | | | 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
* Fix some grammar in comments, error messages and documentation.gavinp@chromium.org2012-04-032-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* A new 'traffic' tab is introduced and clicking the button on the tab would ↵lipalani@chromium.org2012-04-016-4/+67
| | | | | | | | | | | | 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
* Remove executable bit from traffic_recorder_unittest.cc.mihaip@chromium.org2012-03-311-0/+0
| | | | | | | | 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
* We store the past 10 records of client server communication in a queue in ↵lipalani@chromium.org2012-03-315-4/+201
| | | | | | | | | | | | | | | 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
* sync: Move the error Action enum to sync_enums.proto so that it can be ↵albertb@chromium.org2012-03-261-7/+7
| | | | | | | | | | | | | | | 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
* sync: Count and report reflected updatesrlarocque@chromium.org2012-03-241-16/+50
| | | | | | | | | | | | | | | | | | | 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
* [Sync] Improve nigori conflict and rollback handling.zea@chromium.org2012-03-232-5/+118
| | | | | | | | | | | | | | | | 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
* This would allow the developers to view the client/server sync traffic. In ↵lipalani@chromium.org2012-03-224-2/+82
| | | | | | | | | | | | | | | 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
* Separate SetPassphrase into encryption and decryption APIsrsimha@chromium.org2012-03-203-8/+8
| | | | | | | | | | | | | | | | | | | | 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
* Trim code form sync's ServerConnectionManagerrlarocque@chromium.org2012-03-176-169/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - 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
* Add 'client' parameter to sync requestsrlarocque@chromium.org2012-03-161-1/+10
| | | | | | | | | | | | | | 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
* JSONWriter cleanup: integrate pretty print into write options.ericdingle@chromium.org2012-03-162-2/+4
| | | | | | | | | | | 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
* [Sync] Move 'sync' target to sync/akalin@chromium.org2012-03-1576-0/+17695
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