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

Fix 6804: Avoid the OnPreferenceChanged callback when preferences are changed by CookiePrefService #3903

Merged
merged 4 commits into from
Nov 11, 2019

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Nov 7, 2019

Fix brave/brave-browser#6804

Submitter Checklist:

Test Plan:

  1. Navigate to brave://settings/shields
  2. Toggle Cookie settings from Only block only cross-site cookies to Block All Cookies
  3. Refresh and verify the setting remains the same
  4. Toggle the setting to Block-Cross site cookies and refresh
  5. Verify the setting remains the same
  6. Navigate to https://www.whatismybrowser.com/detect/are-third-party-cookies-enabled and verify that third party cookies are blocked
  7. Toggle the setting to Block all cookies -> Allow all cookies
  8. Navigate to https://www.whatismybrowser.com/detect/are-third-party-cookies-enabled and verify that all cookies are allowed

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@jumde jumde self-assigned this Nov 7, 2019
@jumde jumde added the CI/skip Do not run CI builds (except noplatform) label Nov 7, 2019
@jumde jumde changed the title [WIP] Fix 6804: Avoid the OnPreferenceChanged callback when preferences are changed by CookiePrefService Fix 6804: Avoid the OnPreferenceChanged callback when preferences are changed by CookiePrefService Nov 7, 2019
@jumde jumde removed the CI/skip Do not run CI builds (except noplatform) label Nov 7, 2019
@jumde jumde force-pushed the cookie_pref_loop branch 3 times, most recently from 3df0d0b to 78e64a6 Compare November 8, 2019 08:10
@mihaiplesa mihaiplesa added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Nov 8, 2019
@mihaiplesa
Copy link
Collaborator

Has passed for Android, iOS, Linux, Windows so applied skip labels and will need to be re-built only on macOS (unless no new changes pushed).

@jumde jumde removed CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Nov 8, 2019
@jumde jumde force-pushed the cookie_pref_loop branch 2 times, most recently from 8f8ec26 to 51ceebd Compare November 9, 2019 00:35

Lock lock_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please write a detailed comment explaining this approach? It is sort of non-trivial to understand

@jumde jumde merged commit 698378e into master Nov 11, 2019
@jumde jumde deleted the cookie_pref_loop branch November 11, 2019 17:32
@jumde jumde added this to the 0.74.x - Nightly milestone Nov 12, 2019
petemill pushed a commit that referenced this pull request Jul 27, 2020
Make build step sign binaries and generate widevine sig files on Windows
petemill pushed a commit that referenced this pull request Jul 28, 2020
Make build step sign binaries and generate widevine sig files on Windows
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.

Global Shields toggle for cookies does not work
4 participants