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

Issue 2747293005: Reimplement AccountManagerHelper singleton instance creation (Closed)

Created:
3 years, 9 months ago by bsazonov
Modified:
3 years, 8 months ago
CC:
chromium-reviews, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Reimplement AccountManagerHelper singleton instance creation Current implementation of AccountManagerHelper can instantiate wrong delegate type if get() is called in too early. This CL fixes it. AtomicReference is used to avoid relying on external synchronization, because get() can be called from different threads. This CL also adds AccountManagerHelper.resetAccountManagerHelperForTests method that makes sure there's no interference between tests. BUG=698258 Review-Url: https://codereview.chromium.org/2747293005 Cr-Commit-Position: refs/heads/master@{#463963} Committed: https://chromium.googlesource.com/chromium/src/+/1305003fecaca059c4e3a5678eb48f0a00a46317

Patch Set 1 #

Patch Set 2 : Fixed tests #

Patch Set 3 : Rebased, fixed tests and added AccountManagerHelper.resetAccountManagerHelperForTests #

Patch Set 4 : Rebased again #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -22 lines) Patch
M chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PasswordViewingTypeTest.java View 1 2 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java View 1 2 3 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/signin/SigninHelperTest.java View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/signin/SigninTest.java View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderTest.java View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/util/FeatureUtilitiesTest.java View 1 2 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/android/junit/src/org/chromium/chrome/browser/invalidation/InvalidationControllerTest.java View 1 2 3 4 chunks +11 lines, -0 lines 0 comments Download
M chrome/android/junit/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderUnitTest.java View 1 2 3 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/signin/SigninTestUtil.java View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M components/signin/core/browser/account_tracker_service_unittest.cc View 1 2 chunks +3 lines, -0 lines 0 comments Download
M components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerHelper.java View 1 2 3 4 chunks +22 lines, -20 lines 0 comments Download
M components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M components/signin/core/browser/android/javatests/src/org/chromium/components/signin/test/AccountManagerHelperTest.java View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M components/signin/core/browser/child_account_info_fetcher.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M components/signin/core/browser/child_account_info_fetcher.cc View 1 1 chunk +6 lines, -0 lines 0 comments Download
M components/signin/core/browser/child_account_info_fetcher_android.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M components/signin/core/browser/child_account_info_fetcher_android.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download
M components/sync/android/javatests/src/org/chromium/components/sync/AndroidSyncSettingsTest.java View 1 2 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 74 (68 generated)
bsazonov
Roger, please review changes in components/signin/. Tommy, please review changes in chrome/ and components/sync/.
3 years, 8 months ago (2017-03-31 15:55:42 UTC) #54
nyquist
lgtm
3 years, 8 months ago (2017-04-04 05:53:28 UTC) #58
Roger Tawa OOO till Jul 10th
lgtm
3 years, 8 months ago (2017-04-11 14:14:47 UTC) #59
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2747293005/180001
3 years, 8 months ago (2017-04-12 08:18:05 UTC) #70
commit-bot: I haz the power
Committed patchset #4 (id:180001) as https://chromium.googlesource.com/chromium/src/+/1305003fecaca059c4e3a5678eb48f0a00a46317
3 years, 8 months ago (2017-04-12 08:28:27 UTC) #73
Donn Denman
3 years, 8 months ago (2017-04-12 16:39:07 UTC) #74
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:180001) has been created in
https://codereview.chromium.org/2817723002/ by donnd@chromium.org.

The reason for reverting is: Caused multiple bot failures, see
crbug.com/710901..

Powered by Google App Engine
This is Rietveld 408576698