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

Call ContentSettingsService::OnExtensionPrefsLoaded when adding component extensions #62

Merged
merged 3 commits into from
Mar 23, 2018

Conversation

yrliou
Copy link
Member

@yrliou yrliou commented Mar 21, 2018

Fixing brave/brave#96.

Summary of changes:

  1. Although we ensure an entry of ContentSettingsStore will be registered for component extensions when fixing brave/brave#81, content settings are not restored after restart since ContentSettingsService::OnExtensionPrefsLoaded wasn't triggered to populate the values into content setting store. This patch triggers it which leads to a call into ContentSettingsStore::SetExtensionContentSettingFromList to populate the values.

  2. Revise chromium's implementation of SetExtensionContentSettingFromList by delaying the content setting changed notification after all values in the list have been set. Before this change, I could only see the first entry from the list being saved into content setting store.

If there's a cleaner way to fix this, please feel free to suggest, thanks.

@yrliou yrliou requested a review from bbondy March 21, 2018 20:05
@yrliou yrliou force-pushed the restore_extension_content_settings branch 4 times, most recently from 9ed54a8 to 087443e Compare March 22, 2018 19:39
@yrliou yrliou force-pushed the restore_extension_content_settings branch from 087443e to c28967d Compare March 22, 2018 19:48
@yrliou
Copy link
Member Author

yrliou commented Mar 23, 2018

Reduce patching into chromium code directly by making subclasses per @bbondy's suggestion.
Built and run on Mac, content settings are restored after restarting the browser.

@bbondy
Copy link
Member

bbondy commented Mar 23, 2018

Much cleaner, thanks. 🎉

We just need to fixup patches/chrome-browser-extensions-extension_service.cc.patch at a future date. But you didn't introduce that here so merging.

@bbondy bbondy merged commit 8d2912b into master Mar 23, 2018
@yrliou yrliou deleted the restore_extension_content_settings branch March 27, 2018 06:07
bbondy pushed a commit that referenced this pull request Feb 18, 2019
set proper TARGET_GEN_DIR for dev builds
fmarier pushed a commit that referenced this pull request Oct 29, 2019
Add more support for transpiling CSS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants