Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash while migrating obsolete fingerprinting shields settings in C88 #13443

Closed
iefremov opened this issue Jan 8, 2021 · 10 comments · Fixed by brave/brave-core#7549
Closed

Comments

@iefremov
Copy link
Contributor

iefremov commented Jan 8, 2021

happens in 88.0.4324.51 if there are some fingerprinting (not fingerprintingV2) content settings to migrate.

[41370:775:0109/003930.781245:FATAL:pref_service.cc(546)] Check failed: false. Trying to get an unregistered pref: profile.content_settings.exceptions.fingerprinting
0   libbase.dylib                       0x000000011569e6a9 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   libbase.dylib                       0x000000011557fb03 base::debug::StackTrace::StackTrace() + 19
2   libbase.dylib                       0x00000001155a4e7f logging::LogMessage::~LogMessage() + 175
3   libbase.dylib                       0x00000001155a5d4e logging::LogMessage::~LogMessage() + 14
4   libprefs.dylib                      0x0000000122e0ed65 PrefService::GetMutableUserPref(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > const&, base::Value::Type) + 485
5   libchrome_dll.dylib                 0x000000010d2a7b50 prefs::ScopedDictionaryPrefUpdate::Get() + 80
6   libchrome_dll.dylib                 0x000000010d2a9402 content_settings::BravePrefProvider::MigrateShieldsSettingsFromResourceIdsForOneType(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > const&, base::Time const&, base::Time const&, content_settings::SessionModel, int) + 130
7   libchrome_dll.dylib                 0x000000010d2a90ac content_settings::BravePrefProvider::MigrateShieldsSettingsFromResourceIds() + 1468
8   libchrome_dll.dylib                 0x000000010d2a866b content_settings::BravePrefProvider::MigrateShieldsSettings(bool) + 27
9   libchrome_dll.dylib                 0x000000010d2a85ca content_settings::BravePrefProvider::BravePrefProvider(PrefService*, bool, bool, bool) + 442
10  libchrome_dll.dylib                 0x000000010f21a12d HostContentSettingsMap::HostContentSettingsMap(PrefService*, bool, bool, bool) + 605
11  libchrome_dll.dylib                 0x000000010e1681ea HostContentSettingsMapFactory::BuildServiceInstanceFor(content::BrowserContext*) const + 298
12  libkeyed_service_content.dylib      0x000000012396660f RefcountedBrowserContextKeyedServiceFactory::BuildServiceInstanceFor(void*) const + 15
13  libkeyed_service_core.dylib         0x000000011e77cb06 RefcountedKeyedServiceFactory::GetServiceForContext(void*, bool) + 198
14  libkeyed_service_content.dylib      0x000000012396657e RefcountedBrowserContextKeyedServiceFactory::GetServiceForBrowserContext(content::BrowserContext*, bool) + 14
15  libchrome_dll.dylib                 0x000000010e167f91 HostContentSettingsMapFactory::GetForProfile(content::BrowserContext*) + 97
16  libchrome_dll.dylib                 0x000000010de9359a brave_shields::CookiePrefServiceFactory::BuildServiceInstanceFor(content::BrowserContext*) const + 42
17  libkeyed_service_content.dylib      0x0000000123966035 BrowserContextKeyedServiceFactory::BuildServiceInstanceFor(void*) const + 21
18  libkeyed_service_core.dylib         0x000000011e77b363 KeyedServiceFactory::GetServiceForContext(void*, bool) + 227
@iefremov iefremov added crash QA/Yes release-notes/exclude OS/Android Fixes related to Android browser functionality OS/Desktop labels Jan 8, 2021
@iefremov iefremov self-assigned this Jan 8, 2021
iefremov added a commit to brave/brave-core that referenced this issue Jan 8, 2021
They were not migrated and thus anyways are not used,
so we can drop them to avoid troubles during migration from
PLUGINS world.

Fix brave/brave-browser#13443
@bsclifton bsclifton added this to the 1.20.x - Nightly milestone Jan 9, 2021
@kjozwiak
Copy link
Member

STR:

@kjozwiak
Copy link
Member

kjozwiak commented Jan 14, 2021

Verification PASSED on macOS 11.1 x64 using the following build:

Brave | 1.19.83 Chromium: 88.0.4324.79 (Official Build) (x86_64)
--- | ---
Revision | bd1e9353659b2491dac971226a973ca3b5684a14-refs/branch-heads/4324@{#1520}
OS | macOS Version 11.1 (Build 20C69)

Verification passed on

Brave | 1.19.83 Chromium: 88.0.4324.79 (Official Build) (64-bit)
-- | --
Revision | bd1e9353659b2491dac971226a973ca3b5684a14-refs/branch-heads/4324@{#1520}
OS | Windows 10 OS Version 2004 (Build 19041.685)


Verification passed on

Brave 1.19.83 Chromium: 88.0.4324.79 (Official Build) (64-bit)
Revision bd1e9353659b2491dac971226a973ca3b5684a14-refs/branch-heads/4324@{#1520}
OS Ubuntu 18.04 LTS

@kjozwiak
Copy link
Member

@iefremov is there a way this can be checked/QA'd on Android? Unfortunately we don't have the ability to edit/replace the Preference file. I guess just making sure upgrades and shields are working?

@iefremov
Copy link
Contributor Author

@kjozwiak I don't know, maybe @samartnik could help

@samartnik
Copy link
Contributor

@kjozwiak @iefremov in theory we could use older version where fingerprinting was used, make fingerprinting changes and try to upgrade it. But I'm not quite sure if it's a valid STR.

@srirambv
Copy link
Contributor

@samartnik isn't that basically #13522 which is failing right now?

@samartnik
Copy link
Contributor

samartnik commented Jan 14, 2021

@srirambv it's different, there settings were not migrating at all. But it's a good point, because you won't be able to reproduce this crash on Android. Older version doesn't migrate settings at all and newer version contains this fix.

@srirambv
Copy link
Contributor

So should we mark this issue as Desktop only for now then since we can get it to crash and new version will include the fix?

@samartnik
Copy link
Contributor

sgtm

@kjozwiak
Copy link
Member

As per the above, going to remove OS/Android 👍

@kjozwiak kjozwiak removed the OS/Android Fixes related to Android browser functionality label Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment