Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pilgrim-brave committed Jun 25, 2021
1 parent 918450a commit f6dd876
Show file tree
Hide file tree
Showing 21 changed files with 173 additions and 116 deletions.
1 change: 0 additions & 1 deletion browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ source_set("browser_process") {
"//brave/browser/ethereum_remote_client/buildflags",
"//brave/components/brave_ads/browser/buildflags",
"//brave/components/brave_referrals/buildflags",
"//brave/components/debounce/browser/buildflags",
"//brave/components/greaselion/browser/buildflags",
"//brave/components/ipfs/buildflags",
"//brave/components/speedreader:buildflags",
Expand Down
5 changes: 0 additions & 5 deletions browser/brave_browser_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include "brave/components/brave_ads/browser/buildflags/buildflags.h"
#include "brave/components/brave_referrals/buildflags/buildflags.h"
#include "brave/components/debounce/browser/buildflags/buildflags.h"
#include "brave/components/greaselion/browser/buildflags/buildflags.h"
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/speedreader/buildflags.h"
Expand Down Expand Up @@ -50,9 +49,7 @@ class GreaselionDownloadService;
} // namespace greaselion

namespace debounce {
#if BUILDFLAG(ENABLE_DEBOUNCE)
class DebounceDownloadService;
#endif
} // namespace debounce

namespace ntp_background_images {
Expand Down Expand Up @@ -93,9 +90,7 @@ class BraveBrowserProcess {
virtual greaselion::GreaselionDownloadService*
greaselion_download_service() = 0;
#endif
#if BUILDFLAG(ENABLE_DEBOUNCE)
virtual debounce::DebounceDownloadService* debounce_download_service() = 0;
#endif
virtual brave_shields::HTTPSEverywhereService* https_everywhere_service() = 0;
virtual brave_component_updater::LocalDataFilesService*
local_data_files_service() = 0;
Expand Down
12 changes: 3 additions & 9 deletions browser/brave_browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "brave/components/brave_shields/browser/https_everywhere_service.h"
#include "brave/components/brave_sync/buildflags/buildflags.h"
#include "brave/components/brave_sync/network_time_helper.h"
#include "brave/components/debounce/browser/debounce_download_service.h"
#include "brave/components/ntp_background_images/browser/features.h"
#include "brave/components/ntp_background_images/browser/ntp_background_images_service.h"
#include "brave/components/p3a/brave_histogram_rewrite.h"
Expand Down Expand Up @@ -65,10 +66,6 @@
#include "brave/components/greaselion/browser/greaselion_download_service.h"
#endif

#if BUILDFLAG(ENABLE_DEBOUNCE)
#include "brave/components/debounce/browser/debounce_download_service.h"
#endif

#if BUILDFLAG(ENABLE_TOR)
#include "brave/components/tor/brave_tor_client_updater.h"
#include "brave/components/tor/pref_names.h"
Expand Down Expand Up @@ -192,9 +189,7 @@ void BraveBrowserProcessImpl::StartBraveServices() {
#if BUILDFLAG(ENABLE_GREASELION)
greaselion_download_service();
#endif
#if BUILDFLAG(ENABLE_DEBOUNCE)
debounce_download_service();
#endif
#if BUILDFLAG(ENABLE_SPEEDREADER)
speedreader_rewriter_service();
#endif
Expand Down Expand Up @@ -267,16 +262,15 @@ BraveBrowserProcessImpl::greaselion_download_service() {
}
#endif

#if BUILDFLAG(ENABLE_DEBOUNCE)
debounce::DebounceDownloadService*
BraveBrowserProcessImpl::debounce_download_service() {
if (!debounce_download_service_) {
debounce_download_service_ =
debounce::DebounceDownloadServiceFactory(local_data_files_service());
std::make_unique<debounce::DebounceDownloadService>(
local_data_files_service());
}
return debounce_download_service_.get();
}
#endif

brave_shields::HTTPSEverywhereService*
BraveBrowserProcessImpl::https_everywhere_service() {
Expand Down
7 changes: 0 additions & 7 deletions browser/brave_browser_process_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "brave/components/brave_ads/browser/buildflags/buildflags.h"
#include "brave/components/brave_component_updater/browser/brave_component.h"
#include "brave/components/brave_referrals/buildflags/buildflags.h"
#include "brave/components/debounce/browser/buildflags/buildflags.h"
#include "brave/components/greaselion/browser/buildflags/buildflags.h"
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/speedreader/buildflags.h"
Expand Down Expand Up @@ -52,9 +51,7 @@ class GreaselionDownloadService;
} // namespace greaselion

namespace debounce {
#if BUILDFLAG(ENABLE_DEBOUNCE)
class DebounceDownloadService;
#endif
} // namespace debounce

namespace ntp_background_images {
Expand Down Expand Up @@ -103,9 +100,7 @@ class BraveBrowserProcessImpl : public BraveBrowserProcess,
#if BUILDFLAG(ENABLE_GREASELION)
greaselion::GreaselionDownloadService* greaselion_download_service() override;
#endif
#if BUILDFLAG(ENABLE_DEBOUNCE)
debounce::DebounceDownloadService* debounce_download_service() override;
#endif
brave_shields::HTTPSEverywhereService* https_everywhere_service() override;
brave_component_updater::LocalDataFilesService* local_data_files_service()
override;
Expand Down Expand Up @@ -161,9 +156,7 @@ class BraveBrowserProcessImpl : public BraveBrowserProcess,
std::unique_ptr<greaselion::GreaselionDownloadService>
greaselion_download_service_;
#endif
#if BUILDFLAG(ENABLE_DEBOUNCE)
std::unique_ptr<debounce::DebounceDownloadService> debounce_download_service_;
#endif
std::unique_ptr<brave_shields::HTTPSEverywhereService>
https_everywhere_service_;
std::unique_ptr<brave_stats::BraveStatsUpdater> brave_stats_updater_;
Expand Down
2 changes: 0 additions & 2 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ BraveContentBrowserClient::CreateURLLoaderThrottles(
}
#endif // ENABLE_SPEEDREADER

#if BUILDFLAG(ENABLE_DEBOUNCE)
auto* settings_map = HostContentSettingsMapFactory::GetForProfile(
Profile::FromBrowserContext(browser_context));
if (std::unique_ptr<blink::URLLoaderThrottle> debounce_throttle =
Expand All @@ -479,7 +478,6 @@ BraveContentBrowserClient::CreateURLLoaderThrottles(
browser_context),
settings_map))
result.push_back(std::move(debounce_throttle));
#endif // ENABLE_DEBOUNCE

return result;
}
Expand Down
1 change: 0 additions & 1 deletion browser/brave_shields/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import(
"//brave/components/brave_perf_predictor/browser/buildflags/buildflags.gni")
import("//brave/components/debounce/browser/buildflags/buildflags.gni")
import("//extensions/buildflags/buildflags.gni")

brave_browser_brave_shields_sources = [
Expand Down
8 changes: 1 addition & 7 deletions browser/browser_context_keyed_service_factories.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,18 @@
#include "brave/browser/brave_ads/ads_service_factory.h"
#include "brave/browser/brave_shields/ad_block_pref_service_factory.h"
#include "brave/browser/brave_shields/cookie_pref_service_factory.h"
#include "brave/browser/debounce/debounce_service_factory.h"
#include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
#include "brave/browser/ntp_background_images/view_counter_service_factory.h"
#include "brave/browser/permissions/permission_lifetime_manager_factory.h"
#include "brave/browser/search_engines/search_engine_provider_service_factory.h"
#include "brave/browser/search_engines/search_engine_tracker.h"
#include "brave/components/brave_rewards/browser/buildflags/buildflags.h"
#include "brave/components/brave_wallet/common/buildflags/buildflags.h"
#include "brave/components/debounce/browser/buildflags/buildflags.h"
#include "brave/components/greaselion/browser/buildflags/buildflags.h"
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"

#if BUILDFLAG(ENABLE_DEBOUNCE)
#include "brave/browser/debounce/debounce_service_factory.h"
#endif

#if BUILDFLAG(ENABLE_GREASELION)
#include "brave/browser/greaselion/greaselion_service_factory.h"
#endif
Expand Down Expand Up @@ -63,9 +59,7 @@ void EnsureBrowserContextKeyedServiceFactoriesBuilt() {
#endif
brave_shields::AdBlockPrefServiceFactory::GetInstance();
brave_shields::CookiePrefServiceFactory::GetInstance();
#if BUILDFLAG(ENABLE_DEBOUNCE)
debounce::DebounceServiceFactory::GetInstance();
#endif
#if BUILDFLAG(ENABLE_GREASELION)
greaselion::GreaselionServiceFactory::GetInstance();
#endif
Expand Down
96 changes: 92 additions & 4 deletions browser/debounce/debounce_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,34 @@ class DebounceBrowserTest : public BaseLocalDataFilesBrowserTest {
GURL simple_landing_url_;
};

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest, QueryStringFilterShieldsDown) {
ASSERT_TRUE(InstallMockExtension());
IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterCrossSite) {
const std::string inputs[] = {
"", "foo=bar", "fbclid=1", "fbclid=2&key=value", "key=value&fbclid=3",
};
const std::string outputs[] = {
// URLs without trackers should be untouched.
"",
"foo=bar",
// URLs with trackers should have those removed.
"",
"key=value",
"key=value",
};

constexpr size_t input_count = base::size(inputs);
static_assert(input_count == base::size(outputs),
"Inputs and outputs must have the same number of elements.");

for (size_t i = 0; i < input_count; i++) {
NavigateToURLAndWaitForRedirects(
url(landing_url(inputs[i], simple_landing_url()), cross_site_url()),
landing_url(outputs[i], simple_landing_url()));
}
}

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterShieldsDown) {
const std::string inputs[] = {
"", "foo=bar", "fbclid=1", "fbclid=2&key=value", "key=value&fbclid=3",
};
Expand All @@ -169,9 +194,72 @@ IN_PROC_BROWSER_TEST_F(DebounceBrowserTest, QueryStringFilterShieldsDown) {
}
}

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest, QueryStringFilterDirectNavigation) {
ASSERT_TRUE(InstallMockExtension());
IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterSameSite) {
const std::string inputs[] = {
"fbclid=1",
"fbclid=2&key=value",
"key=value&fbclid=3",
};
// Same-site requests should be untouched.

constexpr size_t input_count = sizeof(inputs) / sizeof(std::string);

for (size_t i = 0; i < input_count; i++) {
NavigateToURLAndWaitForRedirects(
url(landing_url(inputs[i], simple_landing_url()), same_site_url()),
landing_url(inputs[i], simple_landing_url()));
}
}

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterCrossSiteRedirect) {
const std::string inputs[] = {
"",
"fbclid=1",
};
const std::string outputs[] = {
// URLs without trackers should be untouched.
"",
// URLs with trackers should have those removed.
"",
};

constexpr size_t input_count = sizeof(inputs) / sizeof(std::string);
static_assert(input_count == sizeof(outputs) / sizeof(std::string),
"Inputs and outputs must have the same number of elements.");

for (size_t i = 0; i < input_count; i++) {
// Same-site navigations to a cross-site redirect go through the query
// filter.
NavigateToURLAndWaitForRedirects(
url(landing_url(inputs[i], redirect_to_cross_site_landing_url()),
same_site_url()),
landing_url(outputs[i], simple_landing_url()));
}
}

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterSameSiteRedirect) {
const std::string inputs[] = {
"",
"fbclid=1",
};

constexpr size_t input_count = sizeof(inputs) / sizeof(std::string);

for (size_t i = 0; i < input_count; i++) {
// Same-site navigations to a same-site redirect are exempted from the query
// filter.
NavigateToURLAndWaitForRedirects(
url(landing_url(inputs[i], redirect_to_same_site_landing_url()),
same_site_url()),
landing_url(inputs[i], simple_landing_url()));
}
}

IN_PROC_BROWSER_TEST_F(DebounceBrowserTest,
QueryStringFilterDirectNavigation) {
const std::string inputs[] = {
"",
"abc=1",
Expand Down
3 changes: 2 additions & 1 deletion browser/debounce/debounce_service_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class DebounceServiceFactory : public BrowserContextKeyedServiceFactory {
content::BrowserContext* context) const override;
bool ServiceIsNULLWhileTesting() const override;

DISALLOW_COPY_AND_ASSIGN(DebounceServiceFactory);
DebounceServiceFactory(const DebounceServiceFactory&) = delete;
DebounceServiceFactory& operator=(const DebounceServiceFactory&) = delete;
};

} // namespace debounce
Expand Down
31 changes: 12 additions & 19 deletions browser/debounce/sources.gni
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
import("//brave/components/debounce/browser/buildflags/buildflags.gni")
brave_browser_debounce_sources = [
"//brave/browser/debounce/debounce_service_factory.cc",
"//brave/browser/debounce/debounce_service_factory.h",
]

brave_browser_debounce_sources = []
brave_browser_debounce_deps = []

if (enable_debounce) {
brave_browser_debounce_sources += [
"//brave/browser/debounce/debounce_service_factory.cc",
"//brave/browser/debounce/debounce_service_factory.h",
]

brave_browser_debounce_deps += [
"//base",
"//brave/components/debounce/browser",
"//brave/components/debounce/common",
"//chrome/common",
"//components/keyed_service/content",
"//extensions/browser",
]
}
brave_browser_debounce_deps = [
"//base",
"//brave/components/debounce/browser",
"//brave/components/debounce/common",
"//brave/extensions:common",
"//chrome/common",
"//components/keyed_service/content",
]
1 change: 0 additions & 1 deletion browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ brave_chrome_browser_deps = [
"//brave/components/cosmetic_filters/browser",
"//brave/components/cosmetic_filters/common:mojom",
"//brave/components/crypto_dot_com/browser/buildflags",
"//brave/components/debounce/browser/buildflags",
"//brave/components/decentralized_dns/buildflags",
"//brave/components/gemini/browser/buildflags",
"//brave/components/greaselion/browser/buildflags",
Expand Down
3 changes: 2 additions & 1 deletion chromium_src/chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ const char kBraveDomainBlockDescription[] =
"Enable support for blocking domains with an interstitial page";
const char kBraveDebounceName[] = "Enable debouncing";
const char kBraveDebounceDescription[] =
"Enable support for skipping certain tracking URLs.";
"Enable support for skipping tracking URLs and stripping tracking "
"parameters.";
const char kBraveExtensionNetworkBlockingName[] =
"Enable extension network blocking";
const char kBraveExtensionNetworkBlockingDescription[] =
Expand Down
4 changes: 1 addition & 3 deletions components/debounce/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ source_set("browser") {
"//brave/components/brave_component_updater/browser",
"//brave/components/brave_shields/browser",
"//brave/components/debounce/common",
"//brave/extensions:common",
"//components/content_settings/core/browser",
"//content/public/browser",
"//content/public/common",
"//extensions/browser",
"//services/network/public/cpp",
"//services/network/public/mojom",
"//third_party/blink/public/common",
"//url",
]

public_deps = [ "buildflags" ]
}
7 changes: 0 additions & 7 deletions components/debounce/browser/buildflags/BUILD.gn

This file was deleted.

5 changes: 0 additions & 5 deletions components/debounce/browser/buildflags/buildflags.gni

This file was deleted.

Loading

0 comments on commit f6dd876

Please sign in to comment.