New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 921025 link

Starred by 1 user
Status: Started
Owner:
treib@chromium.org
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Android: "Chrome Sync" and "Master Sync" toggles should be orthogonal

Project Member Reported by treib@chromium.org, Jan 11 
On Android, there are two OS-level toggles that control Sync: "Chrome Sync" and "Master Sync".
"Chrome Sync" essentially maps to DISABLE_REASON_USER_CHOICE aka the IsSyncRequested pref. "Master Sync" maps to DISABLE_REASON_PLATFORM_OVERRIDE, but it *also* sets IsSyncRequested.

This all requires some very careful handling of those states [1,2], and the meaning of "isSyncRequested" is quite muddy. It would be much simpler if the two things were orthogonal.

[1] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java?rcl=dd6d6cf97b6a95c258cb7deb49b6a3ead32ddad1&l=141
[2] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java?rcl=dd6d6cf97b6a95c258cb7deb49b6a3ead32ddad1&l=180

This is a followup to  bug 867901 .
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6827686a55d3ac193c5ea456053f6036405e52ce

commit 6827686a55d3ac193c5ea456053f6036405e52ce
Author: Marc Treib <treib@chromium.org>
Date: Mon Jan 14 11:01:12 2019

Improve Java SyncTests around starting/stopping and Android sync settings

Currently, the interaction between Android's "Chrome sync" setting
(mapping to DISABLE_REASON_USER_CHOICE) and "Master sync" setting
(mapping to DISABLE_REASON_PLATFORM_OVERRIDE) is quite subtle and
fragile.
I'm hoping to simplify this by making the two settings completely
orthogonal. As a first step, this CL improves the test coverage, and
adds some TODOs.
While we're here, also remove a FlakyTest annotation, since the test
doesn't seem to be flaky anymore.

Bug: 921025,  594558 
Change-Id: Idd00cdcb35c3bee1102da7071fc1394c91162ceb
Reviewed-on: https://chromium-review.googlesource.com/c/1406970
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622415}
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTest.java
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/browser/sync/profile_sync_service_android.cc
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/browser/sync/profile_sync_service_android.h
[modify] https://crrev.com/6827686a55d3ac193c5ea456053f6036405e52ce/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java
►

Sign in to add a comment