summaryrefslogtreecommitdiffstats
path: root/sync/engine
Commit message (Collapse)AuthorAgeFilesLines
* 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
generated by cgit v1.1 at 2025-02-14 12:32:49 +0000