Index: chrome/browser/subresource_filter/subresource_filter_browsertest.cc |
diff --git a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc |
index 73312ad27fe43f352030f66c5c17c35d0ee10255..e42c36c3c030d70e18f7528a63ae4a0f0284a8ce 100644 |
--- a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc |
+++ b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc |
@@ -14,12 +14,14 @@ |
#include "base/macros.h" |
#include "base/path_service.h" |
#include "base/strings/string_piece.h" |
+#include "base/strings/string_util.h" |
#include "base/strings/stringprintf.h" |
#include "base/test/histogram_tester.h" |
#include "chrome/browser/browser_process.h" |
#include "chrome/browser/metrics/subprocess_metrics_provider.h" |
#include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h" |
#include "chrome/browser/safe_browsing/test_safe_browsing_service.h" |
+#include "chrome/browser/safe_browsing/v4_test_utils.h" |
#include "chrome/browser/subresource_filter/test_ruleset_publisher.h" |
#include "chrome/browser/ui/browser.h" |
#include "chrome/browser/ui/browser_commands.h" |
@@ -30,6 +32,7 @@ |
#include "chrome/test/base/ui_test_utils.h" |
#include "components/safe_browsing_db/test_database_manager.h" |
#include "components/safe_browsing_db/util.h" |
+#include "components/safe_browsing_db/v4_database.h" |
#include "components/security_interstitials/content/unsafe_resource.h" |
#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h" |
#include "components/subresource_filter/core/browser/subresource_filter_features.h" |
@@ -57,7 +60,7 @@ namespace { |
// The path to a multi-frame document used for tests. |
static constexpr const char kTestFrameSetPath[] = |
- "subresource_filter/frame_set.html"; |
+ "/subresource_filter/frame_set.html"; |
// Names of DocumentLoad histograms. |
constexpr const char kDocumentLoadActivationLevel[] = |
@@ -98,61 +101,34 @@ constexpr const char kEvaluationWallDuration[] = |
constexpr const char kEvaluationCPUDuration[] = |
"SubresourceFilter.SubresourceLoad.Evaluation.CPUDuration"; |
+#if defined(GOOGLE_CHROME_BUILD) |
+// Names of navigation chain patterns histogram. |
+const char kMatchesPatternHistogramName[] = |
+ "SubresourceFilter.PageLoad.RedirectChainMatchPattern"; |
+const char kNavigationChainSize[] = |
+ "SubresourceFilter.PageLoad.RedirectChainLength"; |
+const char kSubresourceFilterOnlySuffix[] = ".SubresourceFilterOnly"; |
+#endif |
+ |
// Other histograms. |
const char kSubresourceFilterPromptHistogram[] = |
"SubresourceFilter.Prompt.NumVisibility"; |
-// Database manager that allows any URL to be configured as blacklisted for |
-// testing. |
-class FakeSafeBrowsingDatabaseManager |
- : public safe_browsing::TestSafeBrowsingDatabaseManager { |
- public: |
- FakeSafeBrowsingDatabaseManager() {} |
- |
- void AddBlacklistedURL(const GURL& url, |
- safe_browsing::SBThreatType threat_type) { |
- url_to_threat_type_[url] = threat_type; |
- } |
- |
- protected: |
- ~FakeSafeBrowsingDatabaseManager() override {} |
- |
- bool CheckBrowseUrl(const GURL& url, Client* client) override { |
- if (!url_to_threat_type_.count(url)) |
- return true; |
- |
- content::BrowserThread::PostTask( |
- content::BrowserThread::IO, FROM_HERE, |
- base::Bind(&Client::OnCheckBrowseUrlResult, base::Unretained(client), |
- url, url_to_threat_type_[url], |
- safe_browsing::ThreatMetadata())); |
- return false; |
- } |
- |
- bool CheckResourceUrl(const GURL& url, Client* client) override { |
- return true; |
- } |
- |
- bool IsSupported() const override { return true; } |
- bool ChecksAreAlwaysAsync() const override { return false; } |
- bool CanCheckResourceType( |
- content::ResourceType /* resource_type */) const override { |
- return true; |
- } |
- |
- safe_browsing::ThreatSource GetThreatSource() const override { |
- return safe_browsing::ThreatSource::LOCAL_PVER4; |
- } |
- |
- bool CheckExtensionIDs(const std::set<std::string>& extension_ids, |
- Client* client) override { |
- return true; |
- } |
- |
- private: |
- std::map<GURL, safe_browsing::SBThreatType> url_to_threat_type_; |
- |
- DISALLOW_COPY_AND_ASSIGN(FakeSafeBrowsingDatabaseManager); |
+// Human readable representation of expected redirect chain match patterns. |
+// The explanations for the buckets given for the following redirect chain: |
+// A->B->C->D, where A is initial URL and D is a final URL. |
+enum RedirectChainMatchPattern { |
+ EMPTY, // No histograms were recorded. |
+ F0M0L1, // D is a Safe Browsing match. |
+ F0M1L0, // B or C, or both are Safe Browsing matches. |
+ F0M1L1, // B or C, or both and D are Safe Browsing matches. |
+ F1M0L0, // A is Safe Browsing match |
+ F1M0L1, // A and D are Safe Browsing matches. |
+ F1M1L0, // B and/or C and A are Safe Browsing matches. |
+ F1M1L1, // B and/or C and A and D are Safe Browsing matches. |
+ NO_REDIRECTS_HIT, // Redirect chain consists of single URL, aka no redirects |
+ // has happened, and this URL was a Safe Browsing hit. |
+ NUM_HIT_PATTERNS, |
}; |
// UI manager that never actually shows any interstitials, but emulates as if |
@@ -180,12 +156,6 @@ GURL GetURLWithFragment(const GURL& url, base::StringPiece fragment) { |
return url.ReplaceComponents(replacements); |
} |
-GURL GetURLWithQuery(const GURL& url, base::StringPiece query) { |
- GURL::Replacements replacements; |
- replacements.SetQueryStr(query); |
- return url.ReplaceComponents(replacements); |
-} |
- |
} // namespace |
namespace subresource_filter { |
@@ -229,17 +199,42 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest { |
// As a workaround, enable the feature here, then enable the feature once |
// again + set up the field trials later. |
void SetUpCommandLine(base::CommandLine* command_line) override { |
- command_line->AppendSwitchASCII(switches::kEnableFeatures, |
- kSafeBrowsingSubresourceFilter.name); |
+ command_line->AppendSwitchASCII( |
+ switches::kEnableFeatures, |
+ base::JoinString({std::string(kSafeBrowsingSubresourceFilter.name), |
engedy
2017/03/21 13:44:16
nit: I think you can drop std::string, should work
melandory
2017/03/24 15:49:27
Done.
|
+ "SafeBrowsingV4OnlyEnabled"}, |
+ ",")); |
} |
- void SetUpInProcessBrowserTestFixture() override { |
- fake_safe_browsing_database_ = new FakeSafeBrowsingDatabaseManager(); |
- test_safe_browsing_factory_.SetTestDatabaseManager( |
- fake_safe_browsing_database_.get()); |
- test_safe_browsing_factory_.SetTestUIManager(new FakeSafeBrowsingUIManager); |
- safe_browsing::SafeBrowsingService::RegisterFactory( |
- &test_safe_browsing_factory_); |
+ void SetUp() override { |
+ sb_factory_ = |
+ base::MakeUnique<safe_browsing::TestSafeBrowsingServiceFactory>( |
+ safe_browsing::V4FeatureList::V4UsageStatus::V4_ONLY); |
+ sb_factory_->SetTestUIManager(new FakeSafeBrowsingUIManager()); |
+ safe_browsing::SafeBrowsingService::RegisterFactory(sb_factory_.get()); |
+ |
+ safe_browsing::V4Database::RegisterStoreFactoryForTest( |
+ base::WrapUnique(new safe_browsing::TestV4StoreFactory())); |
+ |
+ v4_db_factory_ = new safe_browsing::TestV4DatabaseFactory(); |
+ safe_browsing::V4Database::RegisterDatabaseFactoryForTest( |
+ base::WrapUnique(v4_db_factory_)); |
+ |
+ v4_get_hash_factory_ = |
+ new safe_browsing::TestV4GetHashProtocolManagerFactory(); |
+ safe_browsing::V4GetHashProtocolManager::RegisterFactory( |
+ base::WrapUnique(v4_get_hash_factory_)); |
+ InProcessBrowserTest::SetUp(); |
+ } |
+ |
+ void TearDown() override { |
+ InProcessBrowserTest::TearDown(); |
+ // Unregister test factories after InProcessBrowserTest::TearDown |
+ // (which destructs SafeBrowsingService). |
+ safe_browsing::V4GetHashProtocolManager::RegisterFactory(nullptr); |
+ safe_browsing::V4Database::RegisterDatabaseFactoryForTest(nullptr); |
+ safe_browsing::V4Database::RegisterStoreFactoryForTest(nullptr); |
+ safe_browsing::SafeBrowsingService::RegisterFactory(nullptr); |
} |
void SetUpOnMainThread() override { |
@@ -262,9 +257,24 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest { |
return embedded_test_server()->base_url().Resolve(relative_url); |
} |
+ void MarkUrlAsMatchingListWithId( |
+ const GURL& bad_url, |
+ const safe_browsing::ListIdentifier& list_id, |
+ safe_browsing::ThreatPatternType threat_pattern_type) { |
+ safe_browsing::FullHashInfo full_hash_info = |
+ GetFullHashInfoWithMetadata(bad_url, list_id, threat_pattern_type); |
+ v4_db_factory_->MarkPrefixAsBad(list_id, full_hash_info.full_hash); |
+ v4_get_hash_factory_->AddToFullHashCache(full_hash_info); |
+ } |
+ |
void ConfigureAsPhishingURL(const GURL& url) { |
- fake_safe_browsing_database_->AddBlacklistedURL( |
- url, safe_browsing::SB_THREAT_TYPE_URL_PHISHING); |
+ MarkUrlAsMatchingListWithId(url, safe_browsing::GetUrlSocEngId(), |
+ safe_browsing::ThreatPatternType::NONE); |
+ } |
+ |
+ void ConfigureAsSubresourceFilterOnlyURL(const GURL& url) { |
+ MarkUrlAsMatchingListWithId(url, safe_browsing::GetUrlSubresourceFilterId(), |
+ safe_browsing::ThreatPatternType::NONE); |
} |
content::WebContents* web_contents() { |
@@ -340,14 +350,18 @@ class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest { |
private: |
TestRulesetCreator ruleset_creator_; |
- safe_browsing::TestSafeBrowsingServiceFactory test_safe_browsing_factory_; |
- scoped_refptr<FakeSafeBrowsingDatabaseManager> fake_safe_browsing_database_; |
std::unique_ptr<ScopedSubresourceFilterFeatureToggle> scoped_feature_toggle_; |
TestRulesetPublisher test_ruleset_publisher_; |
const bool measure_performance_; |
const bool whitelist_site_on_reload_; |
+ std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory> sb_factory_; |
+ // Owned by the V4Database. |
+ safe_browsing::TestV4DatabaseFactory* v4_db_factory_; |
+ // Owned by the V4GetHashProtocolManager. |
+ safe_browsing::TestV4GetHashProtocolManagerFactory* v4_get_hash_factory_; |
+ |
DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTestImpl); |
}; |
@@ -479,9 +493,9 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, SubFrameActivation) { |
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, |
HistoryNavigationActivation) { |
- GURL url_without_activation(GetTestUrl(kTestFrameSetPath)); |
- GURL url_with_activation( |
- GetURLWithQuery(url_without_activation, "activation")); |
+ GURL url_with_activation(GetTestUrl(kTestFrameSetPath)); |
+ GURL url_without_activation( |
+ embedded_test_server()->GetURL("a.com", kTestFrameSetPath)); |
ConfigureAsPhishingURL(url_with_activation); |
ASSERT_NO_FATAL_FAILURE( |
SetRulesetToDisallowURLsWithPathSuffix("included_script.js")); |
@@ -1007,4 +1021,53 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterWhitelistSiteOnReloadBrowserTest, |
static_cast<int>(ActivationDecision::URL_WHITELISTED), 1); |
} |
+#if defined(GOOGLE_CHROME_BUILD) |
+// This test is only enabled when GOOGLE_CHROME_BUILD is true because the store |
+// that this test uses is only populated on GOOGLE_CHROME_BUILD builds. |
+IN_PROC_BROWSER_TEST_F( |
+ SubresourceFilterBrowserTest, |
+ ExpectRedirectPatternHistogramsAreRecordedForSubresourceFilterOnlyMatch) { |
+ ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix( |
+ "suffix-that-does-not-match-anything")); |
+ |
+ GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html")); |
+ ConfigureAsSubresourceFilterOnlyURL(url); |
+ |
+ base::HistogramTester tester; |
+ ui_test_utils::NavigateToURL(browser(), url); |
+ |
+ EXPECT_THAT(tester.GetAllSamples(std::string(kMatchesPatternHistogramName) + |
+ std::string(kSubresourceFilterOnlySuffix)), |
+ ::testing::ElementsAre(base::Bucket(NO_REDIRECTS_HIT, 1))); |
+ EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) + |
+ std::string(kSubresourceFilterOnlySuffix)), |
+ ::testing::ElementsAre(base::Bucket(1, 1))); |
+} |
+ |
+IN_PROC_BROWSER_TEST_F( |
+ SubresourceFilterBrowserTest, |
+ ExpectRedirectPatternHistogramsAreRecordedForSubresourceFilterOnlyRedirectMatch) { |
+ ASSERT_NO_FATAL_FAILURE( |
+ SetRulesetToDisallowURLsWithPathSuffix("included_script.js")); |
+ const std::string initial_host("a.com"); |
+ const std::string redirected_host("b.com"); |
+ |
+ GURL redirect_url(embedded_test_server()->GetURL( |
+ redirected_host, "/subresource_filter/frame_with_included_script.html")); |
+ GURL url(embedded_test_server()->GetURL( |
+ initial_host, "/server-redirect?" + redirect_url.spec())); |
+ |
+ ConfigureAsSubresourceFilterOnlyURL(url.GetOrigin()); |
+ base::HistogramTester tester; |
+ ui_test_utils::NavigateToURL(browser(), url); |
+ EXPECT_THAT(tester.GetAllSamples(std::string(kMatchesPatternHistogramName) + |
engedy
2017/03/21 13:44:16
nit: Could you please add a comment to explain tha
melandory
2017/03/24 15:49:27
You mean that the real list is empty?
|
+ std::string(kSubresourceFilterOnlySuffix)), |
+ ::testing::IsEmpty()); |
+ |
+ EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) + |
+ std::string(kSubresourceFilterOnlySuffix)), |
+ ::testing::IsEmpty()); |
+} |
+#endif |
+ |
} // namespace subresource_filter |
engedy
2017/03/21 13:44:16
We should have at least one test that verifies end
melandory
2017/03/24 15:49:27
Done.
|