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

Issue 923886 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

NOTREACHED if sync settings are saved while sync is disabled

Project Member Reported by bsazonov@chromium.org, Yesterday (38 hours ago)

Issue description

While implementing "Manage sync" page for https://crbug.com/914056, I've hit the following issue: if ProfileSyncService.setChosenDataTypes [1] is invoked right after ProfileSyncService.requestStart [2], it sometimes crashes Chrome by reaching NOTREACHED in ProfileSyncService::OnPreferredDataTypesPrefChange [3].

Fatal signal 6 (SIGABRT), code -6 in tid 15972 (oid.apps.chrome)
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'google/sailfish/sailfish:7.1.2/NJH47F/4146041:userdebug/dev-keys'
Revision: '0'
ABI: 'arm'
pid: 15972, tid: 15972, name: oid.apps.chrome  >>> com.google.android.apps.chrome <<<
signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
Abort message: '[FATAL:profile_sync_service.cc(1368)] Check failed: false. '
    r0 00000000  r1 00003e64  r2 00000006  r3 00000008
    r4 eb28a58c  r5 00000006  r6 eb28a534  r7 0000010c
    r8 ffe6fe3c  r9 0000003b  sl cd317070  fp e9cf7008
    ip 00000000  sp ffe6fdb0  lr e9cb25c7  pc e9cb4e30  cpsr 60000010

backtrace:
    #00 pc 00049e30  /system/lib/libc.so (tgkill+12)
    #01 pc 000475c3  /system/lib/libc.so (pthread_kill+34)
    #02 pc 0001d635  /system/lib/libc.so (raise+10)
    #03 pc 00019181  /system/lib/libc.so (__libc_android_abort+34)
    #04 pc 00017048  /system/lib/libc.so (abort+4)
    #05 pc 00143dbd  /data/app/com.google.android.apps.chrome-2/lib/arm/libbase.cr.so (_ZN4base5debug13BreakDebuggerEv+20)
    #06 pc 000e2f29  /data/app/com.google.android.apps.chrome-2/lib/arm/libbase.cr.so (_ZN7logging10LogMessageD2Ev+552)
    #07 pc 010720cf  /data/app/com.google.android.apps.chrome-2/lib/arm/libchrome.cr.so
    #08 pc 00c47ee9  /data/app/com.google.android.apps.chrome-2/lib/arm/libchrome.cr.so
    #09 pc 01074d5f  /data/app/com.google.android.apps.chrome-2/lib/arm/libchrome.cr.so
    #10 pc 00b6fa93  /data/app/com.google.android.apps.chrome-2/lib/arm/libchrome.cr.so
    #11 pc 00b6f9d9  /data/app/com.google.android.apps.chrome-2/lib/arm/libchrome.cr.so
    #12 pc 01bf7b4d  /data/app/com.google.android.apps.chrome-2/oat/arm/base.odex (offset 0x19fc000)

This is an important flow for "Manage sync" page, as it should disable sync when all the toggles for data types are turned off.

For the time being, I've moved sync disabling to onStop lifecycle method, but in a long run it makes sense applying state as soon as the user turns the sync on or off.

[1] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java?l=370&rcl=17145a3a105605a589b47b881d51fa0277f899dc
[2] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java?l=501&rcl=17145a3a105605a589b47b881d51fa0277f899dc
[3] https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?l=1368&rcl=c031c01b33a63540fe2debc5ce07ff07ff8e4d6e
 

Comment 1 by treib@google.com, Yesterday (38 hours ago)

I've wondered about that NOTREACHED before, and I don't think there's a good reason for having it. Maybe we can simply remove it.

Comment 2 by treib@chromium.org, Today (12 hours ago)

Owner: bsazonov@chromium.org
Status: Started (was: Untriaged)

Comment 3 by bsazonov@chromium.org, Today (11 hours ago)

Without that NOTREACHED I sometimes get the following line:

I chromium: [INFO:profile_sync_service.cc(1965)] ConfigureDataTypeManager not invoked because engine is not initialized

After this message, sync stays in "Sync in progress..." state and doesn't become active (judging by ProfileSyncService.isSyncActive()).
Project Member

Comment 4 by bugdroid1@chromium.org, Today (11 hours ago)

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

commit 222f6fe93e7faa548d7d7cda8a2bf7a4a0ee1096
Author: Boris Sazonov <bsazonov@chromium.org>
Date: Tue Jan 22 19:11:23 2019

[Sync] Remove NOTREACHED from OnPreferredDataTypesPrefChange

This CL removes NOTREACHED() from
ProfileSyncService::OnPreferredDataTypesPrefChange. This check was
triggered if setChosenDataTypes is invoked soon after requestStart.

Bug: 923886
Change-Id: I1a826ba3b5e1ee64aa0ecba6107e420d0551a7ee
Reviewed-on: https://chromium-review.googlesource.com/c/1426940
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624861}
[modify] https://crrev.com/222f6fe93e7faa548d7d7cda8a2bf7a4a0ee1096/components/browser_sync/profile_sync_service.cc

Sign in to add a comment