From c96b4f94bdbb75540525063b5af236efd7bad836 Mon Sep 17 00:00:00 2001 From: AndriusA Date: Thu, 30 Apr 2020 11:05:57 +0100 Subject: [PATCH 1/7] get rid of unnecessary adblock threads waiting --- .../perf_predictor_tab_helper_browsertest.cc | 55 ++++++++----------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc b/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc index be6c9851688f..020100555504 100644 --- a/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc +++ b/components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc @@ -31,6 +31,11 @@ uint64_t getProfileBandwidthSaved(Browser* browser) { brave_perf_predictor::prefs::kBandwidthSavedBytes); } +uint64_t getProfileAdsBlocked(Browser* browser) { + return browser->profile()->GetPrefs()->GetUint64( + kAdsBlocked); +} + } // namespace class PerfPredictorTabHelperTest : public InProcessBrowserTest { @@ -49,7 +54,6 @@ class PerfPredictorTabHelperTest : public InProcessBrowserTest { void PreRunTestOnMainThread() override { InProcessBrowserTest::PreRunTestOnMainThread(); - WaitForAdBlockServiceThreads(); // Need ad_block_service to be available to get blocking savings predictions ASSERT_TRUE(g_brave_browser_process->ad_block_service()->IsInitialized()); } @@ -64,16 +68,6 @@ class PerfPredictorTabHelperTest : public InProcessBrowserTest { content::SetupCrossSiteRedirector(embedded_test_server()); ASSERT_TRUE(embedded_test_server()->Start()); } - - void WaitForAdBlockServiceThreads() { - scoped_refptr tr_helper(new base::ThreadTestHelper( - g_brave_browser_process->local_data_files_service()->GetTaskRunner())); - ASSERT_TRUE(tr_helper->Run()); - scoped_refptr io_helper(new base::ThreadTestHelper( - base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}) - .get())); - ASSERT_TRUE(io_helper->Run()); - } }; IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NoBlockNoSavings) { @@ -84,12 +78,10 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NoBlockNoSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - bool as_expected = false; - ASSERT_TRUE(ExecuteScriptAndExtractBool(contents, - "setExpectations(1, 0, 0, 0, 0, 0);" - "addImage('logo.png')", - &as_expected)); - EXPECT_TRUE(as_expected); + EXPECT_EQ(true, EvalJsWithManualReply(contents, + "addImage('logo.png');" + "setExpectations(0, 0, 0, 0, 1, 0);" + "xhr('analytics.js')")); // Prediction triggered when web contents are closed contents->Close(); EXPECT_EQ(getProfileBandwidthSaved(browser()), 0ULL); @@ -105,12 +97,12 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, ScriptBlockHasSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - ASSERT_TRUE(ExecJs(contents, "addImage('logo.png')")); - ASSERT_TRUE(EvalJsWithManualReply(contents, + EXPECT_EQ(true, EvalJsWithManualReply(contents, + "addImage('logo.png');" "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')") - .ExtractBool()); - EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); + "xhr('analytics.js')")); + + EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL); // Prediction triggered when web contents are closed contents->Close(); EXPECT_NE(getProfileBandwidthSaved(browser()), 0ULL); @@ -126,24 +118,21 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NewNavigationStoresSavings) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - ASSERT_TRUE(ExecJs(contents, "addImage('logo.png')")); - ASSERT_TRUE(EvalJsWithManualReply(contents, + EXPECT_EQ(true, EvalJsWithManualReply(contents, + "addImage('logo.png');" "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')") - .ExtractBool()); - EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL); + "xhr('analytics.js')")); + EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL); // Prediction triggered when web contents are closed GURL second_url = embedded_test_server()->GetURL("example.com", "/blocking.html"); ui_test_utils::NavigateToURL(browser(), second_url); - ASSERT_TRUE(ExecJs(contents, "addImage('logo.png')")); - ASSERT_TRUE(EvalJsWithManualReply(contents, + EXPECT_EQ(true, EvalJsWithManualReply(contents, + "addImage('logo.png');" "setExpectations(0, 0, 0, 0, 1, 0);" - "xhr('analytics.js')") - .ExtractBool()); + "xhr('analytics.js')")); - auto previous_nav_savings = browser()->profile()->GetPrefs()->GetUint64( - brave_perf_predictor::prefs::kBandwidthSavedBytes); + auto previous_nav_savings = getProfileBandwidthSaved(browser()); EXPECT_NE(previous_nav_savings, 0ULL); // closing the new navigation triggers a second computation From 5b8545df556b68de43ee090758153b8b1f27ba87 Mon Sep 17 00:00:00 2001 From: AndriusA Date: Fri, 1 May 2020 12:01:25 +0100 Subject: [PATCH 2/7] loads named third party config asynchronously --- .../brave_perf_predictor/browser/BUILD.gn | 4 + .../browser/bandwidth_savings_predictor.cc | 11 +- .../browser/bandwidth_savings_predictor.h | 4 +- .../bandwidth_savings_predictor_unittest.cc | 135 +++++++++--------- .../browser/named_third_party_registry.cc | 109 +++++++++----- .../browser/named_third_party_registry.h | 25 ++-- .../named_third_party_registry_factory.cc | 45 ++++++ .../named_third_party_registry_factory.h | 44 ++++++ .../named_third_party_registry_unittest.cc | 25 ++-- .../browser/perf_predictor_tab_helper.cc | 7 +- 10 files changed, 274 insertions(+), 135 deletions(-) create mode 100644 components/brave_perf_predictor/browser/named_third_party_registry_factory.cc create mode 100644 components/brave_perf_predictor/browser/named_third_party_registry_factory.h diff --git a/components/brave_perf_predictor/browser/BUILD.gn b/components/brave_perf_predictor/browser/BUILD.gn index 9283f2097323..696ed8acd930 100644 --- a/components/brave_perf_predictor/browser/BUILD.gn +++ b/components/brave_perf_predictor/browser/BUILD.gn @@ -22,6 +22,10 @@ source_set("browser") { "bandwidth_savings_predictor.h", "named_third_party_registry.cc", "named_third_party_registry.h", + "named_third_party_registry_factory.cc", + "named_third_party_registry_factory.h", + "p3a_bandwidth_savings_permanent_state.cc", + "p3a_bandwidth_savings_permanent_state.h", "p3a_bandwidth_savings_tracker.cc", "p3a_bandwidth_savings_tracker.h", "perf_predictor_page_metrics_observer.cc", diff --git a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.cc b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.cc index 496bef580e80..38208de63811 100644 --- a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.cc +++ b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.cc @@ -9,7 +9,6 @@ #include "base/logging.h" #include "brave/components/brave_perf_predictor/browser/bandwidth_linreg.h" -#include "brave/components/brave_perf_predictor/browser/named_third_party_registry.h" #include "components/page_load_metrics/common/page_load_metrics.mojom.h" #include "content/public/common/resource_load_info.mojom.h" #include "content/public/common/resource_type.h" @@ -17,7 +16,9 @@ namespace brave_perf_predictor { -BandwidthSavingsPredictor::BandwidthSavingsPredictor() = default; +BandwidthSavingsPredictor::BandwidthSavingsPredictor( + const NamedThirdPartyRegistry* registry) + : tp_registry_(registry) {} BandwidthSavingsPredictor::~BandwidthSavingsPredictor() = default; @@ -54,10 +55,8 @@ void BandwidthSavingsPredictor::OnSubresourceBlocked( const std::string& resource_url) { feature_map_["adblockRequests"] += 1; - const NamedThirdPartyRegistry* tp_registry = - NamedThirdPartyRegistry::GetInstance(); - if (tp_registry) { - const auto tp_name = tp_registry->GetThirdParty(resource_url); + if (tp_registry_) { + const auto tp_name = tp_registry_->GetThirdParty(resource_url); if (tp_name.has_value()) feature_map_["thirdParties." + tp_name.value() + ".blocked"] = 1; } diff --git a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h index 6ec127cc5f5f..1307a5221436 100644 --- a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h +++ b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h @@ -11,6 +11,7 @@ #include "base/containers/flat_map.h" #include "base/gtest_prod_util.h" #include "url/gurl.h" +#include "brave/components/brave_perf_predictor/browser/named_third_party_registry.h" namespace page_load_metrics { namespace mojom { @@ -39,7 +40,7 @@ namespace brave_perf_predictor { // of any resources fully loaded or blocked. class BandwidthSavingsPredictor { public: - BandwidthSavingsPredictor(); + explicit BandwidthSavingsPredictor(const NamedThirdPartyRegistry* registry); ~BandwidthSavingsPredictor(); BandwidthSavingsPredictor(const BandwidthSavingsPredictor&) = delete; @@ -62,6 +63,7 @@ class BandwidthSavingsPredictor { FeaturiseResourceLoading); GURL main_frame_url_; + const NamedThirdPartyRegistry* tp_registry_; // not owned base::flat_map feature_map_; }; diff --git a/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc b/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc index ec61d9e13661..1d9440df2662 100644 --- a/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc +++ b/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc @@ -6,6 +6,7 @@ #include "brave/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h" #include "base/containers/flat_map.h" +#include "base/run_loop.h" #include "base/time/time.h" #include "chrome/browser/predictors/loading_test_util.h" #include "components/page_load_metrics/common/page_load_metrics.mojom.h" @@ -16,140 +17,144 @@ namespace brave_perf_predictor { -TEST(BandwidthSavingsPredictorTest, FeaturiseBlocked) { - BandwidthSavingsPredictor predictor; - predictor.OnSubresourceBlocked("https://google-analytics.com"); - EXPECT_EQ(predictor.feature_map_["adblockRequests"], 1); - EXPECT_EQ(predictor.feature_map_["thirdParties.Google Analytics.blocked"], +class BandwidthSavingsPredictorTest : public ::testing::Test { + public: + BandwidthSavingsPredictorTest() { + tp_registry_ = std::make_unique(); + tp_registry_->InitializeDefault(); + predictor_ = + std::make_unique(tp_registry_.get()); + // Allow the third party registry to initialize + base::RunLoop().RunUntilIdle(); + } + + protected: + std::unique_ptr tp_registry_; + std::unique_ptr predictor_; +}; + +TEST_F(BandwidthSavingsPredictorTest, FeaturiseBlocked) { + predictor_->OnSubresourceBlocked("https://google-analytics.com"); + EXPECT_EQ(predictor_->feature_map_["adblockRequests"], 1); + EXPECT_EQ(predictor_->feature_map_["thirdParties.Google Analytics.blocked"], 1); - predictor.OnSubresourceBlocked("https://test.m.facebook.com"); - EXPECT_EQ(predictor.feature_map_["adblockRequests"], 2); + predictor_->OnSubresourceBlocked("https://test.m.facebook.com"); + EXPECT_EQ(predictor_->feature_map_["adblockRequests"], 2); } -TEST(BandwidthSavingsPredictorTest, FeaturiseTiming) { - BandwidthSavingsPredictor predictor; +TEST_F(BandwidthSavingsPredictorTest, FeaturiseTiming) { const auto empty_timing = page_load_metrics::CreatePageLoadTiming(); - predictor.OnPageLoadTimingUpdated(*empty_timing); - EXPECT_EQ(predictor.feature_map_["metrics.firstMeaningfulPaint"], 0); - EXPECT_EQ(predictor.feature_map_["metrics.observedDomContentLoaded"], 0); - EXPECT_EQ(predictor.feature_map_["metrics.observedFirstVisualChange"], 0); - EXPECT_EQ(predictor.feature_map_["metrics.observedLoad"], 0); - EXPECT_EQ(predictor.feature_map_["metrics.interactive"], 0); + predictor_->OnPageLoadTimingUpdated(*empty_timing); + EXPECT_EQ(predictor_->feature_map_["metrics.firstMeaningfulPaint"], 0); + EXPECT_EQ(predictor_->feature_map_["metrics.observedDomContentLoaded"], 0); + EXPECT_EQ(predictor_->feature_map_["metrics.observedFirstVisualChange"], 0); + EXPECT_EQ(predictor_->feature_map_["metrics.observedLoad"], 0); + EXPECT_EQ(predictor_->feature_map_["metrics.interactive"], 0); auto timing = page_load_metrics::CreatePageLoadTiming(); timing->document_timing->dom_content_loaded_event_start = base::TimeDelta::FromMilliseconds(1000); - predictor.OnPageLoadTimingUpdated(*timing); - EXPECT_EQ(predictor.feature_map_["metrics.observedDomContentLoaded"], 1000); + predictor_->OnPageLoadTimingUpdated(*timing); + EXPECT_EQ(predictor_->feature_map_["metrics.observedDomContentLoaded"], 1000); timing->document_timing->load_event_start = base::TimeDelta::FromMilliseconds(2000); - predictor.OnPageLoadTimingUpdated(*timing); - EXPECT_EQ(predictor.feature_map_["metrics.observedLoad"], 2000); + predictor_->OnPageLoadTimingUpdated(*timing); + EXPECT_EQ(predictor_->feature_map_["metrics.observedLoad"], 2000); timing->paint_timing->first_meaningful_paint = base::TimeDelta::FromMilliseconds(1500); - predictor.OnPageLoadTimingUpdated(*timing); - EXPECT_EQ(predictor.feature_map_["metrics.firstMeaningfulPaint"], 1500); + predictor_->OnPageLoadTimingUpdated(*timing); + EXPECT_EQ(predictor_->feature_map_["metrics.firstMeaningfulPaint"], 1500); timing->paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(800); - predictor.OnPageLoadTimingUpdated(*timing); - EXPECT_EQ(predictor.feature_map_["metrics.observedFirstVisualChange"], 800); + predictor_->OnPageLoadTimingUpdated(*timing); + EXPECT_EQ(predictor_->feature_map_["metrics.observedFirstVisualChange"], 800); timing->interactive_timing->interactive = base::TimeDelta::FromMilliseconds(2500); - predictor.OnPageLoadTimingUpdated(*timing); - EXPECT_EQ(predictor.feature_map_["metrics.interactive"], 2500); + predictor_->OnPageLoadTimingUpdated(*timing); + EXPECT_EQ(predictor_->feature_map_["metrics.interactive"], 2500); } -TEST(BandwidthSavingsPredictorTest, FeaturiseResourceLoading) { - BandwidthSavingsPredictor predictor; - EXPECT_EQ(predictor.feature_map_["resources.third-party.requestCount"], 0); +TEST_F(BandwidthSavingsPredictorTest, FeaturiseResourceLoading) { + EXPECT_EQ(predictor_->feature_map_["resources.third-party.requestCount"], 0); const GURL main_frame("https://brave.com/"); auto fp_style = predictors::CreateResourceLoadInfo( "https://brave.com/style.css", content::ResourceType::kStylesheet); fp_style->raw_body_bytes = 1000; - predictor.OnResourceLoadComplete(main_frame, *fp_style); - EXPECT_EQ(predictor.feature_map_["resources.third-party.requestCount"], 0); - EXPECT_EQ(predictor.feature_map_["resources.stylesheet.requestCount"], 1); - EXPECT_EQ(predictor.feature_map_["resources.stylesheet.size"], 1000); + predictor_->OnResourceLoadComplete(main_frame, *fp_style); + EXPECT_EQ(predictor_->feature_map_["resources.third-party.requestCount"], 0); + EXPECT_EQ(predictor_->feature_map_["resources.stylesheet.requestCount"], 1); + EXPECT_EQ(predictor_->feature_map_["resources.stylesheet.size"], 1000); auto tp_style = predictors::CreateResourceLoadInfo( "https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.js", content::ResourceType::kScript); tp_style->raw_body_bytes = 1001; - predictor.OnResourceLoadComplete(main_frame, *tp_style); + predictor_->OnResourceLoadComplete(main_frame, *tp_style); - EXPECT_EQ(predictor.feature_map_["resources.third-party.requestCount"], 1); - EXPECT_EQ(predictor.feature_map_["resources.stylesheet.requestCount"], 1); - EXPECT_EQ(predictor.feature_map_["resources.script.requestCount"], 1); - EXPECT_EQ(predictor.feature_map_["resources.stylesheet.size"], 1000); - EXPECT_EQ(predictor.feature_map_["resources.script.size"], 1001); + EXPECT_EQ(predictor_->feature_map_["resources.third-party.requestCount"], 1); + EXPECT_EQ(predictor_->feature_map_["resources.stylesheet.requestCount"], 1); + EXPECT_EQ(predictor_->feature_map_["resources.script.requestCount"], 1); + EXPECT_EQ(predictor_->feature_map_["resources.stylesheet.size"], 1000); + EXPECT_EQ(predictor_->feature_map_["resources.script.size"], 1001); - EXPECT_EQ(predictor.feature_map_["resources.total.requestCount"], 2); - EXPECT_EQ(predictor.feature_map_["resources.total.size"], 2001); + EXPECT_EQ(predictor_->feature_map_["resources.total.requestCount"], 2); + EXPECT_EQ(predictor_->feature_map_["resources.total.size"], 2001); } -TEST(BandwidthSavingsPredictorTest, PredictZeroNoData) { - BandwidthSavingsPredictor predictor; - EXPECT_EQ(predictor.PredictSavingsBytes(), 0); +TEST_F(BandwidthSavingsPredictorTest, PredictZeroNoData) { + EXPECT_EQ(predictor_->PredictSavingsBytes(), 0); } -TEST(BandwidthSavingsPredictorTest, PredictZeroInternalUrl) { - BandwidthSavingsPredictor predictor; - +TEST_F(BandwidthSavingsPredictorTest, PredictZeroInternalUrl) { const GURL main_frame("brave://version"); auto res = predictors::CreateResourceLoadInfo("brave://version"); - predictor.OnResourceLoadComplete(main_frame, *res); + predictor_->OnResourceLoadComplete(main_frame, *res); - EXPECT_EQ(predictor.PredictSavingsBytes(), 0); + EXPECT_EQ(predictor_->PredictSavingsBytes(), 0); } -TEST(BandwidthSavingsPredictorTest, PredictZeroBadFrame) { - BandwidthSavingsPredictor predictor; - +TEST_F(BandwidthSavingsPredictorTest, PredictZeroBadFrame) { const GURL main_frame(""); auto res = predictors::CreateResourceLoadInfo( "https://brave.com/style.css", content::ResourceType::kStylesheet); res->raw_body_bytes = 1000; - predictor.OnResourceLoadComplete(main_frame, *res); + predictor_->OnResourceLoadComplete(main_frame, *res); - EXPECT_EQ(predictor.PredictSavingsBytes(), 0); + EXPECT_EQ(predictor_->PredictSavingsBytes(), 0); } -TEST(BandwidthSavingsPredictorTest, PredictZeroNoBlocks) { - BandwidthSavingsPredictor predictor; - +TEST_F(BandwidthSavingsPredictorTest, PredictZeroNoBlocks) { const GURL main_frame("https://brave.com"); auto res = predictors::CreateResourceLoadInfo( "https://brave.com/style.css", content::ResourceType::kStylesheet); res->raw_body_bytes = 1000; - predictor.OnResourceLoadComplete(main_frame, *res); + predictor_->OnResourceLoadComplete(main_frame, *res); - EXPECT_EQ(predictor.PredictSavingsBytes(), 0); + EXPECT_EQ(predictor_->PredictSavingsBytes(), 0); } -TEST(BandwidthSavingsPredictorTest, PredictNonZero) { - BandwidthSavingsPredictor predictor; - +TEST_F(BandwidthSavingsPredictorTest, PredictNonZero) { const GURL main_frame("https://brave.com"); auto res = predictors::CreateResourceLoadInfo( "https://brave.com/style.css", content::ResourceType::kStylesheet); res->raw_body_bytes = 200000; res->total_received_bytes = 200000; - predictor.OnResourceLoadComplete(main_frame, *res); + predictor_->OnResourceLoadComplete(main_frame, *res); - predictor.OnSubresourceBlocked("https://google-analytics.com/ga.js"); + predictor_->OnSubresourceBlocked("https://google-analytics.com/ga.js"); // resource still seen as complete, but with 0 bytes auto blocked = predictors::CreateResourceLoadInfo( "https://google-analytics.com/ga.js", content::ResourceType::kScript); blocked->raw_body_bytes = 0; - predictor.OnResourceLoadComplete(main_frame, *blocked); + predictor_->OnResourceLoadComplete(main_frame, *blocked); - EXPECT_NE(predictor.PredictSavingsBytes(), 0); + EXPECT_NE(predictor_->PredictSavingsBytes(), 0); } } // namespace brave_perf_predictor diff --git a/components/brave_perf_predictor/browser/named_third_party_registry.cc b/components/brave_perf_predictor/browser/named_third_party_registry.cc index c9c10b38db16..5c30b370cb7a 100644 --- a/components/brave_perf_predictor/browser/named_third_party_registry.cc +++ b/components/brave_perf_predictor/browser/named_third_party_registry.cc @@ -5,12 +5,19 @@ #include "brave/components/brave_perf_predictor/browser/named_third_party_registry.h" +#include + +#include "base/bind.h" #include "base/containers/flat_set.h" #include "base/json/json_reader.h" #include "base/logging.h" #include "base/metrics/histogram_macros.h" #include "base/strings/string_piece.h" #include "base/strings/string_util.h" +#include "base/task/post_task.h" +#include "base/task/thread_pool.h" +#include "base/task_runner_util.h" +#include "base/threading/sequenced_task_runner_handle.h" #include "base/values.h" #include "brave/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h" #include "components/grit/brave_components_resources.h" @@ -20,32 +27,19 @@ namespace brave_perf_predictor { -// static -NamedThirdPartyRegistry* NamedThirdPartyRegistry::GetInstance() { - auto* extractor = base::Singleton::get(); - // By default initialize from packaged resources - if (!extractor->IsInitialized()) { - bool initialized = extractor->InitializeFromResource(); - if (!initialized) { - VLOG(2) << "Initialization from resource failed, marking as initialized " - << "will not retry"; - extractor->MarkInitialized(true); - } - } - return extractor; -} +namespace { -bool NamedThirdPartyRegistry::LoadMappings(const base::StringPiece entities, - bool discard_irrelevant) { - // Reset previous mappings - entity_by_domain_.clear(); - entity_by_root_domain_.clear(); +std::tuple, + base::flat_map> +ParseMappings(const base::StringPiece entities, bool discard_irrelevant) { + base::flat_map entity_by_domain; + base::flat_map entity_by_root_domain; // Parse the JSON base::Optional document = base::JSONReader::Read(entities); if (!document || !document->is_list()) { LOG(ERROR) << "Cannot parse the third-party entities list"; - return false; + return {}; } // Collect the mappings @@ -68,7 +62,7 @@ bool NamedThirdPartyRegistry::LoadMappings(const base::StringPiece entities, const base::StringPiece entity_domain(entity_domain_it.GetString()); const auto inserted = - entity_by_domain_.emplace(entity_domain, *entity_name); + entity_by_domain.emplace(entity_domain, *entity_name); if (!inserted.second) { VLOG(2) << "Malformed data: duplicate domain " << entity_domain; } @@ -76,28 +70,69 @@ bool NamedThirdPartyRegistry::LoadMappings(const base::StringPiece entities, entity_domain, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); - auto root_entity_entry = entity_by_root_domain_.find(root_domain); - if (root_entity_entry != entity_by_root_domain_.end() && + auto root_entity_entry = entity_by_root_domain.find(root_domain); + if (root_entity_entry != entity_by_root_domain.end() && root_entity_entry->second != *entity_name) { // If there is a clash at root domain level, neither is correct - entity_by_root_domain_.erase(root_entity_entry); + entity_by_root_domain.erase(root_entity_entry); } else { - entity_by_root_domain_.emplace(root_domain, *entity_name); + entity_by_root_domain.emplace(root_domain, *entity_name); } } } - entity_by_domain_.shrink_to_fit(); - entity_by_root_domain_.shrink_to_fit(); + entity_by_domain.shrink_to_fit(); + entity_by_root_domain.shrink_to_fit(); + return std::make_tuple(entity_by_domain, entity_by_root_domain); +} + +std::tuple, + base::flat_map> +ParseFromResource(int resource_id) { + // TODO(AndriusA): insert trace event here + SCOPED_UMA_HISTOGRAM_TIMER( + "Brave.Savings.NamedThirdPartyRegistry.LoadTimeMS"); + auto& resource_bundle = ui::ResourceBundle::GetSharedInstance(); + std::string data_resource = + resource_bundle.LoadDataResourceString(resource_id); + // Parse resource, discarding irrelevant entities + return ParseMappings(data_resource, true); +} + +} // namespace + +bool NamedThirdPartyRegistry::LoadMappings(const base::StringPiece entities, + bool discard_irrelevant) { + // Reset previous mappings + entity_by_domain_.clear(); + entity_by_root_domain_.clear(); + initialized_ = false; + + tie(entity_by_domain_, entity_by_root_domain_) = + ParseMappings(entities, discard_irrelevant); + if (entity_by_domain_.size() == 0 || entity_by_root_domain_.size() == 0) + return false; + + initialized_ = true; + return true; +} + +void NamedThirdPartyRegistry::UpdateMappings( + std::tuple, + base::flat_map> entity_mappings) { + tie(entity_by_domain_, entity_by_root_domain_) = entity_mappings; VLOG(2) << "Loaded " << entity_by_domain_.size() << " mappings by domain and " << entity_by_root_domain_.size() << " by root domain; size"; initialized_ = true; - - return true; } base::Optional NamedThirdPartyRegistry::GetThirdParty( const base::StringPiece request_url) const { + if (!IsInitialized()) { + VLOG(2) << "Named Third Party Registry not initialized"; + return base::nullopt; + } + const GURL url(request_url); if (!url.is_valid()) return base::nullopt; @@ -122,16 +157,12 @@ NamedThirdPartyRegistry::NamedThirdPartyRegistry() = default; NamedThirdPartyRegistry::~NamedThirdPartyRegistry() = default; -bool NamedThirdPartyRegistry::InitializeFromResource() { - const auto resource_id = IDR_THIRD_PARTY_ENTITIES; - // TODO(AndriusA): insert trace event here - SCOPED_UMA_HISTOGRAM_TIMER( - "Brave.Savings.NamedThirdPartyRegistry.LoadTimeMS"); - auto& resource_bundle = ui::ResourceBundle::GetSharedInstance(); - std::string data_resource = - resource_bundle.LoadDataResourceString(resource_id); - // Parse resource, discarding irrelevant entities - return LoadMappings(data_resource, true); +void NamedThirdPartyRegistry::InitializeDefault() { + base::PostTaskAndReplyWithResult( + FROM_HERE, {base::ThreadPool(), base::MayBlock()}, + base::BindOnce(&ParseFromResource, IDR_THIRD_PARTY_ENTITIES), + base::BindOnce(&NamedThirdPartyRegistry::UpdateMappings, + weak_factory_.GetWeakPtr())); } } // namespace brave_perf_predictor diff --git a/components/brave_perf_predictor/browser/named_third_party_registry.h b/components/brave_perf_predictor/browser/named_third_party_registry.h index ffda465dcfb3..17ad67d5c496 100644 --- a/components/brave_perf_predictor/browser/named_third_party_registry.h +++ b/components/brave_perf_predictor/browser/named_third_party_registry.h @@ -9,41 +9,44 @@ #include #include "base/containers/flat_map.h" -#include "base/memory/singleton.h" +#include "base/memory/weak_ptr.h" #include "base/values.h" +#include "components/keyed_service/core/keyed_service.h" namespace brave_perf_predictor { // Retrieves publicly known Third Party (organisation) for a given URL, using // data from the Third Party Web repository // (https://github.com/patrickhulce/third-party-web). -class NamedThirdPartyRegistry { +class NamedThirdPartyRegistry : public KeyedService { public: + NamedThirdPartyRegistry(); + ~NamedThirdPartyRegistry() override; + NamedThirdPartyRegistry(const NamedThirdPartyRegistry&) = delete; NamedThirdPartyRegistry& operator=(const NamedThirdPartyRegistry&) = delete; - static NamedThirdPartyRegistry* GetInstance(); // Parse the provided mappings (in JSON format), potentially discarding // entities not relevant to the bandwith prediction model (i.e. those not // seen in training the model). - bool LoadMappings(const base::StringPiece entities, - bool discard_irrelevant); + bool LoadMappings(const base::StringPiece entities, bool discard_irrelevant); + // Default initialization - asynchronously load from bundled resource + void InitializeDefault(); base::Optional GetThirdParty( const base::StringPiece domain) const; private: - friend struct base::DefaultSingletonTraits; - - NamedThirdPartyRegistry(); - ~NamedThirdPartyRegistry(); - bool IsInitialized() const { return initialized_; } void MarkInitialized(bool initialized) { initialized_ = initialized; } - bool InitializeFromResource(); + void UpdateMappings( + std::tuple, + base::flat_map> entity_mappings); bool initialized_ = false; base::flat_map entity_by_domain_; base::flat_map entity_by_root_domain_; + + base::WeakPtrFactory weak_factory_{this}; }; } // namespace brave_perf_predictor diff --git a/components/brave_perf_predictor/browser/named_third_party_registry_factory.cc b/components/brave_perf_predictor/browser/named_third_party_registry_factory.cc new file mode 100644 index 000000000000..8decd5a8c17a --- /dev/null +++ b/components/brave_perf_predictor/browser/named_third_party_registry_factory.cc @@ -0,0 +1,45 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_perf_predictor/browser/named_third_party_registry_factory.h" + +#include "brave/components/brave_perf_predictor/browser/named_third_party_registry.h" +#include "chrome/browser/profiles/profile.h" +#include "components/keyed_service/content/browser_context_dependency_manager.h" + +namespace brave_perf_predictor { + +// static +NamedThirdPartyRegistryFactory* NamedThirdPartyRegistryFactory::GetInstance() { + return base::Singleton::get(); +} + +NamedThirdPartyRegistry* NamedThirdPartyRegistryFactory::GetForBrowserContext( + content::BrowserContext* context) { + return static_cast( + NamedThirdPartyRegistryFactory::GetInstance() + ->GetServiceForBrowserContext(context, true /*create*/)); +} + +NamedThirdPartyRegistryFactory::NamedThirdPartyRegistryFactory() + : BrowserContextKeyedServiceFactory( + "NamedThirdPartyRegistry", + BrowserContextDependencyManager::GetInstance()) {} + +NamedThirdPartyRegistryFactory::~NamedThirdPartyRegistryFactory() {} + +KeyedService* NamedThirdPartyRegistryFactory::BuildServiceInstanceFor( + content::BrowserContext* context) const { + auto* registry = new NamedThirdPartyRegistry(); + registry->InitializeDefault(); + return registry; +} + +bool NamedThirdPartyRegistryFactory::ServiceIsCreatedWithBrowserContext() + const { + return true; +} + +} // namespace brave_perf_predictor diff --git a/components/brave_perf_predictor/browser/named_third_party_registry_factory.h b/components/brave_perf_predictor/browser/named_third_party_registry_factory.h new file mode 100644 index 000000000000..507cedfe9d1b --- /dev/null +++ b/components/brave_perf_predictor/browser/named_third_party_registry_factory.h @@ -0,0 +1,44 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_PERF_PREDICTOR_BROWSER_NAMED_THIRD_PARTY_REGISTRY_FACTORY_H_ +#define BRAVE_COMPONENTS_BRAVE_PERF_PREDICTOR_BROWSER_NAMED_THIRD_PARTY_REGISTRY_FACTORY_H_ + +#include "base/memory/singleton.h" +#include "components/keyed_service/content/browser_context_keyed_service_factory.h" +#include "components/keyed_service/core/keyed_service.h" + +class Profile; + +namespace brave_perf_predictor { + +class NamedThirdPartyRegistry; + +class NamedThirdPartyRegistryFactory + : public BrowserContextKeyedServiceFactory { + public: + static NamedThirdPartyRegistryFactory* GetInstance(); + static NamedThirdPartyRegistry* GetForBrowserContext( + content::BrowserContext* context); + + private: + friend struct base::DefaultSingletonTraits; + NamedThirdPartyRegistryFactory(); + ~NamedThirdPartyRegistryFactory() override; + + NamedThirdPartyRegistryFactory(const NamedThirdPartyRegistryFactory&) = + delete; + NamedThirdPartyRegistryFactory& operator=( + const NamedThirdPartyRegistryFactory&) = delete; + + // BrowserContextKeyedServiceFactory overrides: + KeyedService* BuildServiceInstanceFor( + content::BrowserContext* context) const override; + bool ServiceIsCreatedWithBrowserContext() const override; +}; + +} // namespace brave_perf_predictor + +#endif // BRAVE_COMPONENTS_BRAVE_PERF_PREDICTOR_BROWSER_NAMED_THIRD_PARTY_REGISTRY_FACTORY_H_ diff --git a/components/brave_perf_predictor/browser/named_third_party_registry_unittest.cc b/components/brave_perf_predictor/browser/named_third_party_registry_unittest.cc index 29b581b32db6..f19362b73215 100644 --- a/components/brave_perf_predictor/browser/named_third_party_registry_unittest.cc +++ b/components/brave_perf_predictor/browser/named_third_party_registry_unittest.cc @@ -41,7 +41,8 @@ namespace { std::string LoadFile() { base::FilePath source_root; base::PathService::Get(base::DIR_SOURCE_ROOT, &source_root); - auto path = source_root.Append(FILE_PATH_LITERAL("brave")) + auto path = + source_root.Append(FILE_PATH_LITERAL("brave")) .Append(FILE_PATH_LITERAL("components")) .Append(FILE_PATH_LITERAL("brave_perf_predictor")) .Append(FILE_PATH_LITERAL("resources")) @@ -49,39 +50,41 @@ std::string LoadFile() { std::string value; const bool ok = ReadFileToString(path, &value); - if (!ok) return {}; + if (!ok) + return {}; return value; } } // namespace TEST(NamedThirdPartyRegistryTest, HandlesEmptyJSON) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); bool parsed = extractor->LoadMappings("", false); EXPECT_FALSE(parsed); } TEST(NamedThirdPartyRegistryTest, ParsesJSON) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); bool parsed = extractor->LoadMappings(test_mapping, false); EXPECT_TRUE(parsed); } TEST(NamedThirdPartyRegistryTest, HandlesInvalidJSON) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); - bool parsed = extractor->LoadMappings(R"([{"name":"Google Analytics")", false); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); + bool parsed = + extractor->LoadMappings(R"([{"name":"Google Analytics")", false); EXPECT_FALSE(parsed); } TEST(NamedThirdPartyRegistryTest, HandlesFullDataset) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); auto dataset = LoadFile(); bool parsed = extractor->LoadMappings(dataset, true); EXPECT_TRUE(parsed); } TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyURLTest) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); auto dataset = LoadFile(); extractor->LoadMappings(dataset, true); @@ -91,7 +94,7 @@ TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyURLTest) { } TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyHostnameTest) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); auto dataset = LoadFile(); extractor->LoadMappings(dataset, true); auto entity = extractor->GetThirdParty("https://google-analytics.com"); @@ -100,7 +103,7 @@ TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyHostnameTest) { } TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyRootDomainTest) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); auto dataset = LoadFile(); extractor->LoadMappings(dataset, true); auto entity = extractor->GetThirdParty("https://test.m.facebook.com"); @@ -109,7 +112,7 @@ TEST(NamedThirdPartyRegistryTest, ExtractsThirdPartyRootDomainTest) { } TEST(NamedThirdPartyRegistryTest, HandlesUnrecognisedThirdPartyTest) { - NamedThirdPartyRegistry* extractor = NamedThirdPartyRegistry::GetInstance(); + NamedThirdPartyRegistry* extractor = new NamedThirdPartyRegistry(); auto dataset = LoadFile(); extractor->LoadMappings(dataset, true); auto entity = extractor->GetThirdParty("http://example.com"); diff --git a/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc b/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc index 66df599bae68..7fa39bda70b0 100644 --- a/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc +++ b/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_perf_predictor/browser/perf_predictor_tab_helper.h" +#include "brave/components/brave_perf_predictor/browser/named_third_party_registry_factory.h" #include "brave/components/brave_perf_predictor/common/pref_names.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" @@ -38,10 +39,12 @@ content::WebContents* GetWebContents(int render_process_id, PerfPredictorTabHelper::PerfPredictorTabHelper( content::WebContents* web_contents) - : WebContentsObserver(web_contents), - bandwidth_predictor_(std::make_unique()) { + : WebContentsObserver(web_contents) { if (web_contents->GetBrowserContext()->IsOffTheRecord()) return; + bandwidth_predictor_ = std::make_unique( + NamedThirdPartyRegistryFactory::GetForBrowserContext( + web_contents->GetBrowserContext())); bandwidth_tracker_ = std::make_unique( user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())); From ba063e1e5a193ecfa920c77a7e608c536a0f6f84 Mon Sep 17 00:00:00 2001 From: AndriusA Date: Fri, 1 May 2020 12:18:48 +0100 Subject: [PATCH 3/7] swaps NTP HTTPS upgrades stat for bandwidth saved --- .../ui/webui/brave_new_tab_message_handler.cc | 3 -- components/brave_new_tab_ui/api/stats.ts | 4 +- .../containers/newTab/stats.tsx | 46 ++++++++++++++++--- .../storage/new_tab_storage.ts | 2 +- components/definitions/newTab.d.ts | 2 +- components/test/testData.ts | 2 +- 6 files changed, 44 insertions(+), 15 deletions(-) diff --git a/browser/ui/webui/brave_new_tab_message_handler.cc b/browser/ui/webui/brave_new_tab_message_handler.cc index 7bba8a43e5e4..f8f573fab294 100644 --- a/browser/ui/webui/brave_new_tab_message_handler.cc +++ b/browser/ui/webui/brave_new_tab_message_handler.cc @@ -52,9 +52,6 @@ base::DictionaryValue GetStatsDictionary(PrefService* prefs) { stats_data.SetInteger( "javascriptBlockedStat", prefs->GetUint64(kJavascriptBlocked)); - stats_data.SetInteger( - "httpsUpgradesStat", - prefs->GetUint64(kHttpsUpgrades)); stats_data.SetInteger( "fingerprintingBlockedStat", prefs->GetUint64(kFingerprintingBlocked)); diff --git a/components/brave_new_tab_ui/api/stats.ts b/components/brave_new_tab_ui/api/stats.ts index 18620c2014dc..d5d7fe1428d5 100644 --- a/components/brave_new_tab_ui/api/stats.ts +++ b/components/brave_new_tab_ui/api/stats.ts @@ -13,8 +13,8 @@ export type Stats = { adsBlockedStat: number, javascriptBlockedStat: number, - httpsUpgradesStat: number, - fingerprintingBlockedStat: number + fingerprintingBlockedStat: number, + bandwidthSavedStat: number } type StatsUpdatedHandler = (statsData: Stats) => void diff --git a/components/brave_new_tab_ui/containers/newTab/stats.tsx b/components/brave_new_tab_ui/containers/newTab/stats.tsx index c58bbbc40182..bc5acf05119e 100644 --- a/components/brave_new_tab_ui/containers/newTab/stats.tsx +++ b/components/brave_new_tab_ui/containers/newTab/stats.tsx @@ -21,8 +21,37 @@ class Stats extends React.Component { return this.props.stats.adsBlockedStat || 0 } - get httpsUpgradedCount () { - return this.props.stats.httpsUpgradesStat || 0 + get estimatedBandwidthSaved () { + const estimatedBWSaved = this.props.stats.bandwidthSavedStat + if (estimatedBWSaved) { + const bytes = estimatedBWSaved < 1024 + const kilobytes = estimatedBWSaved < 1024 * 1024 + const megabytes = estimatedBWSaved < 1024 * 1024 * 1024 + + let counter + let id + if (bytes) { + counter = estimatedBWSaved + id = 'B' + } else if (kilobytes) { + counter = (estimatedBWSaved / 1024).toFixed(0) + id = 'KB' + } else if (megabytes) { + counter = (estimatedBWSaved / 1024 / 1024).toFixed(1) + id = 'MB' + } else { + counter = (estimatedBWSaved / 1024 / 1024 / 1024).toFixed(2) + id = 'GB' + } + + return { + id, + value: counter, + args: JSON.stringify({ value: counter }) + } + } else { + return false + } } get estimatedTimeSaved () { @@ -59,8 +88,8 @@ class Stats extends React.Component { render () { const trackedBlockersCount = this.adblockCount.toLocaleString() - const httpsUpgradedCount = this.httpsUpgradedCount.toLocaleString() const timeSaved = this.estimatedTimeSaved + const bandwidthSaved = this.estimatedBandwidthSaved return ( @@ -68,10 +97,13 @@ class Stats extends React.Component { description={getLocale('adsTrackersBlocked')} counter={trackedBlockersCount} /> - + {bandwidthSaved && + + } Date: Fri, 1 May 2020 12:34:28 +0100 Subject: [PATCH 4/7] fix sentence casing per https://github.com/brave/brave-browser/issues/8969 --- components/resources/brave_components_strings.grd | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/resources/brave_components_strings.grd b/components/resources/brave_components_strings.grd index 37b2359d3ebd..7834577d0e9a 100644 --- a/components/resources/brave_components_strings.grd +++ b/components/resources/brave_components_strings.grd @@ -148,10 +148,10 @@ - Ads and Trackers Blocked + Ads and trackers blocked HTTPS Upgrades - Estimated Time Saved - Estimated Bandwidth Saved + Estimated time saved + Estimated bandwidth saved Top site removed. Undo Close From 277cc15c12230d5387363db4ad71c75d11d7a656 Mon Sep 17 00:00:00 2001 From: AndriusA Date: Fri, 1 May 2020 14:49:32 +0100 Subject: [PATCH 5/7] removes the use of `interactive` metric from the model ahead of C83 changes Resolves https://github.com/brave/brave-browser/issues/9176 --- .../browser/bandwidth_linreg_parameters.h | 222 +++++++++--------- .../browser/bandwidth_linreg_unittest.cc | 3 +- .../brave_perf_predictor/python/config.py | 6 +- .../brave_perf_predictor/python/model.py | 2 +- 4 files changed, 112 insertions(+), 121 deletions(-) diff --git a/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h b/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h index a626b02fc16c..5e348a5525d7 100644 --- a/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h +++ b/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h @@ -16,166 +16,165 @@ namespace brave_perf_predictor { -constexpr double model_intercept = 5.085562016132402; -constexpr int feature_count = 214; +constexpr double model_intercept = 5.085407773814489; +constexpr int feature_count = 213; constexpr std::array model_coefficients = { -0.12352367722740858, --0.0, +0.12337659866050482, -0.0, 0.0, 0.0, 0.0, -0.006892384968665546, --0.0053006306015809974, -0.012808044375561328, --0.022568426204584803, +0.006894977221883717, +-0.0053057834130837605, +0.012862385567785137, +-0.022571575512130217, 0.0, -0.031262883891716595, --0.008895304473470696, +0.03128111390815784, +-0.008905984853438976, 0.0, --0.012542033549705674, --0.018165453504801546, --0.008479435850304733, +-0.012493676378740588, +-0.01817560552389195, +-0.008378072321895977, 0.0, 0.0, -0.002311112946961536, -0.04901700232714016, +0.002283231356917989, +0.048919735990114545, 0.0, 0.0, -0.0, -0.03701018507587659, -0.014184085455013776, +0.03710665420979194, +0.014291396463246602, -0.0, -0.10265583719234812, -0.05237925617436799, +0.102892550977104, +0.052414725498173366, 0.0, -0.2561434657008117, -0.3954099722860754, -0.4241724431436048, -0.2554360194633748, +0.25621205523724505, +0.39537140102916657, +0.42410309202890667, +0.25546300017617013, -0.0, -0.1110185928151764, +0.11108001260714759, 0.0, 0.0, -0.0, -0.369424591785374, --0.0621249125494256, +0.369347502104488, +-0.0621473488525576, 0.0, --0.19227234809590055, -0.009790817504584938, +-0.19225865475037224, +0.009684176926811635, -0.0, -0.5981006783511988, +0.5984031539555243, 0.0, -0.0, 0.0, 0.0, -0.0, --0.154225297347611, -0.5815994580458169, -0.07673442247118749, +-0.15442052414735336, +0.5819796534516541, +0.07669089364860023, -0.0, -0.07985643438092534, +0.07996581319924465, 0.0, -0.08301654084812561, -0.27974978085336505, --0.3135774687124175, +0.08300572314082845, +0.27985657360814103, +-0.31342877799575186, 0.0, 0.0, 0.0, -0.0, -0.21721484909061123, +0.21725044259235132, -0.0, 0.0, --0.6603212899417499, -0.47382998205049726, +-0.6601745015855838, +0.47381119601381766, 0.0, -0.031566994679697126, +0.03181202307277254, 0.0, -0.0, 0.0, 0.0, 0.0, -0.41987279273153577, +0.41982354308978265, -0.0, -0.0, 0.0, --0.15704804281369258, -0.05136354590905017, +-0.15701236163909485, +0.05143595038841564, -0.0, --1.3967861171544764, -0.012049836970958124, --0.16190555154009043, +-1.3967255076899114, +0.012164951411760265, +-0.16190436401823916, 0.0, -0.32236468110744876, --0.10807182142138586, +0.3220696681595943, +-0.1082526947395474, 0.0, -0.28113499778312084, -0.347455093844389, +0.2810643037362584, +0.3475914458610097, 0.0, -0.0, -0.21577214682210177, +0.2159865469721421, -0.0, 0.0, --0.3494463283215819, --0.15598019593473778, -0.2937099798752649, +-0.34943241394679975, +-0.15571843423959625, +0.2937142943175603, -0.0, -0.0, --0.6670081562284095, -0.03155616836841969, +-0.6666677255779397, +0.03158187579337792, -0.0, -0.0, -0.0, --0.4821830747864856, --0.1060109448038188, +-0.4825339029883909, +-0.10536520325606542, 0.0, 0.0, -0.18025872807868917, +0.18019166627132638, -0.0, -0.059651718175110925, -0.15412094403273932, +0.060628635086959023, +0.1535326725086174, 0.0, 0.0, -0.4104485276968183, --0.0, +0.41012738584685204, -0.0, --0.0573642521280568, --0.0183387630905243, +-3.9714104351330406e-06, +-0.054919851303674425, +-0.02084871656373664, -0.0, -0.08825537070503531, +0.08831247903148058, -0.0, -0.3576615077317695, +0.3576186594301512, -0.0, -0.20454045632867554, +0.2045708428966265, -0.0, --0.22669760740718553, -0.3391651045388519, -0.40841054414438194, -3.320355416478889e-05, +-0.22633684673764687, +0.33920619628350673, +0.4087551349593832, +0.00014400861713193862, -0.0, -0.009846843806251981, -0.6879938701965732, --0.09809945092041333, +0.009788614358444537, +0.6882150688542139, +-0.09819510616132636, 0.0, 0.0, 0.0, -0.24660181712712595, +0.24647318516426506, 0.0, 0.0, -0.2971937705828685, +0.2974896639283871, -0.0, -0.00042832988368958795, -0.0011714853122736216, -0.3681970714031466, +0.1767238117388548, +0.0971655540387981, +0.09612967228805114, 0.0, --0.4757811965402313, --0.48264457834902585, +-0.47559667034732106, +-0.48262999096637776, -0.0, --0.14897936787284055, +-0.14856884527780762, -0.0, 0.0, 0.0, --0.06864524030396776, +-0.06867861932624512, 0.0, 0.0, 0.0, @@ -184,63 +183,62 @@ constexpr std::array model_coefficients = { -0.0, -0.0, -0.0, --0.0939514698836343, --0.17375737654279771, -0.4280298407194896, -0.07921243395532432, +-0.07016021046322078, +-0.19673925717707294, +0.4279422735214434, +0.07935762516999385, 0.0, 0.0, -0.002297138241541185, +0.001748304149647706, -0.0, -0.0, -0.0, --0.01300337447750894, +-0.01305037928240179, -0.0, -0.0643992239129706, -0.19270927069891977, --0.40965167510194395, +0.2558178171151348, +0.0013887823536121427, +-0.4098203655723224, -0.0, -0.0, --0.12522557448112873, +-0.1250453331155213, 0.0, 0.0, -0.07914189703224955, -0.030383389451194256, +0.10399781691346788, +0.005453211270251716, -0.0, -0.03990905584174161, -1.157147484387678, +0.03981426371078261, +1.157031326454923, 0.0, -0.0, 0.0, 0.0, -0.013087405412848738, +0.4259520667147177, -0.0, -0.17464108300530065, -0.25197880082746205, +6.557171791471991e-07, +0.014181325019371424, 0.0, --0.09217632122309656, +-0.09220432982116501, 0.0, 0.0, 0.0, -0.0, -0.0, --0.13491648330155384, +-0.1103248978017494, -0.0, --0.03688074309361575, --0.058518504088209695, +-0.03628699126870285, +-0.08325838279176839, 0.0, -0.0, --0.03592820829466529, --0.3346781157893913, +-0.3706138376948898, +-2.0791965507275406e-05, -0.0 }; -constexpr unsigned int standardise_feat_count = 24; +constexpr unsigned int standardise_feat_count = 23; constexpr std::array standardise_feat_means = { 11.155786350148368, 1016.2856083086053, -1902.963649851632, 1181.3004451038576, 717.0712166172107, 1987.0066765578636, @@ -267,7 +265,6 @@ constexpr std::array standardise_feat_means = { constexpr std::array standardise_feat_scale = { 12.073811227476089, 926.9436100765363, -1793.6185415841003, 979.2250013928962, 779.8742124298378, 1919.1239982290276, @@ -294,7 +291,6 @@ constexpr std::array standardise_feat_scale = { const std::array feature_sequence{ "adblockRequests", "metrics.firstMeaningfulPaint", - "metrics.interactive", "metrics.observedDomContentLoaded", "metrics.observedFirstVisualChange", "metrics.observedLoad", @@ -718,10 +714,6 @@ const base::flat_map stdfactor_map = { "metrics.firstMeaningfulPaint", { 1016.2856083086053, 926.9436100765363 } }, - { - "metrics.interactive", - { 1902.963649851632, 1793.6185415841003 } - }, { "metrics.observedDomContentLoaded", { 1181.3004451038576, 979.2250013928962 } diff --git a/components/brave_perf_predictor/browser/bandwidth_linreg_unittest.cc b/components/brave_perf_predictor/browser/bandwidth_linreg_unittest.cc index c729a74a8aa6..f137f30fc68a 100644 --- a/components/brave_perf_predictor/browser/bandwidth_linreg_unittest.cc +++ b/components/brave_perf_predictor/browser/bandwidth_linreg_unittest.cc @@ -19,7 +19,7 @@ TEST(BraveSavingsPredictorTest, FeatureArrayGetsPrediction) { TEST(BraveSavingsPredictorTest, HandlesSpecificVectorExample) { // This test needs to be updated for any change in the model constexpr std::array sample = { - 20, 129, 225, 225, 142, 925, 5, 34662, 3, 317818, 9, 1702888, + 20, 129, 225, 142, 925, 5, 34662, 3, 317818, 9, 1702888, 0, 0, 1, 324, 32, 238315, 9, 90131, 54, 2367498, 59, 2384138, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -77,7 +77,6 @@ TEST(BraveSavingsPredictorTest, HandesSpecificFeaturemapExample) { {"thirdParties.Po.st.blocked", 1}, {"adblockRequests", 20}, {"metrics.firstMeaningfulPaint", 129}, - {"metrics.interactive", 225}, {"metrics.observedDomContentLoaded", 225}, {"metrics.observedFirstVisualChange", 142}, {"metrics.observedLoad", 925}, diff --git a/components/brave_perf_predictor/python/config.py b/components/brave_perf_predictor/python/config.py index f3dff71e0f15..3ca97235a5d9 100644 --- a/components/brave_perf_predictor/python/config.py +++ b/components/brave_perf_predictor/python/config.py @@ -20,7 +20,7 @@ 'feature_selector__k': 'all', 'model__alpha': 0.00025, 'model__fit_intercept': True, - 'model__max_iter': 20000, + 'model__max_iter': 10000, 'model__normalize': True, 'model__selection': 'random', 'pre_processor__standardise__with_mean': True, @@ -31,11 +31,11 @@ 'pre_processor__standardise__with_mean': [True], 'pre_processor__standardise__with_std': [True], # 'pre_processor__num_cols_processor__polynomials__degree': [1], - 'feature_selector__k': [200, 'all'], + 'feature_selector__k': ['all'], # 'feature_selector__threshold': [0.0001, 0.001, 0.00001, 0], 'model__alpha': [0.0005, 0.00025, 0.0001], 'model__normalize': [True], - 'model__max_iter': [10000, 20000], + 'model__max_iter': [10000, 20000, 30000], 'model__fit_intercept': [True], 'model__selection': ['random'] } diff --git a/components/brave_perf_predictor/python/model.py b/components/brave_perf_predictor/python/model.py index b7db96338e21..06cb3966b5e0 100644 --- a/components/brave_perf_predictor/python/model.py +++ b/components/brave_perf_predictor/python/model.py @@ -36,7 +36,7 @@ def _load_dataset(path): # Load data and reset index df = pd.read_csv(path, index_col='url', sep='\t') df = df.reset_index(drop=True) - df = df.drop(columns=['experiment']) + df = df.drop(columns=['experiment', 'metrics.interactive']) # Log transform target df['adblockSummary.wastedBytes_target_log10'] = np.log10( From 55226fd962d219677c24c137d8f95ead73f6bca8 Mon Sep 17 00:00:00 2001 From: AndriusA Date: Mon, 4 May 2020 12:20:24 +0100 Subject: [PATCH 6/7] fix tests --- .../browser/bandwidth_savings_predictor.h | 4 ++-- .../browser/bandwidth_savings_predictor_unittest.cc | 6 +++++- .../browser/named_third_party_registry.cc | 2 -- .../browser/named_third_party_registry.h | 1 + .../browser/perf_predictor_tab_helper.cc | 8 ++++---- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h index 1307a5221436..26966d1ced2b 100644 --- a/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h +++ b/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h @@ -10,8 +10,8 @@ #include "base/containers/flat_map.h" #include "base/gtest_prod_util.h" -#include "url/gurl.h" #include "brave/components/brave_perf_predictor/browser/named_third_party_registry.h" +#include "url/gurl.h" namespace page_load_metrics { namespace mojom { @@ -63,7 +63,7 @@ class BandwidthSavingsPredictor { FeaturiseResourceLoading); GURL main_frame_url_; - const NamedThirdPartyRegistry* tp_registry_; // not owned + const NamedThirdPartyRegistry* tp_registry_; // not owned base::flat_map feature_map_; }; diff --git a/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc b/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc index 1d9440df2662..975664bf829e 100644 --- a/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc +++ b/components/brave_perf_predictor/browser/bandwidth_savings_predictor_unittest.cc @@ -5,8 +5,11 @@ #include "brave/components/brave_perf_predictor/browser/bandwidth_savings_predictor.h" +#include + #include "base/containers/flat_map.h" #include "base/run_loop.h" +#include "base/test/task_environment.h" #include "base/time/time.h" #include "chrome/browser/predictors/loading_test_util.h" #include "components/page_load_metrics/common/page_load_metrics.mojom.h" @@ -25,10 +28,11 @@ class BandwidthSavingsPredictorTest : public ::testing::Test { predictor_ = std::make_unique(tp_registry_.get()); // Allow the third party registry to initialize - base::RunLoop().RunUntilIdle(); + env_.RunUntilIdle(); } protected: + base::test::TaskEnvironment env_; std::unique_ptr tp_registry_; std::unique_ptr predictor_; }; diff --git a/components/brave_perf_predictor/browser/named_third_party_registry.cc b/components/brave_perf_predictor/browser/named_third_party_registry.cc index 5c30b370cb7a..7fe65860331d 100644 --- a/components/brave_perf_predictor/browser/named_third_party_registry.cc +++ b/components/brave_perf_predictor/browser/named_third_party_registry.cc @@ -16,8 +16,6 @@ #include "base/strings/string_util.h" #include "base/task/post_task.h" #include "base/task/thread_pool.h" -#include "base/task_runner_util.h" -#include "base/threading/sequenced_task_runner_handle.h" #include "base/values.h" #include "brave/components/brave_perf_predictor/browser/bandwidth_linreg_parameters.h" #include "components/grit/brave_components_resources.h" diff --git a/components/brave_perf_predictor/browser/named_third_party_registry.h b/components/brave_perf_predictor/browser/named_third_party_registry.h index 17ad67d5c496..e06648dc785c 100644 --- a/components/brave_perf_predictor/browser/named_third_party_registry.h +++ b/components/brave_perf_predictor/browser/named_third_party_registry.h @@ -7,6 +7,7 @@ #define BRAVE_COMPONENTS_BRAVE_PERF_PREDICTOR_BROWSER_NAMED_THIRD_PARTY_REGISTRY_H_ #include +#include #include "base/containers/flat_map.h" #include "base/memory/weak_ptr.h" diff --git a/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc b/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc index 7fa39bda70b0..5ac11030f622 100644 --- a/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc +++ b/components/brave_perf_predictor/browser/perf_predictor_tab_helper.cc @@ -39,12 +39,12 @@ content::WebContents* GetWebContents(int render_process_id, PerfPredictorTabHelper::PerfPredictorTabHelper( content::WebContents* web_contents) - : WebContentsObserver(web_contents) { + : WebContentsObserver(web_contents), + bandwidth_predictor_(std::make_unique( + NamedThirdPartyRegistryFactory::GetForBrowserContext( + web_contents->GetBrowserContext()))) { if (web_contents->GetBrowserContext()->IsOffTheRecord()) return; - bandwidth_predictor_ = std::make_unique( - NamedThirdPartyRegistryFactory::GetForBrowserContext( - web_contents->GetBrowserContext())); bandwidth_tracker_ = std::make_unique( user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())); From 27cc455fa83d07716b03441e11021284ca8554ac Mon Sep 17 00:00:00 2001 From: AndriusA Date: Wed, 6 May 2020 19:27:29 +0100 Subject: [PATCH 7/7] fixes build config --- components/brave_perf_predictor/browser/BUILD.gn | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/brave_perf_predictor/browser/BUILD.gn b/components/brave_perf_predictor/browser/BUILD.gn index 696ed8acd930..b0912e8f9af9 100644 --- a/components/brave_perf_predictor/browser/BUILD.gn +++ b/components/brave_perf_predictor/browser/BUILD.gn @@ -24,8 +24,6 @@ source_set("browser") { "named_third_party_registry.h", "named_third_party_registry_factory.cc", "named_third_party_registry_factory.h", - "p3a_bandwidth_savings_permanent_state.cc", - "p3a_bandwidth_savings_permanent_state.h", "p3a_bandwidth_savings_tracker.cc", "p3a_bandwidth_savings_tracker.h", "perf_predictor_page_metrics_observer.cc",