Add social blocking options #1818

bbondy · 2019-03-01T15:42:12Z

Fix brave/brave-browser#3489
Fix brave/brave-browser#3534

Related PRs:
brave/adblock-lists#54
brave/ad-block#192

The only default behaviour that this PR changes is that a cookie for Google login buttons is now allowed. It adds an option to be able to disable it though.
It also adds 3 options to be able to disable previously always-allowed things (Facebook, Twitter, LinkedIn embeds and login buttons).

It looks like this:

Test plan is in the issue.

Submitter Checklist:

Test Plan:

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

bbondy · 2019-03-11T03:03:34Z

components/brave_shields/browser/ad_block_base_service.cc

@@ -163,7 +183,6 @@ void AdBlockBaseService::OnDATFileDataReady() {
    LOG(ERROR) << "Could not obtain ad block data";
    return;
  }
-  ad_block_client_.reset(new AdBlockClient());


This would remove the already added tags on the instance. Deserialize should already be resetting all members to a valid state.

bbondy · 2019-03-12T04:28:32Z

DEPS

@@ -1,7 +1,7 @@
 use_relative_paths = True

 deps = {
-  "vendor/ad-block": "https://github.com/brave/ad-block.git@534c153a3e8d1c8eda02cc80297b9abf87e9a7da",
+  "vendor/ad-block": "https://github.com/brave/ad-block.git@8ddfa911aeaf044741a9d5b66ddd84c9a3c352e7",


FYI: This will need to be updated once I merge brave/ad-block

bbondy · 2019-03-12T04:29:36Z

browser/autoplay/autoplay_permission_context_unittest.cc

+      RegisterBooleanPref(kGoogleLoginControlType, true);
+    builder.SetPrefService(std::move(prefs));
+    return builder.Build().release();
+  }


This was needed otherwise a new unregistered pref error occurs only for 1 test in this file.

bbondy · 2019-03-12T04:30:01Z

browser/brave_content_browser_client.cc

@@ -167,7 +168,8 @@ bool BraveContentBrowserClient::AllowAccessCookie(
  content_settings::BraveCookieSettings* cookie_settings =
      (content_settings::BraveCookieSettings*)io_data->GetCookieSettings();
  bool allow = !ShouldBlockCookie(allow_brave_shields, allow_1p_cookies,
-                   allow_3p_cookies, first_party, url) &&
+                   allow_3p_cookies, first_party, url,
+                   cookie_settings->GetAllowGoogleAuth()) &&


ShouldBlockCookie will need a refactor but not in the scope of this issue.

Agreed on both counts

bbondy · 2019-03-12T04:30:43Z

browser/brave_profile_prefs.cc

@@ -47,6 +47,10 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
  // Default Brave shields
  registry->RegisterBooleanPref(kHTTPSEVerywhereControlType, true);
  registry->RegisterBooleanPref(kNoScriptControlType, false);
+  registry->RegisterBooleanPref(kGoogleLoginControlType, true);


This is the only thing that actually changes default behaviour in the context of this ticket.

bbondy · 2019-03-12T04:32:15Z

browser/net/brave_network_delegate_base.cc

+      kTwitterEmbedControlType,
+      base::Bind(&BraveNetworkDelegateBase::OnPreferenceChanged,
+                 base::Unretained(this), kTwitterEmbedControlType));
+  user_pref_change_registrar_->Add(


I first tried adding these observers inside of the ad block base class but it caused problems because of threading, it doesn't work from the main thread which prefs needed to.

bbondy · 2019-03-12T04:34:39Z

components/content_settings/core/browser/brave_cookie_settings.cc

 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "brave/common/brave_cookie_blocking.h"
+#include "brave/common/pref_names.h"
+#include "components/prefs/pref_service.h"


components/prefs is already a dep in this BUILD.gn

bbondy · 2019-03-12T04:35:13Z

patches/chrome-browser-resources-settings-basic_page-basic_page.html.patch

@@ -29,6 +30,13 @@ index 65d9633a7c0e4b0b44dd8a0ae2bb7598b3ee1b06..c6a6980abc74d80c8d3b8fa9104c6494
 +            <settings-default-brave-shields-page  prefs="{{prefs}}"></settings-default-brave-shields-page>
 +          </settings-section>
 +        </template>
+        <template is="dom-if" if="[[showPage_(pageVisibility.socialBlocking)]]"


Screenshot available in the issue and PR.


        1st-party-independent cookie exception for Google auth (fixes brave/b…

…rave-browser#3534) This also fixes brave/brave-browser#3115 and brave/brave-browser#1179.


        Add social blocking option controls UI


        Update DEPS


        Hookup social blocking options to ad-block tagging


        Allow Google login cookie only if option is on


        Fix lint


        Add tests for ad-blocking with tagging


        Add Cookie option test and fix tests


        Combine fb-embeds and fb-login tags


        Break social blocking into its own page


        Add tests for prefs to tags mapping


        Bind -> BindRepeating | BindOnce


        Fix unresolved external in brave_cookie_settings on linux


        Merge pull request #1818 from brave/social-blocking-options

Add social blocking options


        Merge pull request #1818 from brave/social-blocking-options

Add social blocking options

simonhong · 2019-03-18T08:58:16Z

browser/resources/settings/social_blocking_page/social_blocking_browser_proxy.js

+  /**
+   * @implements {settings.SocialBlockingBrowserProxy}
+   */
+  class DefaultBraveShieldsBrowserProxyImpl {


This should be SocialBlockingBrowserProxyImpl?

simonhong · 2019-03-18T08:58:16Z

browser/resources/settings/social_blocking_page/social_blocking_browser_proxy.js

+  class DefaultBraveShieldsBrowserProxyImpl {
+  }
+
+  cr.addSingletonGetter(DefaultBraveShieldsBrowserProxyImpl);


iefremov · 2019-07-21T22:27:36Z

browser/net/brave_network_delegate_base.cc

+    const std::string& pref_name) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  PrefService* user_prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
+  allow_google_auth_ = user_prefs->GetBoolean(kGoogleLoginControlType);


it is not thread-safe to write allow_google_auth_ on UI and later read on IO

bbondy added the pr/work-in-progress label Mar 1, 2019

bbondy self-assigned this Mar 1, 2019

bbondy removed the pr/work-in-progress label Mar 1, 2019

bbondy force-pushed the social-blocking-options branch 10 times, most recently from 3c25dd3 to e4bc0dc Mar 4, 2019

bbondy force-pushed the social-blocking-options branch 5 times, most recently from aa7de48 to b175246 Mar 11, 2019

bbondy reviewed Mar 11, 2019

View changes

bbondy force-pushed the social-blocking-options branch from b175246 to 1906125 Mar 11, 2019

bbondy changed the title ~~WIP: Add social blocking options~~ Add social blocking options Mar 11, 2019

bbondy force-pushed the social-blocking-options branch 2 times, most recently from c5a9534 to e6a4f5c Mar 12, 2019

bbondy reviewed Mar 12, 2019

View changes

bbondy force-pushed the social-blocking-options branch from 9c65196 to f80b28d Mar 13, 2019

This was referenced Mar 13, 2019

Block social sharing buttons as trackers by default brave/brave-browser#3241

Open

Prevent Social Buttons from Tracking by replacing them with placeholders brave/brave-browser#534

Open

bbondy force-pushed the social-blocking-options branch from f80b28d to a0b44a6 Mar 14, 2019

Dénes Bán and others added 13 commits Mar 1, 2019

1st-party-independent cookie exception for Google auth (fixes brave/b…

ec009bb

…rave-browser#3534) This also fixes brave/brave-browser#3115 and brave/brave-browser#1179.

Add social blocking option controls UI

364de21

Update DEPS

2f8ebe3

Hookup social blocking options to ad-block tagging

136ef42

Allow Google login cookie only if option is on

a547c23

Fix lint

86e8ec6

Add tests for ad-blocking with tagging

404a406

Add Cookie option test and fix tests

9ddbcf4

Combine fb-embeds and fb-login tags

2220964

Break social blocking into its own page

cba5b16

Add tests for prefs to tags mapping

2a45096

Bind -> BindRepeating | BindOnce

169c927

Fix unresolved external in brave_cookie_settings on linux

Loading status checks…

853394b

bbondy force-pushed the social-blocking-options branch from 54e3222 to 853394b Mar 15, 2019

bbondy merged commit 5938e5a into master Mar 15, 2019
1 of 2 checks passed

1 of 2 checks passed

continuous-integration/jenkins/pr-head This commit cannot be built
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

bbondy added a commit that referenced this pull request Mar 15, 2019

Merge pull request #1818 from brave/social-blocking-options

Loading status checks…

6d2bda3

Add social blocking options

bbondy added this to the 0.64.x - Nightly milestone Mar 15, 2019

bbondy mentioned this pull request Mar 15, 2019

Add social blocking options #1968

Merged

bbondy added a commit that referenced this pull request Mar 15, 2019

Merge pull request #1818 from brave/social-blocking-options

Loading status checks…

616bad8

Add social blocking options

bbondy mentioned this pull request Mar 15, 2019

Add social blocking options #1972

Merged

simonhong reviewed Mar 18, 2019

View changes

NejcZdovc deleted the social-blocking-options branch Mar 18, 2019

bbondy mentioned this pull request Mar 18, 2019

Use correct social blocking proxy impl class name #3760

Closed

petemill mentioned this pull request Mar 18, 2019

WebUI: Settings & 4 different ways to override #1909

Merged

9 of 19 tasks complete

iefremov reviewed Jul 21, 2019

View changes

brave / brave-core

Add social blocking options #1818

Add social blocking options #1818

bbondy commented Mar 1, 2019 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

brave / brave-core

Join GitHub today

Add social blocking options #1818

Add social blocking options #1818

Conversation

bbondy commented Mar 1, 2019 • edited

Submitter Checklist:

Test Plan:

Reviewer Checklist:

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bbondy Mar 12, 2019 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bbondy commented Mar 1, 2019 •

edited

bbondy Mar 12, 2019 •

edited