Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Issue 3305003: New authorization framework for sync. ... (Closed)

Created:
10 years, 3 months ago by John Gregg
Modified:
9 years ago
CC:
chromium-reviews, ncarter (slow), nkostylev+cc_chromium.org, idana, ben+cc_chromium.org, cbentzel+watch_chromium.org, darin-cc_chromium.org, davemoore+watch_chromium.org, Paweł Hajdan Jr., pam+watch_chromium.org, tim (not reviewing)
Visibility:
Public.

Description

New authorization framework for sync. To quote chron's original patch (http://codereview.chromium.org/3148036/show) <blockquote> This patch removes: authenticator.cc, auth_watcher.cc removes calls to user_settings.cc, removes an authenticate PB request to the server, and moves token storage into the Chrome TokenService. This patch introduces the SigninManager, which is an interim solution for user management prior to moving the system into chrome. Other changes include removing the dependency on the sync backend to be running while the sync wizard is intially displayed. This means that the backend can be brought up in response to credentials becoming available. The backend now is always provided credentials on startup. If an auth error occurs, it propogates it up via a notification. Some event handlers were removed and streamlined for more straightforward sync system startup. </blockquote> BUG=51001, 50293, 35158 TEST=Unit tests && Start up sync, log in, log out, run with expired credentials, run with new gaia credentials, run with gaia credentials updated while system is syncing. Try logging in with incorrect username. Trigger CAPTCHA. Try logging out and in repeatedly. Check about:sync works. Try going offline and back online again. Expire gaia credentials and try renewing it with the UI dialog. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58778

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 10

Patch Set 3 : include added files #

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 4

Patch Set 6 : '' #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+1311 lines, -2034 lines) Patch
M chrome/browser/chromeos/login/login_utils.cc View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/preferences_window_controller.mm View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/dom_ui/new_tab_page_sync_handler.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/gtk/options/content_page_gtk.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/net/gaia/token_service.h View 1 2 3 4 5 5 chunks +17 lines, -2 lines 0 comments Download
M chrome/browser/net/gaia/token_service.cc View 1 2 3 4 5 5 chunks +29 lines, -1 line 0 comments Download
A chrome/browser/net/gaia/token_service_unittest.h View 1 chunk +130 lines, -0 lines 0 comments Download
M chrome/browser/net/gaia/token_service_unittest.cc View 1 2 3 4 5 4 chunks +7 lines, -119 lines 0 comments Download
M chrome/browser/prefs/browser_prefs.cc View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/profile.h View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/profile.cc View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/profile_impl.h View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/profile_impl.cc View 1 2 3 4 5 3 chunks +10 lines, -3 lines 0 comments Download
M chrome/browser/sync/abstract_profile_sync_service_test.h View 1 2 3 4 5 3 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sync/engine/all_status.h View 1 2 3 4 5 3 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/sync/engine/all_status.cc View 1 2 3 4 5 5 chunks +9 lines, -38 lines 0 comments Download
D chrome/browser/sync/engine/auth_watcher.h View 1 2 3 4 5 1 chunk +0 lines, -222 lines 0 comments Download
D chrome/browser/sync/engine/auth_watcher.cc View 1 2 3 4 5 1 chunk +0 lines, -352 lines 0 comments Download
D chrome/browser/sync/engine/auth_watcher_unittest.cc View 1 2 3 4 5 1 chunk +0 lines, -235 lines 0 comments Download
D chrome/browser/sync/engine/authenticator.h View 1 2 3 4 5 1 chunk +0 lines, -105 lines 0 comments Download
D chrome/browser/sync/engine/authenticator.cc View 1 2 3 4 5 1 chunk +0 lines, -110 lines 0 comments Download
M chrome/browser/sync/engine/net/server_connection_manager.h View 1 2 3 4 5 4 chunks +7 lines, -11 lines 0 comments Download
M chrome/browser/sync/engine/net/server_connection_manager.cc View 1 2 3 4 5 3 chunks +1 line, -12 lines 0 comments Download
M chrome/browser/sync/engine/net/syncapi_server_connection_manager.h View 1 2 3 4 5 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/sync/engine/syncapi.h View 1 2 3 4 5 4 chunks +17 lines, -46 lines 0 comments Download
M chrome/browser/sync/engine/syncapi.cc View 1 2 3 4 5 32 chunks +108 lines, -342 lines 0 comments Download
M chrome/browser/sync/engine/syncapi_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/engine/syncer_proto_util.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/engine/syncer_proto_util.cc View 1 2 3 4 5 4 chunks +6 lines, -9 lines 0 comments Download
M chrome/browser/sync/engine/syncer_proto_util_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/engine/syncer_thread.h View 1 2 3 4 5 4 chunks +3 lines, -8 lines 0 comments Download
M chrome/browser/sync/engine/syncer_thread.cc View 1 2 3 4 5 4 chunks +11 lines, -25 lines 0 comments Download
M chrome/browser/sync/engine/syncer_thread_unittest.cc View 1 2 3 4 5 13 chunks +15 lines, -6 lines 0 comments Download
M chrome/browser/sync/engine/syncer_types.h View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/sync/engine/syncer_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/glue/sync_backend_host.h View 1 2 3 4 5 8 chunks +26 lines, -28 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_host.cc View 1 2 3 4 5 11 chunks +32 lines, -29 lines 0 comments Download
M chrome/browser/sync/profile_sync_factory.h View 1 2 3 4 5 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_factory_impl.h View 1 2 3 4 5 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_factory_impl.cc View 1 2 3 4 5 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_factory_impl_unittest.cc View 1 2 3 4 5 7 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/sync/profile_sync_factory_mock.h View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service.h View 1 2 3 4 5 11 chunks +30 lines, -23 lines 0 comments Download
M chrome/browser/sync/profile_sync_service.cc View 1 2 3 4 5 21 chunks +187 lines, -88 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_autofill_unittest.cc View 1 2 3 4 5 3 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_mock.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_password_unittest.cc View 1 2 3 4 5 3 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_preference_unittest.cc View 1 2 3 4 5 3 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_session_unittest.cc View 1 2 3 4 5 3 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_startup_unittest.cc View 1 2 3 4 5 8 chunks +20 lines, -44 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_typed_url_unittest.cc View 1 2 3 4 5 3 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_unittest.cc View 1 2 3 4 5 7 chunks +20 lines, -15 lines 0 comments Download
M chrome/browser/sync/sessions/sync_session_context.h View 1 2 3 4 5 4 chunks +0 lines, -7 lines 0 comments Download
M chrome/browser/sync/sessions/sync_session_unittest.cc View 1 2 3 4 5 4 chunks +4 lines, -4 lines 0 comments Download
A chrome/browser/sync/signin_manager.h View 1 chunk +81 lines, -0 lines 0 comments Download
A chrome/browser/sync/signin_manager.cc View 1 chunk +95 lines, -0 lines 0 comments Download
A chrome/browser/sync/signin_manager_unittest.cc View 1 chunk +93 lines, -0 lines 0 comments Download
M chrome/browser/sync/sync_setup_wizard_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/sync_ui_util.cc View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/syncable/directory_manager.h View 1 2 3 4 5 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/sync/syncable/directory_manager.cc View 1 2 3 4 5 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/browser/sync/test_profile_sync_service.h View 1 2 3 4 5 3 chunks +5 lines, -4 lines 0 comments Download
A chrome/browser/sync/token_migrator.h View 1 2 3 4 5 1 chunk +51 lines, -0 lines 3 comments Download
A chrome/browser/sync/token_migrator.cc View 1 2 3 4 5 1 chunk +53 lines, -0 lines 0 comments Download
M chrome/browser/views/options/content_page_view.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 5 chunks +15 lines, -11 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 4 chunks +2 lines, -2 lines 0 comments Download
M chrome/common/notification_type.h View 1 2 3 4 5 2 chunks +27 lines, -4 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/test/live_sync/live_sync_test.cc View 1 2 3 4 5 4 chunks +9 lines, -1 line 0 comments Download
D chrome/test/live_sync/offline_sync_test.cc View 1 2 3 4 5 1 chunk +0 lines, -29 lines 0 comments Download
M chrome/test/live_sync/profile_sync_service_test_harness.h View 1 2 3 4 5 3 chunks +4 lines, -8 lines 0 comments Download
M chrome/test/live_sync/profile_sync_service_test_harness.cc View 1 2 3 4 5 9 chunks +38 lines, -33 lines 0 comments Download
M chrome/test/live_sync/single_client_live_bookmarks_sync_test.cc View 1 2 3 4 5 1 chunk +31 lines, -0 lines 0 comments Download
M chrome/test/profile_mock.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/sync/engine/mock_connection_manager.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/sync/engine/syncer_command_test.h View 1 2 3 4 5 2 chunks +4 lines, -1 line 0 comments Download
M chrome/test/testing_profile.h View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/test/testing_profile.cc View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
John Gregg
Branching the change into my own CL and adding the TokenMigration code. Tim I addressed ...
10 years, 3 months ago (2010-09-01 23:57:23 UTC) #1
tim (not reviewing)
http://codereview.chromium.org/3305003/diff/15002/13063 File chrome/browser/sync/token_migrator.cc (right): http://codereview.chromium.org/3305003/diff/15002/13063#newcode34 chrome/browser/sync/token_migrator.cc:34: void TokenMigrator::TryMigration() { can we add a CurrentlyOn dcheck? ...
10 years, 3 months ago (2010-09-02 01:05:01 UTC) #2
Raghu Simha
Test code LGTM. http://codereview.chromium.org/3305003/diff/18004/19077 File chrome/test/live_sync/profile_sync_service_test_harness.cc (left): http://codereview.chromium.org/3305003/diff/18004/19077#oldcode310 chrome/test/live_sync/profile_sync_service_test_harness.cc:310: // Choose datatypes to be synced. ...
10 years, 3 months ago (2010-09-03 00:50:45 UTC) #3
John Gregg
http://codereview.chromium.org/3305003/diff/15002/13063 File chrome/browser/sync/token_migrator.cc (right): http://codereview.chromium.org/3305003/diff/15002/13063#newcode34 chrome/browser/sync/token_migrator.cc:34: void TokenMigrator::TryMigration() { On 2010/09/02 01:05:02, timsteele wrote: > ...
10 years, 3 months ago (2010-09-07 18:33:33 UTC) #4
tim (not reviewing)
10 years, 3 months ago (2010-09-07 22:01:52 UTC) #5
LGTM with nits

http://codereview.chromium.org/3305003/diff/75001/3070
File chrome/browser/sync/token_migrator.h (right):

http://codereview.chromium.org/3305003/diff/75001/3070#newcode28
chrome/browser/sync/token_migrator.h:28: // This runs as a deferred task on the
IO thread.
nit- s / IO / DB

http://codereview.chromium.org/3305003/diff/75001/3070#newcode31
chrome/browser/sync/token_migrator.h:31: // This runs as a deferred task on the
UI thread (only after the IO thread is
nit s/IO/DB

http://codereview.chromium.org/3305003/diff/75001/3070#newcode47
chrome/browser/sync/token_migrator.h:47: FilePath database_location_;
DISALLOW_COPY_AND_ASSIGN

Powered by Google App Engine
This is Rietveld 408576698