Commit 68b5a64d authored by Jered Gray's avatar Jered Gray Committed by Commit Bot

Prevent top-level whitelisting of RESOURCE_LOADING_HINTS

Top-level whitelisting for a URL with a PreviewsType of
RESOURCE_LOADING_HINTS doesn't make much sense as the optimizations come
from the page hints and the page hints themselves can provide more
accurate whitelisting data. Because of this, the existence of top-level
whitelisting with a PreviewType of RESOURCE_LOADING_HINTS indicates a
bug with the component's hints.

We're now DCHECKing if the component's hints include top-level
RESOURCE_LOADING_HINTS whitelisting.

IsWhitelisted() itself has been split into IsWhitelistedAtTopLevel()
and IsWhitelistedInPageHints(). IsWhitelistedAtTopLevel() is skipped if
the type is PreviewsType::RESOURCE_LOADING_HINTS.

Additionally, some const-related cleanup was done with function calls in
PreviewHints and HintCache.

Lastly, ResourceLoadingHintsBrowserTest has been modified to test the
full hint flow where it sets and uses page hints. Doing this exposed an
issue with ResourceLoadingHintsHttpsWhitelistedRedirectToHttps, which
was incorrectly passing as a result of some faked data (redirects don't
currently work with hints). The test has been disabled until hints are
made to work with redirects.

Change-Id: I165ccb9ffeba742840a9def15e283a0e254dbb86
Reviewed-on: https://chromium-review.googlesource.com/c/1256007
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarDoug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596455}
parent 8f733b9e
...@@ -298,7 +298,7 @@ class PreviewsOptimizationGuideBrowserTest : public PreviewsBrowserTest { ...@@ -298,7 +298,7 @@ class PreviewsOptimizationGuideBrowserTest : public PreviewsBrowserTest {
void SetNoScriptWhitelist( void SetNoScriptWhitelist(
std::vector<std::string> whitelisted_noscript_sites) { std::vector<std::string> whitelisted_noscript_sites) {
const optimization_guide::ComponentInfo& component_info = const optimization_guide::ComponentInfo& component_info =
test_component_creator_.CreateComponentInfoWithWhitelist( test_component_creator_.CreateComponentInfoWithTopLevelWhitelist(
optimization_guide::proto::NOSCRIPT, whitelisted_noscript_sites); optimization_guide::proto::NOSCRIPT, whitelisted_noscript_sites);
g_browser_process->optimization_guide_service()->ProcessHints( g_browser_process->optimization_guide_service()->ProcessHints(
component_info); component_info);
......
...@@ -14,8 +14,6 @@ ...@@ -14,8 +14,6 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/metrics/subprocess_metrics_provider.h" #include "chrome/browser/metrics/subprocess_metrics_provider.h"
#include "chrome/browser/previews/previews_service.h"
#include "chrome/browser/previews/previews_service_factory.h"
#include "chrome/browser/previews/resource_loading_hints/resource_loading_hints_web_contents_observer.h" #include "chrome/browser/previews/resource_loading_hints/resource_loading_hints_web_contents_observer.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
...@@ -132,31 +130,17 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest { ...@@ -132,31 +130,17 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
cmd->AppendSwitchASCII("force-effective-connection-type", "Slow-2G"); cmd->AppendSwitchASCII("force-effective-connection-type", "Slow-2G");
} }
void SetResourceLoadingHintsPatterns() { void SetResourceLoadingHints(const std::vector<std::string>& hints_sites) {
Profile* profile = browser()->profile(); std::vector<std::string> resource_patterns;
DCHECK(!profile->IsOffTheRecord()); resource_patterns.push_back("foo.jpg");
resource_patterns.push_back("png");
resource_patterns.push_back("woff2");
PreviewsService* previews_service =
PreviewsServiceFactory::GetForProfile(profile);
previews::PreviewsUIService* previews_ui_service =
previews_service->previews_ui_service();
std::vector<std::string> hints;
hints.push_back("foo.jpg");
hints.push_back("png");
hints.push_back("woff2");
previews_ui_service->SetResourceLoadingHintsResourcePatternsToBlock(
https_url_, hints);
}
void SetResourceLoadingHintsWhitelist(
const std::vector<std::string>&
whitelisted_resource_loading_hints_sites) {
const optimization_guide::ComponentInfo& component_info = const optimization_guide::ComponentInfo& component_info =
test_component_creator_.CreateComponentInfoWithWhitelist( test_component_creator_.CreateComponentInfoWithPageHints(
optimization_guide::proto::RESOURCE_LOADING, optimization_guide::proto::RESOURCE_LOADING, hints_sites,
whitelisted_resource_loading_hints_sites); resource_patterns);
g_browser_process->optimization_guide_service()->ProcessHints( g_browser_process->optimization_guide_service()->ProcessHints(
component_info); component_info);
...@@ -275,11 +259,15 @@ class ResourceLoadingHintsBrowserTest ...@@ -275,11 +259,15 @@ class ResourceLoadingHintsBrowserTest
// Previews InfoBar (which these tests triggers) does not work on Mac. // Previews InfoBar (which these tests triggers) does not work on Mac.
// See https://crbug.com/782322 for details. Also occasional flakes on win7 // See https://crbug.com/782322 for details. Also occasional flakes on win7
// (https://crbug.com/789542). // (https://crbug.com/789542).
// Additionally, ResourceLoadingHintsHttpsWhitelistedRedirectToHttps is disabled
// on all OS types until hints are made to work with redirects.
// TODO(jegray): Re-enable ResourceLoadingHintsHttpsWhitelistedRedirectToHttps
// when support for redirects is added: https://crbug.com/891752
#if !defined(OS_MACOSX) && !defined(OS_WIN) #if !defined(OS_MACOSX) && !defined(OS_WIN)
#define MAYBE_ResourceLoadingHintsHttpsWhitelisted \ #define MAYBE_ResourceLoadingHintsHttpsWhitelisted \
ResourceLoadingHintsHttpsWhitelisted ResourceLoadingHintsHttpsWhitelisted
#define MAYBE_ResourceLoadingHintsHttpsWhitelistedRedirectToHttps \ #define MAYBE_ResourceLoadingHintsHttpsWhitelistedRedirectToHttps \
ResourceLoadingHintsHttpsWhitelistedRedirectToHttps DISABLED_ResourceLoadingHintsHttpsWhitelistedRedirectToHttps
#define MAYBE_ResourceLoadingHintsHttpsNoWhitelisted \ #define MAYBE_ResourceLoadingHintsHttpsNoWhitelisted \
ResourceLoadingHintsHttpsNoWhitelisted ResourceLoadingHintsHttpsNoWhitelisted
#define MAYBE_ResourceLoadingHintsHttp ResourceLoadingHintsHttp #define MAYBE_ResourceLoadingHintsHttp ResourceLoadingHintsHttp
...@@ -301,13 +289,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -301,13 +289,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
MAYBE_ResourceLoadingHintsHttpsWhitelisted) { MAYBE_ResourceLoadingHintsHttpsWhitelisted) {
SetExpectedFooJpgRequest(false); SetExpectedFooJpgRequest(false);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
SetResourceLoadingHintsPatterns();
TestOptimizationGuideServiceObserver observer; TestOptimizationGuideServiceObserver observer;
AddTestOptimizationGuideServiceObserver(&observer); AddTestOptimizationGuideServiceObserver(&observer);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// Whitelist test URL for resource loading hints. // Whitelist test URL for resource loading hints.
SetResourceLoadingHintsWhitelist({https_url().host()}); SetResourceLoadingHints({https_url().host()});
observer.WaitForNotification(); observer.WaitForNotification();
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
...@@ -324,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -324,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 1); static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 1);
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"Previews.InfoBarAction.ResourceLoadingHints", 0, 1); "Previews.InfoBarAction.ResourceLoadingHints", 0, 1);
// SetResourceLoadingHintsPatterns sets 3 resource loading hints patterns. // SetResourceLoadingHints sets 3 resource loading hints patterns.
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 1); "ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 1);
...@@ -332,7 +320,9 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -332,7 +320,9 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
// again. // again.
SetExpectedFooJpgRequest(false); SetExpectedFooJpgRequest(false);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
ui_test_utils::NavigateToURL(browser(), https_url()); ui_test_utils::NavigateToURL(browser(), https_url());
RetryForHistogramUntilCountReached( RetryForHistogramUntilCountReached(
&histogram_tester, "ResourceLoadingHints.CountBlockedSubresourcePatterns", &histogram_tester, "ResourceLoadingHints.CountBlockedSubresourcePatterns",
2); 2);
...@@ -343,7 +333,7 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -343,7 +333,7 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 2); static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 2);
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"Previews.InfoBarAction.ResourceLoadingHints", 0, 2); "Previews.InfoBarAction.ResourceLoadingHints", 0, 2);
// SetResourceLoadingHintsPatterns sets 3 resource loading hints patterns. // SetResourceLoadingHints sets 3 resource loading hints patterns.
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 2); "ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 2);
} }
...@@ -353,15 +343,16 @@ IN_PROC_BROWSER_TEST_F( ...@@ -353,15 +343,16 @@ IN_PROC_BROWSER_TEST_F(
MAYBE_ResourceLoadingHintsHttpsWhitelistedRedirectToHttps) { MAYBE_ResourceLoadingHintsHttpsWhitelistedRedirectToHttps) {
SetExpectedFooJpgRequest(false); SetExpectedFooJpgRequest(false);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
SetResourceLoadingHintsPatterns();
TestOptimizationGuideServiceObserver observer; TestOptimizationGuideServiceObserver observer;
AddTestOptimizationGuideServiceObserver(&observer); AddTestOptimizationGuideServiceObserver(&observer);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
SetResourceLoadingHintsWhitelist({https_url().host()}); SetResourceLoadingHints({https_url().host()});
observer.WaitForNotification(); observer.WaitForNotification();
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ui_test_utils::NavigateToURL(browser(), redirect_url()); ui_test_utils::NavigateToURL(browser(), redirect_url());
RetryForHistogramUntilCountReached( RetryForHistogramUntilCountReached(
...@@ -372,7 +363,7 @@ IN_PROC_BROWSER_TEST_F( ...@@ -372,7 +363,7 @@ IN_PROC_BROWSER_TEST_F(
static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 1); static_cast<int>(previews::PreviewsEligibilityReason::ALLOWED), 1);
histogram_tester.ExpectTotalCount( histogram_tester.ExpectTotalCount(
"Previews.InfoBarAction.ResourceLoadingHints", 1); "Previews.InfoBarAction.ResourceLoadingHints", 1);
// SetResourceLoadingHintsPatterns sets 3 resource loading hints patterns. // SetResourceLoadingHints sets 3 resource loading hints patterns.
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 1); "ResourceLoadingHints.CountBlockedSubresourcePatterns", 3, 1);
} }
...@@ -381,18 +372,20 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -381,18 +372,20 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
MAYBE_ResourceLoadingHintsHttpsNoWhitelisted) { MAYBE_ResourceLoadingHintsHttpsNoWhitelisted) {
SetExpectedFooJpgRequest(true); SetExpectedFooJpgRequest(true);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
SetResourceLoadingHintsPatterns();
TestOptimizationGuideServiceObserver observer; TestOptimizationGuideServiceObserver observer;
AddTestOptimizationGuideServiceObserver(&observer); AddTestOptimizationGuideServiceObserver(&observer);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
SetResourceLoadingHintsWhitelist({});
SetResourceLoadingHints({});
observer.WaitForNotification(); observer.WaitForNotification();
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
// The URL is not whitelisted. // The URL is not whitelisted.
ui_test_utils::NavigateToURL(browser(), https_url()); ui_test_utils::NavigateToURL(browser(), https_url());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"Previews.EligibilityReason.ResourceLoadingHints", "Previews.EligibilityReason.ResourceLoadingHints",
static_cast<int>( static_cast<int>(
...@@ -408,13 +401,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -408,13 +401,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
MAYBE_ResourceLoadingHintsHttp) { MAYBE_ResourceLoadingHintsHttp) {
SetExpectedFooJpgRequest(true); SetExpectedFooJpgRequest(true);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
SetResourceLoadingHintsPatterns();
TestOptimizationGuideServiceObserver observer; TestOptimizationGuideServiceObserver observer;
AddTestOptimizationGuideServiceObserver(&observer); AddTestOptimizationGuideServiceObserver(&observer);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// Whitelist test HTTP URL for resource loading hints. // Whitelist test HTTP URL for resource loading hints.
SetResourceLoadingHintsWhitelist({https_url().host()}); SetResourceLoadingHints({https_url().host()});
observer.WaitForNotification(); observer.WaitForNotification();
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
...@@ -435,13 +428,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest, ...@@ -435,13 +428,13 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
MAYBE_ResourceLoadingHintsHttpsWhitelistedNoTransform) { MAYBE_ResourceLoadingHintsHttpsWhitelistedNoTransform) {
SetExpectedFooJpgRequest(true); SetExpectedFooJpgRequest(true);
SetExpectedBarJpgRequest(true); SetExpectedBarJpgRequest(true);
SetResourceLoadingHintsPatterns();
TestOptimizationGuideServiceObserver observer; TestOptimizationGuideServiceObserver observer;
AddTestOptimizationGuideServiceObserver(&observer); AddTestOptimizationGuideServiceObserver(&observer);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// Whitelist test URL for resource loading hints. // Whitelist test URL for resource loading hints.
SetResourceLoadingHintsWhitelist({https_url().host()}); SetResourceLoadingHints({https_url().host()});
observer.WaitForNotification(); observer.WaitForNotification();
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
......
...@@ -24,14 +24,51 @@ TestComponentCreator::~TestComponentCreator() { ...@@ -24,14 +24,51 @@ TestComponentCreator::~TestComponentCreator() {
} }
optimization_guide::ComponentInfo optimization_guide::ComponentInfo
TestComponentCreator::CreateComponentInfoWithWhitelist( TestComponentCreator::CreateComponentInfoWithTopLevelWhitelist(
optimization_guide::proto::OptimizationType optimization_type, optimization_guide::proto::OptimizationType optimization_type,
const std::vector<std::string>& whitelisted_host_suffixes) { const std::vector<std::string>& whitelisted_host_suffixes) {
std::string version_string = base::IntToString(next_component_version_++); optimization_guide::proto::Configuration config;
base::FilePath hints_path = GetFilePath(version_string); for (const auto& whitelisted_site : whitelisted_host_suffixes) {
WriteConfigToFile(optimization_type, hints_path, whitelisted_host_suffixes); optimization_guide::proto::Hint* hint = config.add_hints();
return optimization_guide::ComponentInfo(base::Version(version_string), hint->set_key(whitelisted_site);
hints_path); hint->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::Optimization* optimization =
hint->add_whitelisted_optimizations();
optimization->set_optimization_type(optimization_type);
}
return WriteConfigToFileAndReturnComponentInfo(config);
}
optimization_guide::ComponentInfo
TestComponentCreator::CreateComponentInfoWithPageHints(
optimization_guide::proto::OptimizationType optimization_type,
const std::vector<std::string>& page_hint_host_suffixes,
const std::vector<std::string>& resource_blocking_patterns) {
optimization_guide::proto::Configuration config;
for (const auto& page_hint_site : page_hint_host_suffixes) {
optimization_guide::proto::Hint* hint = config.add_hints();
hint->set_key(page_hint_site);
hint->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::PageHint* page_hint = hint->add_page_hints();
page_hint->set_page_pattern("*");
optimization_guide::proto::Optimization* optimization =
page_hint->add_whitelisted_optimizations();
optimization->set_optimization_type(optimization_type);
for (auto resource_blocking_pattern : resource_blocking_patterns) {
optimization_guide::proto::ResourceLoadingHint* resource_loading_hint =
optimization->add_resource_loading_hints();
resource_loading_hint->set_loading_optimization_type(
optimization_guide::proto::LOADING_BLOCK_RESOURCE);
resource_loading_hint->set_resource_pattern(resource_blocking_pattern);
}
}
return WriteConfigToFileAndReturnComponentInfo(config);
} }
base::FilePath TestComponentCreator::GetFilePath(std::string file_path_suffix) { base::FilePath TestComponentCreator::GetFilePath(std::string file_path_suffix) {
...@@ -42,21 +79,10 @@ base::FilePath TestComponentCreator::GetFilePath(std::string file_path_suffix) { ...@@ -42,21 +79,10 @@ base::FilePath TestComponentCreator::GetFilePath(std::string file_path_suffix) {
} }
void TestComponentCreator::WriteConfigToFile( void TestComponentCreator::WriteConfigToFile(
optimization_guide::proto::OptimizationType optimization_type, const base::FilePath& file_path,
base::FilePath file_path, const optimization_guide::proto::Configuration& config) {
std::vector<std::string> whitelisted_host_suffixes) {
base::ScopedAllowBlockingForTesting allow_blocking; base::ScopedAllowBlockingForTesting allow_blocking;
optimization_guide::proto::Configuration config;
for (auto whitelisted_noscript_site : whitelisted_host_suffixes) {
optimization_guide::proto::Hint* hint = config.add_hints();
hint->set_key(whitelisted_noscript_site);
hint->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::Optimization* optimization =
hint->add_whitelisted_optimizations();
optimization->set_optimization_type(optimization_type);
}
std::string serialized_config; std::string serialized_config;
ASSERT_TRUE(config.SerializeToString(&serialized_config)); ASSERT_TRUE(config.SerializeToString(&serialized_config));
...@@ -65,5 +91,15 @@ void TestComponentCreator::WriteConfigToFile( ...@@ -65,5 +91,15 @@ void TestComponentCreator::WriteConfigToFile(
serialized_config.length())); serialized_config.length()));
} }
optimization_guide::ComponentInfo
TestComponentCreator::WriteConfigToFileAndReturnComponentInfo(
const optimization_guide::proto::Configuration& config) {
std::string version_string = base::IntToString(next_component_version_++);
base::FilePath file_path = GetFilePath(version_string);
WriteConfigToFile(file_path, config);
return optimization_guide::ComponentInfo(base::Version(version_string),
file_path);
}
} // namespace testing } // namespace testing
} // namespace optimization_guide } // namespace optimization_guide
...@@ -5,6 +5,9 @@ ...@@ -5,6 +5,9 @@
#ifndef COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_ #ifndef COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
#define COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_ #define COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
#include <string>
#include <vector>
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -25,9 +28,16 @@ class TestComponentCreator { ...@@ -25,9 +28,16 @@ class TestComponentCreator {
// Creates component data based on |whitelisted_host_suffixes| for // Creates component data based on |whitelisted_host_suffixes| for
// optimization with type |optimization_type| to a temp file // optimization with type |optimization_type| to a temp file
// and returns the ComponentInfo for it. // and returns the ComponentInfo for it.
optimization_guide::ComponentInfo CreateComponentInfoWithWhitelist( optimization_guide::ComponentInfo CreateComponentInfoWithTopLevelWhitelist(
optimization_guide::proto::OptimizationType optimization_type, optimization_guide::proto::OptimizationType optimization_type,
const std::vector<std::string>& whitelisted_host_suffixes); const std::vector<std::string>& whitelisted_host_suffixes);
// Creates component data based on |whitelisted_host_suffixes| with page hints
// for type |optimization_type| blocking resources specified by
// |resource_patterns|.and returns the ComponentInfo for it.
optimization_guide::ComponentInfo CreateComponentInfoWithPageHints(
optimization_guide::proto::OptimizationType optimization_type,
const std::vector<std::string>& whitelisted_host_suffixes,
const std::vector<std::string>& resource_patterns);
private: private:
// Returns the scoped temp directory path with the |file_path_suffix| that is // Returns the scoped temp directory path with the |file_path_suffix| that is
...@@ -35,12 +45,15 @@ class TestComponentCreator { ...@@ -35,12 +45,15 @@ class TestComponentCreator {
// The file itself will not be automatically created. // The file itself will not be automatically created.
base::FilePath GetFilePath(std::string file_path_suffix); base::FilePath GetFilePath(std::string file_path_suffix);
// Writes a configuration of hints for |optimization_type| with the // Writes a configuration of hints to the file path.
// |whitelisted_host_suffixes| to the file path.
void WriteConfigToFile( void WriteConfigToFile(
optimization_guide::proto::OptimizationType optimization_type, const base::FilePath& file_path,
base::FilePath file_path, const optimization_guide::proto::Configuration& config);
std::vector<std::string> whitelisted_host_suffixes);
// Writes a configuration of hints to the file path and returns the
// ComponentInfo for it.
optimization_guide::ComponentInfo WriteConfigToFileAndReturnComponentInfo(
const optimization_guide::proto::Configuration& config);
std::unique_ptr<base::ScopedTempDir> scoped_temp_dir_; std::unique_ptr<base::ScopedTempDir> scoped_temp_dir_;
int next_component_version_; int next_component_version_;
......
...@@ -32,7 +32,7 @@ void HintCache::LoadHint(const std::string& host, HintLoadedCallback callback) { ...@@ -32,7 +32,7 @@ void HintCache::LoadHint(const std::string& host, HintLoadedCallback callback) {
// TODO(dougarnett): Add backing store support to load from. // TODO(dougarnett): Add backing store support to load from.
} }
bool HintCache::IsHintLoaded(const std::string& host) { bool HintCache::IsHintLoaded(const std::string& host) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::string host_suffix = DetermineHostSuffix(host); std::string host_suffix = DetermineHostSuffix(host);
if (host_suffix.empty()) { if (host_suffix.empty()) {
......
...@@ -41,7 +41,7 @@ class HintCache { ...@@ -41,7 +41,7 @@ class HintCache {
void LoadHint(const std::string& host, HintLoadedCallback callback); void LoadHint(const std::string& host, HintLoadedCallback callback);
// Returns whether there is hint data available for |host| in memory. // Returns whether there is hint data available for |host| in memory.
bool IsHintLoaded(const std::string& host); bool IsHintLoaded(const std::string& host) const;
// Returns the hint data for |host| found in memory, otherwise nullptr. // Returns the hint data for |host| found in memory, otherwise nullptr.
const optimization_guide::proto::Hint* GetHint(const std::string& host) const; const optimization_guide::proto::Hint* GetHint(const std::string& host) const;
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include <utility>
#include "base/files/file.h" #include "base/files/file.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
...@@ -191,7 +192,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig( ...@@ -191,7 +192,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
// The condition set ID is a simple increasing counter that matches the // The condition set ID is a simple increasing counter that matches the
// order of hints in the config (where earlier hints in the config take // order of hints in the config (where earlier hints in the config take
// precendence over later hints in the config if there are multiple matches). // precedence over later hints in the config if there are multiple matches).
url_matcher::URLMatcherConditionSet::ID id = 0; url_matcher::URLMatcherConditionSet::ID id = 0;
url_matcher::URLMatcherConditionFactory* condition_factory = url_matcher::URLMatcherConditionFactory* condition_factory =
hints->url_matcher_.condition_factory(); hints->url_matcher_.condition_factory();
...@@ -231,11 +232,17 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig( ...@@ -231,11 +232,17 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
base::Optional<PreviewsType> previews_type = base::Optional<PreviewsType> previews_type =
ConvertProtoOptimizationTypeToPreviewsOptimizationType( ConvertProtoOptimizationTypeToPreviewsOptimizationType(
optimization.optimization_type()); optimization.optimization_type());
if (previews_type.has_value()) { if (!previews_type.has_value()) {
continue;
}
// Resource loading hints should always be page hints; if they appear as
// top-level whitelisted optimizations, then it indicates a bug.
DCHECK(previews_type != PreviewsType::RESOURCE_LOADING_HINTS);
whitelisted_optimizations.insert(std::make_pair( whitelisted_optimizations.insert(std::make_pair(
previews_type.value(), optimization.inflation_percent())); previews_type.value(), optimization.inflation_percent()));
} }
}
url_matcher::URLMatcherCondition condition = url_matcher::URLMatcherCondition condition =
condition_factory->CreateHostSuffixCondition(hint.key()); condition_factory->CreateHostSuffixCondition(hint.key());
all_conditions.push_back(new url_matcher::URLMatcherConditionSet( all_conditions.push_back(new url_matcher::URLMatcherConditionSet(
...@@ -283,7 +290,9 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig( ...@@ -283,7 +290,9 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
"ResourceLoadingHints.PageHints.TotalReceived", "ResourceLoadingHints.PageHints.TotalReceived",
total_page_patterns_with_resource_loading_hints_received); total_page_patterns_with_resource_loading_hints_received);
} }
if (!all_conditions.empty()) {
hints->url_matcher_.AddConditionSets(all_conditions); hints->url_matcher_.AddConditionSets(all_conditions);
}
// Extract any supported large scale blacklists from the configuration. // Extract any supported large scale blacklists from the configuration.
hints->ParseOptimizationFilters(config); hints->ParseOptimizationFilters(config);
...@@ -396,13 +405,26 @@ void PreviewsHints::Initialize() { ...@@ -396,13 +405,26 @@ void PreviewsHints::Initialize() {
bool PreviewsHints::IsWhitelisted(const GURL& url, bool PreviewsHints::IsWhitelisted(const GURL& url,
PreviewsType type, PreviewsType type,
int* out_inflation_percent) { int* out_inflation_percent) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!url.has_host()) if (!url.has_host())
return false; return false;
// First check for being whitelisted at top level for host suffix. return IsWhitelistedAtTopLevel(url, type, out_inflation_percent) ||
IsWhitelistedInPageHints(url, type, out_inflation_percent);
}
bool PreviewsHints::IsWhitelistedAtTopLevel(const GURL& url,
PreviewsType type,
int* out_inflation_percent) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Resource loading hints are not processed in the top-level whitelist.
if (type == PreviewsType::RESOURCE_LOADING_HINTS) {
return false;
}
std::set<url_matcher::URLMatcherConditionSet::ID> matches = std::set<url_matcher::URLMatcherConditionSet::ID> matches =
url_matcher_.MatchURL(url); url_matcher_.MatchURL(url);
...@@ -425,6 +447,14 @@ bool PreviewsHints::IsWhitelisted(const GURL& url, ...@@ -425,6 +447,14 @@ bool PreviewsHints::IsWhitelisted(const GURL& url,
} }
} }
return false;
}
bool PreviewsHints::IsWhitelistedInPageHints(const GURL& url,
PreviewsType type,
int* out_inflation_percent) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!hint_cache_) if (!hint_cache_)
return false; return false;
...@@ -460,7 +490,7 @@ bool PreviewsHints::IsWhitelisted(const GURL& url, ...@@ -460,7 +490,7 @@ bool PreviewsHints::IsWhitelisted(const GURL& url,
return false; return false;
} }
bool PreviewsHints::IsBlacklisted(const GURL& url, PreviewsType type) { bool PreviewsHints::IsBlacklisted(const GURL& url, PreviewsType type) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!url.has_host()) if (!url.has_host())
......
...@@ -48,16 +48,16 @@ class PreviewsHints { ...@@ -48,16 +48,16 @@ class PreviewsHints {
void Initialize(); void Initialize();
// Whether the URL is whitelisted for the given previews type. If so, // Whether the URL is whitelisted for the given previews type. If so,
// |out_inflation_percent| will be populated if meta data available for it. // |out_inflation_percent| will be populated if metadata is available for it.
// This first checks the top-level whitelist and, if not whitelisted there, // This first checks the top-level whitelist and, if not whitelisted there,
// it will check the HintCache for having a loaded, matching PageHint that // it will check the HintCache for having a loaded, matching PageHint that
// whitelists it. // whitelists it.
bool IsWhitelisted(const GURL& url, bool IsWhitelisted(const GURL& url,
PreviewsType type, PreviewsType type,
int* out_inflation_percent); int* out_inflation_percent) const;
// Whether the URL is blacklisted for the given previews type. // Whether the URL is blacklisted for the given previews type.
bool IsBlacklisted(const GURL& url, PreviewsType type); bool IsBlacklisted(const GURL& url, PreviewsType type) const;
// Returns whether |url| may have PageHints and triggers asynchronous load // Returns whether |url| may have PageHints and triggers asynchronous load
// of such hints are not currently available synchronously. |callback| is // of such hints are not currently available synchronously. |callback| is
...@@ -70,6 +70,20 @@ class PreviewsHints { ...@@ -70,6 +70,20 @@ class PreviewsHints {
PreviewsHints(); PreviewsHints();
// Returns whether |url| is whitelisted in |whitelist_|. If it is, then
// |out_inflation_percent| will be populated if metadata is available for it.
// NOTE: PreviewsType::RESOURCE_LOADING_HINTS cannot be whitelisted at the
// top-level.
bool IsWhitelistedAtTopLevel(const GURL& url,
PreviewsType type,
int* out_inflation_percent) const;
// Returns whether |url| is whitelisted in the page hints contained within
// |hint_cache_|. If it is, then |out_inflation_percent| will be populated if
// metadata is available for it.
bool IsWhitelistedInPageHints(const GURL& url,
PreviewsType type,
int* out_inflation_percent) const;
// Parses optimization filters from |config| and populates corresponding // Parses optimization filters from |config| and populates corresponding
// supported blacklists in this object. // supported blacklists in this object.
void ParseOptimizationFilters( void ParseOptimizationFilters(
......
...@@ -299,28 +299,44 @@ TEST_F(PreviewsOptimizationGuideTest, ...@@ -299,28 +299,44 @@ TEST_F(PreviewsOptimizationGuideTest,
// Test when resource loading hints are enabled. // Test when resource loading hints are enabled.
TEST_F(PreviewsOptimizationGuideTest, TEST_F(PreviewsOptimizationGuideTest,
ProcessHintsWhitelistForResourceLoadingHintsPopulatedCorrectly) { ProcessHintsWhitelistForResourceLoadingHintsPopulatedCorrectly) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
optimization_guide::proto::Configuration config; optimization_guide::proto::Configuration config;
optimization_guide::proto::Hint* hint1 = config.add_hints(); optimization_guide::proto::Hint* hint1 = config.add_hints();
hint1->set_key("facebook.com"); hint1->set_key("facebook.com");
hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX); hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
page_hint1->set_page_pattern("*");
optimization_guide::proto::Optimization* optimization1 = optimization_guide::proto::Optimization* optimization1 =
hint1->add_whitelisted_optimizations(); page_hint1->add_whitelisted_optimizations();
optimization1->set_optimization_type( optimization1->set_optimization_type(
optimization_guide::proto::RESOURCE_LOADING); optimization_guide::proto::RESOURCE_LOADING);
// Add a second optimization to ensure that the applicable optimizations are
// still whitelisted. // Add additional optimizations to ensure that the applicable optimizations
// are still whitelisted.
optimization_guide::proto::Optimization* optimization2 = optimization_guide::proto::Optimization* optimization2 =
hint1->add_whitelisted_optimizations(); hint1->add_whitelisted_optimizations();
optimization2->set_optimization_type( optimization2->set_optimization_type(
optimization_guide::proto::TYPE_UNSPECIFIED); optimization_guide::proto::TYPE_UNSPECIFIED);
optimization_guide::proto::PageHint* page_hint2 = hint1->add_page_hints();
page_hint2->set_page_pattern("*");
optimization_guide::proto::Optimization* optimization3 =
page_hint2->add_whitelisted_optimizations();
optimization3->set_optimization_type(optimization_guide::proto::NOSCRIPT);
// Add a second hint. // Add a second hint.
optimization_guide::proto::Hint* hint2 = config.add_hints(); optimization_guide::proto::Hint* hint2 = config.add_hints();
hint2->set_key("twitter.com"); hint2->set_key("twitter.com");
hint2->set_key_representation(optimization_guide::proto::HOST_SUFFIX); hint2->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::Optimization* optimization3 = optimization_guide::proto::PageHint* page_hint3 = hint2->add_page_hints();
hint2->add_whitelisted_optimizations(); page_hint3->set_page_pattern("*");
optimization3->set_optimization_type( optimization_guide::proto::Optimization* optimization4 =
page_hint3->add_whitelisted_optimizations();
optimization4->set_optimization_type(
optimization_guide::proto::RESOURCE_LOADING); optimization_guide::proto::RESOURCE_LOADING);
ProcessHints(config, "2.0.0"); ProcessHints(config, "2.0.0");
RunUntilIdle(); RunUntilIdle();
...@@ -341,6 +357,9 @@ TEST_F(PreviewsOptimizationGuideTest, ...@@ -341,6 +357,9 @@ TEST_F(PreviewsOptimizationGuideTest,
TEST_F( TEST_F(
PreviewsOptimizationGuideTest, PreviewsOptimizationGuideTest,
ProcessHintsWhitelistForNoScriptAndResourceLoadingHintsPopulatedCorrectly) { ProcessHintsWhitelistForNoScriptAndResourceLoadingHintsPopulatedCorrectly) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
optimization_guide::proto::Configuration config; optimization_guide::proto::Configuration config;
optimization_guide::proto::Hint* hint1 = config.add_hints(); optimization_guide::proto::Hint* hint1 = config.add_hints();
hint1->set_key("facebook.com"); hint1->set_key("facebook.com");
...@@ -348,20 +367,25 @@ TEST_F( ...@@ -348,20 +367,25 @@ TEST_F(
optimization_guide::proto::Optimization* optimization1 = optimization_guide::proto::Optimization* optimization1 =
hint1->add_whitelisted_optimizations(); hint1->add_whitelisted_optimizations();
optimization1->set_optimization_type(optimization_guide::proto::NOSCRIPT); optimization1->set_optimization_type(optimization_guide::proto::NOSCRIPT);
// Add a second optimization to ensure that the applicable optimizations are // Add a second optimization to ensure that the applicable optimizations are
// still whitelisted. // still whitelisted.
optimization_guide::proto::Optimization* optimization2 = optimization_guide::proto::Optimization* optimization2 =
hint1->add_whitelisted_optimizations(); hint1->add_whitelisted_optimizations();
optimization2->set_optimization_type( optimization2->set_optimization_type(
optimization_guide::proto::TYPE_UNSPECIFIED); optimization_guide::proto::TYPE_UNSPECIFIED);
// Add a second hint. // Add a second hint.
optimization_guide::proto::Hint* hint2 = config.add_hints(); optimization_guide::proto::Hint* hint2 = config.add_hints();
hint2->set_key("twitter.com"); hint2->set_key("twitter.com");
hint2->set_key_representation(optimization_guide::proto::HOST_SUFFIX); hint2->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::PageHint* page_hint1 = hint2->add_page_hints();
page_hint1->set_page_pattern("*");
optimization_guide::proto::Optimization* optimization3 = optimization_guide::proto::Optimization* optimization3 =
hint2->add_whitelisted_optimizations(); page_hint1->add_whitelisted_optimizations();
optimization3->set_optimization_type( optimization3->set_optimization_type(
optimization_guide::proto::RESOURCE_LOADING); optimization_guide::proto::RESOURCE_LOADING);
ProcessHints(config, "2.0.0"); ProcessHints(config, "2.0.0");
RunUntilIdle(); RunUntilIdle();
...@@ -389,6 +413,9 @@ TEST_F( ...@@ -389,6 +413,9 @@ TEST_F(
void PreviewsOptimizationGuideTest::DoExperimentFlagTest( void PreviewsOptimizationGuideTest::DoExperimentFlagTest(
base::Optional<std::string> experiment_name, base::Optional<std::string> experiment_name,
bool expect_enabled) { bool expect_enabled) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
optimization_guide::proto::Configuration config; optimization_guide::proto::Configuration config;
// Create a hint with two optimizations. One may be marked experimental // Create a hint with two optimizations. One may be marked experimental
...@@ -403,9 +430,12 @@ void PreviewsOptimizationGuideTest::DoExperimentFlagTest( ...@@ -403,9 +430,12 @@ void PreviewsOptimizationGuideTest::DoExperimentFlagTest(
optimization1->set_experiment_name(experiment_name.value()); optimization1->set_experiment_name(experiment_name.value());
} }
optimization1->set_optimization_type(optimization_guide::proto::NOSCRIPT); optimization1->set_optimization_type(optimization_guide::proto::NOSCRIPT);
// RESOURCE_LOADING is never marked experimental. // RESOURCE_LOADING is never marked experimental.
optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
page_hint1->set_page_pattern("*");
optimization_guide::proto::Optimization* optimization2 = optimization_guide::proto::Optimization* optimization2 =
hint1->add_whitelisted_optimizations(); page_hint1->add_whitelisted_optimizations();
optimization2->set_optimization_type( optimization2->set_optimization_type(
optimization_guide::proto::RESOURCE_LOADING); optimization_guide::proto::RESOURCE_LOADING);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment