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

Issue 964563002: Replace SetAuthenticatedUsername with SetAuthenticatedAccountId. (Closed)

Created:
5 years, 10 months ago by Roger Tawa OOO till Jul 10th
Modified:
5 years, 8 months ago
CC:
chromium-reviews, donnd+watch_chromium.org, zea+watch_chromium.org, tzik, dhollowa+watch_chromium.org, dougw+watch_chromium.org, skanuj+watch_chromium.org, noyau+watch_chromium.org, aandrey+blink_chromium.org, stevenjb+watch_chromium.org, tim+watch_chromium.org, extensions-reviews_chromium.org, melevin+watch_chromium.org, dzhioev+watch_chromium.org, pvalenzuela+watch_chromium.org, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, dbeam+watch-options_chromium.org, nhiroki, jfweitz+watch_chromium.org, pam+watch_chromium.org, oshima+watch_chromium.org, Jered, maniscalco+watch_chromium.org, chrome-apps-syd-reviews_chromium.org, michaelpg+watch-options_chromium.org, tfarina, maxbogue+watch_chromium.org, yurys, David Black, davemoore+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, James Su, kinuko+fileapi, pfeldman
Base URL:
https://chromium.googlesource.com/chromium/src.git@priv
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Replace SetAuthenticatedUsername with SetAuthenticatedAccountId. Depends on https://codereview.chromium.org/964553002/ BUG=341408 Committed: https://crrev.com/a1e15d29e401465bacd0681f256af567e7b8242f Cr-Commit-Position: refs/heads/master@{#325594}

Patch Set 1 : rebased #

Total comments: 45

Patch Set 2 : Address review comments #

Patch Set 3 : rebased #

Patch Set 4 : Address review comments #

Total comments: 9

Patch Set 5 : Address review comments #

Patch Set 6 : rebased #

Patch Set 7 : Fix gaia-id #

Total comments: 4

Patch Set 8 : Fix order of init #

Total comments: 14

Patch Set 9 : Fix sync tests #

Patch Set 10 : rebased #

Total comments: 5

Patch Set 11 : Address review comments #

Total comments: 2

Patch Set 12 : Remove commented line #

Patch Set 13 : To try android bots #

Patch Set 14 : rebased #

Total comments: 2

Patch Set 15 : forsync #

Patch Set 16 : rebased #

Patch Set 17 : Fix gn files #

Patch Set 18 : merged #

Patch Set 19 : Fix merge problem #

Patch Set 20 : Fix android test #

Patch Set 21 : rebased #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+986 lines, -438 lines) Patch
M chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +18 lines, -0 lines 0 comments Download
M chrome/browser/app_controller_mac_unittest.mm View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/autocomplete/search_provider_unittest.cc View 3 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/autofill/personal_data_manager_factory.cc View 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/file_manager/file_manager_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/existing_user_controller_browsertest.cc View 8 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/login_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/login_manager_test.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/login_manager_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/session/stub_login_session_manager_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/login/session/user_session_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +12 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/login/signin/oauth2_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 15 chunks +40 lines, -20 lines 0 comments Download
M chrome/browser/chromeos/login/supervised/supervised_user_test_base.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/policy/consumer_enrollment_handler_unittest.cc View 3 chunks +17 lines, -9 lines 0 comments Download
M chrome/browser/devtools/device/webrtc/devtools_bridge_client_browsertest.cc View 3 chunks +22 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api_unittest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/identity/identity_apitest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +12 lines, -10 lines 0 comments Download
M chrome/browser/extensions/api/screenlock_private/screenlock_private_apitest.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/external_provider_impl_chromeos_unittest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/policy/cloud/cloud_policy_browsertest.cc View 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 10 chunks +40 lines, -23 lines 0 comments Download
M chrome/browser/prefs/chrome_pref_service_factory.cc View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/profiles/gaia_info_update_service_unittest.cc View 4 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/services/gcm/fake_signin_manager.cc View 3 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/signin/account_reconcilor_unittest.cc View 1 2 3 4 5 6 7 8 9 34 chunks +129 lines, -74 lines 0 comments Download
M chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/signin/fake_signin_manager.h View 2 chunks +9 lines, -3 lines 0 comments Download
M chrome/browser/signin/fake_signin_manager.cc View 4 chunks +18 lines, -9 lines 0 comments Download
M chrome/browser/signin/signin_global_error_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/signin/signin_manager_factory.cc View 1 2 3 3 chunks +12 lines, -3 lines 0 comments Download
M chrome/browser/signin/signin_manager_unittest.cc View 10 chunks +82 lines, -21 lines 0 comments Download
M chrome/browser/signin/signin_names_io_thread_unittest.cc View 1 2 chunks +8 lines, -1 line 0 comments Download
M chrome/browser/signin/signin_tracker_unittest.cc View 1 4 chunks +22 lines, -3 lines 0 comments Download
M chrome/browser/supervised_user/supervised_user_service_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_autofill_unittest.cc View 1 2 3 4 5 6 7 8 5 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_mock.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_startup_unittest.cc View 1 2 3 4 5 6 7 8 22 chunks +62 lines, -38 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_typed_url_unittest.cc View 4 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_unittest.cc View 5 chunks +12 lines, -3 lines 0 comments Download
M chrome/browser/sync/sync_ui_util_unittest.cc View 1 2 3 4 5 6 7 8 4 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/sync/test/integration/profile_sync_service_harness.cc View 1 2 3 4 5 6 7 8 3 chunks +13 lines, -3 lines 0 comments Download
M chrome/browser/ui/app_list/speech_auth_helper_unittest.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc View 3 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc View 3 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/ui/autofill/country_combobox_model_unittest.cc View 2 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/search/search_tab_helper_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/sync/one_click_signin_sync_starter.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/sync/one_click_signin_sync_starter.cc View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/sync/one_click_signin_sync_starter_unittest.cc View 3 chunks +8 lines, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bubble_view_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options/chromeos/shared_options_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options/options_ui_browsertest.cc View 3 chunks +7 lines, -2 lines 1 comment Download
M chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/signin/inline_login_handler_impl.cc View 1 2 3 chunks +5 lines, -8 lines 0 comments Download
M chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/base/chrome_render_view_host_test_harness.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/test/data/webui/ntp4_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chromeos/login/auth/stub_authenticator.cc View 1 2 3 4 1 chunk +4 lines, -1 line 0 comments Download
M components/autofill.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +2 lines, -0 lines 0 comments Download
M components/autofill/core/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
M components/autofill/core/browser/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_metrics_unittest.cc View 7 chunks +23 lines, -3 lines 0 comments Download
M components/autofill/core/browser/autofill_test_utils.cc View 2 chunks +13 lines, -1 line 0 comments Download
M components/autofill/core/browser/personal_data_manager.h View 4 chunks +10 lines, -0 lines 0 comments Download
M components/autofill/core/browser/personal_data_manager.cc View 1 2 3 4 5 6 7 3 chunks +9 lines, -2 lines 0 comments Download
M components/autofill/core/browser/personal_data_manager_unittest.cc View 5 chunks +27 lines, -1 line 0 comments Download
M components/signin/core/browser/account_tracker_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -2 lines 0 comments Download
M components/signin/core/browser/account_tracker_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +9 lines, -6 lines 0 comments Download
M components/signin/core/browser/signin_manager.h View 7 chunks +16 lines, -14 lines 0 comments Download
M components/signin/core/browser/signin_manager.cc View 1 11 chunks +59 lines, -38 lines 0 comments Download
M components/signin/core/browser/signin_manager_base.h View 1 2 3 6 chunks +23 lines, -13 lines 0 comments Download
M components/signin/core/browser/signin_manager_base.cc View 1 2 3 3 chunks +81 lines, -49 lines 0 comments Download
M components/signin/core/common/signin_pref_names.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M components/signin/core/common/signin_pref_names.cc View 1 2 3 2 chunks +11 lines, -1 line 0 comments Download

Messages

Total messages: 114 (68 generated)
Roger Tawa OOO till Jul 10th
Hi Mike, Please take a look. Thanks.
5 years, 8 months ago (2015-04-08 12:46:18 UTC) #56
Mike Lerman
Hi Roger, I looked at components/signin, c/b/signin, also inline login handler and the profiles test. ...
5 years, 8 months ago (2015-04-08 14:45:28 UTC) #57
Roger Tawa OOO till Jul 10th
Thanks Mike, see below. https://codereview.chromium.org/964563002/diff/1040001/chrome/browser/signin/signin_names_io_thread_unittest.cc File chrome/browser/signin/signin_names_io_thread_unittest.cc (right): https://codereview.chromium.org/964563002/diff/1040001/chrome/browser/signin/signin_names_io_thread_unittest.cc#newcode76 chrome/browser/signin/signin_names_io_thread_unittest.cc:76: service->SeedAccountInfo(gaia_id, email); On 2015/04/08 14:45:26, ...
5 years, 8 months ago (2015-04-08 20:24:21 UTC) #58
Mike Lerman
Very cool! Congrats on getting this out :) LGTM. https://codereview.chromium.org/964563002/diff/1040001/components/signin/core/browser/signin_manager_base.cc File components/signin/core/browser/signin_manager_base.cc (right): https://codereview.chromium.org/964563002/diff/1040001/components/signin/core/browser/signin_manager_base.cc#newcode51 components/signin/core/browser/signin_manager_base.cc:51: ...
5 years, 8 months ago (2015-04-08 20:49:28 UTC) #59
Roger Tawa OOO till Jul 10th
Thanks Mike. Comments addressed. https://codereview.chromium.org/964563002/diff/1040001/components/signin/core/browser/signin_manager_base.cc File components/signin/core/browser/signin_manager_base.cc (right): https://codereview.chromium.org/964563002/diff/1040001/components/signin/core/browser/signin_manager_base.cc#newcode51 components/signin/core/browser/signin_manager_base.cc:51: client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUserAccountId); On 2015/04/08 20:49:28, Mike ...
5 years, 8 months ago (2015-04-09 15:59:47 UTC) #60
Roger Tawa OOO till Jul 10th
Hi Evan, Xiyuan, Can you please take a look? These are mostly test changes because ...
5 years, 8 months ago (2015-04-09 16:05:07 UTC) #62
xiyuan
Mostly good. https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc File chrome/browser/chromeos/login/session/user_session_manager.cc (right): https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc#newcode937 chrome/browser/chromeos/login/session/user_session_manager.cc:937: signin_manager->SetAuthenticatedAccountInfo(user_context.GetGaiaID(), Did you mean to use |gaia_id| ...
5 years, 8 months ago (2015-04-09 20:59:21 UTC) #63
xiyuan
https://codereview.chromium.org/964563002/diff/1100001/chromeos/login/auth/stub_authenticator.cc File chromeos/login/auth/stub_authenticator.cc (right): https://codereview.chromium.org/964563002/diff/1100001/chromeos/login/auth/stub_authenticator.cc#newcode40 chromeos/login/auth/stub_authenticator.cc:40: if (expected_user_context_.GetUserID() == user_context.GetUserID()) { On 2015/04/09 20:59:21, xiyuan ...
5 years, 8 months ago (2015-04-09 21:02:12 UTC) #64
Roger Tawa OOO till Jul 10th
Thanks Xiyuan. Comments addressed, see below. https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc File chrome/browser/chromeos/login/session/user_session_manager.cc (right): https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc#newcode937 chrome/browser/chromeos/login/session/user_session_manager.cc:937: signin_manager->SetAuthenticatedAccountInfo(user_context.GetGaiaID(), On 2015/04/09 ...
5 years, 8 months ago (2015-04-10 01:52:57 UTC) #65
xiyuan
https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc File chrome/browser/chromeos/login/session/user_session_manager.cc (right): https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc#newcode937 chrome/browser/chromeos/login/session/user_session_manager.cc:937: signin_manager->SetAuthenticatedAccountInfo(user_context.GetGaiaID(), On 2015/04/10 01:52:57, Roger Tawa wrote: > On ...
5 years, 8 months ago (2015-04-10 02:50:45 UTC) #66
Roger Tawa OOO till Jul 10th
Good catch Xiyuan, thanks. https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc File chrome/browser/chromeos/login/session/user_session_manager.cc (right): https://codereview.chromium.org/964563002/diff/1100001/chrome/browser/chromeos/login/session/user_session_manager.cc#newcode937 chrome/browser/chromeos/login/session/user_session_manager.cc:937: signin_manager->SetAuthenticatedAccountInfo(user_context.GetGaiaID(), On 2015/04/10 02:50:45, xiyuan ...
5 years, 8 months ago (2015-04-10 12:22:13 UTC) #67
xiyuan
chromeos/* LGTM
5 years, 8 months ago (2015-04-10 13:54:17 UTC) #68
Evan Stade
sorry for delay review, was ooo wednesday then it slipped under my radar yesterday. https://codereview.chromium.org/964563002/diff/1160001/components/autofill/core/browser/personal_data_manager.cc ...
5 years, 8 months ago (2015-04-10 13:56:15 UTC) #69
Roger Tawa OOO till Jul 10th
Thanks Evan. See comment below. https://codereview.chromium.org/964563002/diff/1160001/components/autofill/core/browser/personal_data_manager.cc File components/autofill/core/browser/personal_data_manager.cc (right): https://codereview.chromium.org/964563002/diff/1160001/components/autofill/core/browser/personal_data_manager.cc#newcode255 components/autofill/core/browser/personal_data_manager.cc:255: SetPrefService(pref_service); On 2015/04/10 13:56:15, ...
5 years, 8 months ago (2015-04-10 14:35:34 UTC) #70
Roger Tawa OOO till Jul 10th
Thanks Evan. See comment below. https://codereview.chromium.org/964563002/diff/1160001/components/autofill/core/browser/personal_data_manager.cc File components/autofill/core/browser/personal_data_manager.cc (right): https://codereview.chromium.org/964563002/diff/1160001/components/autofill/core/browser/personal_data_manager.cc#newcode255 components/autofill/core/browser/personal_data_manager.cc:255: SetPrefService(pref_service); On 2015/04/10 13:56:15, ...
5 years, 8 months ago (2015-04-10 14:35:34 UTC) #71
Roger Tawa OOO till Jul 10th
Hi Drew, Ken, Nicolas, Zel, Can you please take a look? These are mostly test ...
5 years, 8 months ago (2015-04-10 14:54:30 UTC) #73
Evan Stade
lgtm
5 years, 8 months ago (2015-04-10 19:56:32 UTC) #74
Nicolas Zea
It seems like it's pretty easy to accidentally swap an account id with an account ...
5 years, 8 months ago (2015-04-10 20:15:29 UTC) #75
Roger Tawa OOO till Jul 10th
Both gaia-id and email are strings. While email has a pretty well known form, gaia-id ...
5 years, 8 months ago (2015-04-11 00:23:08 UTC) #76
Nicolas Zea
sync LGTM
5 years, 8 months ago (2015-04-11 00:27:39 UTC) #77
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964563002/1200001
5 years, 8 months ago (2015-04-11 02:29:38 UTC) #80
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/55694)
5 years, 8 months ago (2015-04-11 02:41:03 UTC) #82
Roger Tawa OOO till Jul 10th
Hi Bo, Can you please take at android_webview/ ? There are only gyp changes. Thanks.
5 years, 8 months ago (2015-04-13 14:32:25 UTC) #84
Andrew T Wilson (Slow)
LGTM with a couple nits/questions https://codereview.chromium.org/964563002/diff/1220001/chrome/browser/policy/cloud/cloud_policy_browsertest.cc File chrome/browser/policy/cloud/cloud_policy_browsertest.cc (right): https://codereview.chromium.org/964563002/diff/1220001/chrome/browser/policy/cloud/cloud_policy_browsertest.cc#newcode89 chrome/browser/policy/cloud/cloud_policy_browsertest.cc:89: return "gaia-id-user@example.com"; Why is ...
5 years, 8 months ago (2015-04-13 14:32:53 UTC) #85
boliu
https://codereview.chromium.org/964563002/diff/1220001/android_webview/android_webview.gyp File android_webview/android_webview.gyp (right): https://codereview.chromium.org/964563002/diff/1220001/android_webview/android_webview.gyp#newcode169 android_webview/android_webview.gyp:169: '../components/components.gyp:signin_core_browser', It's not immediately clear to me why this ...
5 years, 8 months ago (2015-04-13 15:35:52 UTC) #86
Roger Tawa OOO till Jul 10th
Thanks Drew. Comments addressed, see below. I also noticed that kTestUser was being used where ...
5 years, 8 months ago (2015-04-13 15:44:57 UTC) #87
Ken Rockot(use gerrit already)
c/b/extensions lgtm https://codereview.chromium.org/964563002/diff/1240001/chrome/browser/extensions/api/identity/identity_apitest.cc File chrome/browser/extensions/api/identity/identity_apitest.cc (right): https://codereview.chromium.org/964563002/diff/1240001/chrome/browser/extensions/api/identity/identity_apitest.cc#newcode586 chrome/browser/extensions/api/identity/identity_apitest.cc:586: //SignIn(account_key); nit: remove this
5 years, 8 months ago (2015-04-13 16:03:01 UTC) #88
Roger Tawa OOO till Jul 10th
On 2015/04/13 15:35:52, boliu wrote: > https://codereview.chromium.org/964563002/diff/1220001/android_webview/android_webview.gyp > File android_webview/android_webview.gyp (right): > > https://codereview.chromium.org/964563002/diff/1220001/android_webview/android_webview.gyp#newcode169 > ...
5 years, 8 months ago (2015-04-13 16:38:54 UTC) #89
zel
lgtm
5 years, 8 months ago (2015-04-13 16:56:39 UTC) #90
Roger Tawa OOO till Jul 10th
Thanks Ken. https://codereview.chromium.org/964563002/diff/1240001/chrome/browser/extensions/api/identity/identity_apitest.cc File chrome/browser/extensions/api/identity/identity_apitest.cc (right): https://codereview.chromium.org/964563002/diff/1240001/chrome/browser/extensions/api/identity/identity_apitest.cc#newcode586 chrome/browser/extensions/api/identity/identity_apitest.cc:586: //SignIn(account_key); On 2015/04/13 16:03:01, Ken Rockot wrote: ...
5 years, 8 months ago (2015-04-13 17:45:10 UTC) #91
Roger Tawa OOO till Jul 10th
Hi Mark, Can you please look at the Mac files? These are mostly test changes ...
5 years, 8 months ago (2015-04-13 20:37:50 UTC) #93
Mark Mentovai
LGTM. I only looked at the .mm files.
5 years, 8 months ago (2015-04-13 20:44:33 UTC) #94
Roger Tawa OOO till Jul 10th
Hi Bernhard, Colin, Scott, Can you please take a look? These are mostly test changes ...
5 years, 8 months ago (2015-04-14 16:43:05 UTC) #97
blundell
https://codereview.chromium.org/964563002/diff/1300001/components/BUILD.gn File components/BUILD.gn (right): https://codereview.chromium.org/964563002/diff/1300001/components/BUILD.gn#newcode265 components/BUILD.gn:265: "//components/signin/core/browser:test_support", hmm, is there no //components/signin/core/browser:unit_tests target?
5 years, 8 months ago (2015-04-14 17:30:58 UTC) #98
Andrew T Wilson (Slow)
still lgtm
5 years, 8 months ago (2015-04-14 17:37:02 UTC) #99
sky
LGTM
5 years, 8 months ago (2015-04-14 20:23:50 UTC) #100
blundell
https://codereview.chromium.org/964563002/diff/1300001/components/BUILD.gn File components/BUILD.gn (right): https://codereview.chromium.org/964563002/diff/1300001/components/BUILD.gn#newcode265 components/BUILD.gn:265: "//components/signin/core/browser:test_support", On 2015/04/14 17:30:58, blundell wrote: > hmm, is ...
5 years, 8 months ago (2015-04-15 08:45:19 UTC) #101
Bernhard Bauer
prefs/ and supervised_user/ LGTM.
5 years, 8 months ago (2015-04-15 10:14:49 UTC) #102
Roger Tawa OOO till Jul 10th
Thanks Colin. I see now. Undid changes to components/BUILD.gn and made appropriate change to components/autofill/core/browser/BUILD.gn. ...
5 years, 8 months ago (2015-04-15 19:03:14 UTC) #103
blundell
lgtm
5 years, 8 months ago (2015-04-15 19:57:17 UTC) #104
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964563002/1430001
5 years, 8 months ago (2015-04-16 21:43:14 UTC) #107
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964563002/1450001
5 years, 8 months ago (2015-04-17 02:07:49 UTC) #110
commit-bot: I haz the power
Committed patchset #21 (id:1450001)
5 years, 8 months ago (2015-04-17 04:16:51 UTC) #111
commit-bot: I haz the power
Patchset 21 (id:??) landed as https://crrev.com/a1e15d29e401465bacd0681f256af567e7b8242f Cr-Commit-Position: refs/heads/master@{#325594}
5 years, 8 months ago (2015-04-17 04:17:52 UTC) #112
Evan Stade
options_ui_browsertest.cc lgtm https://codereview.chromium.org/964563002/diff/1450001/chrome/browser/ui/webui/options/options_ui_browsertest.cc File chrome/browser/ui/webui/options/options_ui_browsertest.cc (right): https://codereview.chromium.org/964563002/diff/1450001/chrome/browser/ui/webui/options/options_ui_browsertest.cc#newcode285 chrome/browser/ui/webui/options/options_ui_browsertest.cc:285: EXPECT_TRUE(browser()->profile()->GetProfileUserName() != user); nit: EXPECT_NE
5 years, 8 months ago (2015-04-17 18:08:49 UTC) #113
Evan Stade
5 years, 8 months ago (2015-04-17 18:08:51 UTC) #114
Message was sent while issue was closed.
options_ui_browsertest.cc lgtm

https://codereview.chromium.org/964563002/diff/1450001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/options_ui_browsertest.cc (right):

https://codereview.chromium.org/964563002/diff/1450001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/options_ui_browsertest.cc:285:
EXPECT_TRUE(browser()->profile()->GetProfileUserName() != user);
nit: EXPECT_NE

Powered by Google App Engine
This is Rietveld 408576698