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

Issue 1062973004: [Sync] Ensure isSyncable is set when signed in. (Closed)

Created:
5 years, 8 months ago by maxbogue
Modified:
5 years, 8 months ago
Reviewers:
nyquist
CC:
chromium-reviews, tim+watch_chromium.org, zea+watch_chromium.org, maxbogue+watch_chromium.org, pvalenzuela+watch_chromium.org, plaree+watch_chromium.org, maniscalco+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Sync] Ensure isSyncable is set when signed in. This is a more extensive version of http://crrev.com/1075343003 for landing on trunk. Previously, if isSyncable is false, but sync is enabled for Chrome, even if you sign in, Chrome would never set syncable to true. This CL ensures that syncable is always set to true if there is an account signed in, regardless of whether chrome sync is enabled or not. Additionally, it sets syncable to false when no account is signed in, to ensure that the user does not see a non-functional switch in the Android settings. The new AccountManager setup in the test is necessary so that the test accounts can be cleaned up by the loop in updateSyncability(). BUG=475299 TEST=Regression tests added, plus: Ensure that $ adb shell dumpsys content | grep chrome should show syncable=0 and enabled=true Sign in to Chrome. Now dumpsys should say syncable=1 and enabled=true. Committed: https://crrev.com/fd45731d49e2274221be3df2bd2fb3c8b0bed613 Cr-Commit-Position: refs/heads/master@{#326122}

Patch Set 1 #

Patch Set 2 : Improve a comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -13 lines) Patch
M sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java View 1 3 chunks +11 lines, -6 lines 0 comments Download
M sync/android/javatests/src/org/chromium/sync/AndroidSyncSettingsTest.java View 4 chunks +56 lines, -2 lines 0 comments Download
M sync/test/android/javatests/src/org/chromium/sync/test/util/MockSyncContentResolverDelegate.java View 2 chunks +1 line, -5 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
maxbogue
PTAL. I modified your patch so that things are updated when the account changes as ...
5 years, 8 months ago (2015-04-20 22:39:25 UTC) #2
nyquist
lgtm
5 years, 8 months ago (2015-04-21 21:14:28 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1062973004/20001
5 years, 8 months ago (2015-04-21 21:17:08 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 8 months ago (2015-04-21 21:22:16 UTC) #6
commit-bot: I haz the power
5 years, 8 months ago (2015-04-21 21:23:04 UTC) #7
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/fd45731d49e2274221be3df2bd2fb3c8b0bed613
Cr-Commit-Position: refs/heads/master@{#326122}

Powered by Google App Engine
This is Rietveld 408576698