Skip to content

Commit ad63b66

Browse files
committed
Merge pull request #703 from brave/fix-tp-count
Count TP blocks separately on newtab page
1 parent 64d413f commit ad63b66

4 files changed

+41
-5
lines changed

browser/net/brave_ad_block_tp_network_delegate_helper.cc

+11-5
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,14 @@ void OnBeforeURLRequestAdBlockTPOnTaskRunner(std::shared_ptr<BraveRequestInfo> c
100100
if (!g_brave_browser_process->tracking_protection_service()->
101101
ShouldStartRequest(ctx->request_url, ctx->resource_type, ctx->tab_origin.host())) {
102102
ctx->new_url_spec = GetBlankDataURLForResourceType(ctx->resource_type).spec();
103+
ctx->blocked_by = kTrackerBlocked;
103104
} else if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest(
104105
ctx->request_url, ctx->resource_type, ctx->tab_origin.host()) ||
105106
!g_brave_browser_process->ad_block_regional_service()
106107
->ShouldStartRequest(ctx->request_url, ctx->resource_type,
107108
ctx->tab_origin.host())) {
108109
ctx->new_url_spec = GetBlankDataURLForResourceType(ctx->resource_type).spec();
110+
ctx->blocked_by = kAdBlocked;
109111
}
110112
}
111113

@@ -115,11 +117,15 @@ void OnBeforeURLRequestDispatchOnIOThread(
115117
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
116118
if (!ctx->new_url_spec.empty() &&
117119
ctx->new_url_spec != ctx->request_url.spec()) {
118-
// TODO: If we ever want to differentiate ads from tracking library
119-
// counts, then use brave_shields::kTrackers below.
120-
brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
121-
ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
122-
brave_shields::kAds);
120+
if (ctx->blocked_by == kAdBlocked) {
121+
brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
122+
ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
123+
brave_shields::kAds);
124+
} else if (ctx->blocked_by == kTrackerBlocked) {
125+
brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
126+
ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
127+
brave_shields::kTrackers);
128+
}
123129
}
124130

125131
next_callback.Run();

browser/net/url_context.h

+8
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ enum BraveNetworkDelegateEventType {
3737
kUnknownEventType
3838
};
3939

40+
enum BlockedBy {
41+
kNotBlocked ,
42+
kAdBlocked,
43+
kTrackerBlocked,
44+
kOtherBlocked
45+
};
46+
4047
struct BraveRequestInfo {
4148
BraveRequestInfo();
4249
~BraveRequestInfo();
@@ -57,6 +64,7 @@ struct BraveRequestInfo {
5764
GURL* allowed_unsafe_redirect_url = nullptr;
5865
BraveNetworkDelegateEventType event_type = kUnknownEventType;
5966
const base::ListValue* referral_headers_list = nullptr;
67+
BlockedBy blocked_by = kNotBlocked;
6068
// Default to invalid type for resource_type, so delegate helpers
6169
// can properly detect that the info couldn't be obtained.
6270
content::ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;

components/brave_shields/browser/ad_block_service_browsertest.cc

+13
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
#include "base/test/thread_test_helper.h"
77
#include "brave/browser/brave_browser_process_impl.h"
88
#include "brave/common/brave_paths.h"
9+
#include "brave/common/pref_names.h"
910
#include "brave/components/brave_shields/browser/ad_block_service.h"
1011
#include "brave/components/brave_shields/browser/ad_block_regional_service.h"
1112
#include "chrome/browser/ui/browser.h"
1213
#include "chrome/browser/extensions/extension_browsertest.h"
1314
#include "chrome/test/base/ui_test_utils.h"
15+
#include "components/prefs/pref_service.h"
1416
#include "content/public/test/browser_test_utils.h"
1517

1618
using extensions::ExtensionBrowserTest;
@@ -150,6 +152,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByDefaultBlocker) {
150152
kDefaultAdBlockComponentTestId,
151153
kDefaultAdBlockComponentTestBase64PublicKey);
152154
ASSERT_TRUE(InstallDefaultAdBlockExtension());
155+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
153156

154157
GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
155158
ui_test_utils::NavigateToURL(browser(), url);
@@ -163,6 +166,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByDefaultBlocker) {
163166
"addElement('ad_banner.png')",
164167
&img_loaded));
165168
EXPECT_FALSE(img_loaded);
169+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
166170
}
167171

168172
// Load a page with an image which is not an ad, and make sure it is NOT blocked.
@@ -171,6 +175,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NotAdsDoNotGetBlockedByDefaultBlocker
171175
kDefaultAdBlockComponentTestId,
172176
kDefaultAdBlockComponentTestBase64PublicKey);
173177
ASSERT_TRUE(InstallDefaultAdBlockExtension());
178+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
174179

175180
GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
176181
ui_test_utils::NavigateToURL(browser(), url);
@@ -184,6 +189,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NotAdsDoNotGetBlockedByDefaultBlocker
184189
"addElement('logo.png')",
185190
&img_loaded));
186191
EXPECT_TRUE(img_loaded);
192+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
187193
}
188194

189195
// Load a page with an ad image, and make sure it is blocked by the
@@ -193,6 +199,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByRegionalBlocker) {
193199
ASSERT_EQ(g_browser_process->GetApplicationLocale(), "fr");
194200

195201
ASSERT_TRUE(StartAdBlockRegionalService());
202+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
196203

197204
SetRegionalComponentIdAndBase64PublicKeyForTest(
198205
kRegionalAdBlockComponentTestId,
@@ -211,6 +218,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByRegionalBlocker) {
211218
"addElement('ad_fr.png')",
212219
&img_loaded));
213220
EXPECT_FALSE(img_loaded);
221+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
214222
}
215223

216224
// Load a page with an image which is not an ad, and make sure it is
@@ -220,6 +228,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NotAdsDoNotGetBlockedByRegionalBlocke
220228
ASSERT_EQ(g_browser_process->GetApplicationLocale(), "fr");
221229

222230
ASSERT_TRUE(StartAdBlockRegionalService());
231+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
223232

224233
SetRegionalComponentIdAndBase64PublicKeyForTest(
225234
kRegionalAdBlockComponentTestId,
@@ -238,6 +247,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, NotAdsDoNotGetBlockedByRegionalBlocke
238247
"addElement('logo.png')",
239248
&img_loaded));
240249
EXPECT_TRUE(img_loaded);
250+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
241251
}
242252

243253
// Upgrade from v3 to v4 format data file and make sure v4-specific ad
@@ -257,6 +267,8 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedAfterDataFileVersionUpgr
257267
SetDATFileVersionForTest("4");
258268
ASSERT_TRUE(InstallDefaultAdBlockExtension("adblock-v4", 0));
259269

270+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
271+
260272
GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
261273
ui_test_utils::NavigateToURL(browser(), url);
262274
content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
@@ -269,4 +281,5 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedAfterDataFileVersionUpgr
269281
"addElement('v4_specific_banner.png')",
270282
&img_loaded));
271283
EXPECT_FALSE(img_loaded);
284+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
272285
}

components/brave_shields/browser/tracking_protection_service_browsertest.cc

+9
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
#include "base/test/thread_test_helper.h"
88
#include "brave/browser/brave_browser_process_impl.h"
99
#include "brave/common/brave_paths.h"
10+
#include "brave/common/pref_names.h"
1011
#include "brave/components/brave_shields/browser/tracking_protection_service.h"
1112
#include "chrome/browser/ui/browser.h"
1213
#include "chrome/browser/extensions/extension_browsertest.h"
1314
#include "chrome/browser/net/url_request_mock_util.h"
1415
#include "chrome/test/base/ui_test_utils.h"
16+
#include "components/prefs/pref_service.h"
1517
#include "content/public/test/browser_test_utils.h"
1618
#include "net/dns/mock_host_resolver.h"
1719

@@ -111,6 +113,8 @@ class TrackingProtectionServiceTest : public ExtensionBrowserTest {
111113
IN_PROC_BROWSER_TEST_F(TrackingProtectionServiceTest, TrackerReferencedFromTrustedDomainNotBlocked) {
112114
ASSERT_TRUE(InstallTrackingProtectionExtension());
113115

116+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kTrackersBlocked), 0ULL);
117+
114118
GURL url = embedded_test_server()->GetURL("365media.com", kTrackingPage);
115119
ui_test_utils::NavigateToURL(browser(), url);
116120
content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
@@ -125,12 +129,15 @@ IN_PROC_BROWSER_TEST_F(TrackingProtectionServiceTest, TrackerReferencedFromTrust
125129
base::StringPrintf(kTrackingScript, test_url.spec().c_str()),
126130
&img_loaded));
127131
EXPECT_TRUE(img_loaded);
132+
133+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kTrackersBlocked), 0ULL);
128134
}
129135

130136
// Load a page that references a tracker from an untrusted domain, and
131137
// make sure it is blocked.
132138
IN_PROC_BROWSER_TEST_F(TrackingProtectionServiceTest, TrackerReferencedFromUntrustedDomainGetsBlocked) {
133139
ASSERT_TRUE(InstallTrackingProtectionExtension());
140+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kTrackersBlocked), 0ULL);
134141

135142
GURL url = embedded_test_server()->GetURL("google.com", kTrackingPage);
136143
ui_test_utils::NavigateToURL(browser(), url);
@@ -146,4 +153,6 @@ IN_PROC_BROWSER_TEST_F(TrackingProtectionServiceTest, TrackerReferencedFromUntru
146153
base::StringPrintf(kTrackingScript, test_url.spec().c_str()),
147154
&img_loaded));
148155
EXPECT_FALSE(img_loaded);
156+
157+
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kTrackersBlocked), 1ULL);
149158
}

0 commit comments

Comments
 (0)