Skip to content

Commit

Permalink
Convert ActivationLevel to a mojom enum
Browse files Browse the repository at this point in the history
The only behavior change here is operator<<, which is pre-defined for mojo
enums. This is in preparation to moving subresource_filter to mojo.

Bug: 820612
Change-Id: I5ff3f4d55588bdb153f5875885824cd0cc315e13
Reviewed-on: https://chromium-review.googlesource.com/1213533
Commit-Queue: Charlie Harrison <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Josh Karlin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#589978}
  • Loading branch information
csharrison authored and Commit Bot committed Sep 10, 2018
1 parent 1e7ae38 commit 91113db
Show file tree
Hide file tree
Showing 43 changed files with 431 additions and 402 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
#include "chrome/test/base/ui_test_utils.h"
#include "components/subresource_filter/content/browser/content_ruleset_service.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/core/common/test_ruleset_utils.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "net/dns/mock_host_resolver.h"
Expand Down Expand Up @@ -134,7 +134,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
SubresourceFilter) {
ResetConfiguration(subresource_filter::Configuration(
subresource_filter::ActivationLevel::DRYRUN,
subresource_filter::mojom::ActivationLevel::kDryRun,
subresource_filter::ActivationScope::ALL_SITES));
base::HistogramTester histogram_tester;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ TEST_F(AdsPageLoadMetricsObserverTest, ResourceBeforeAdFrameCommits) {
TEST_F(AdsPageLoadMetricsObserverTest, AllAdTypesInPage) {
// Make this page DRYRUN.
scoped_configuration().ResetConfiguration(subresource_filter::Configuration(
subresource_filter::ActivationLevel::DRYRUN,
subresource_filter::mojom::ActivationLevel::kDryRun,
subresource_filter::ActivationScope::ALL_SITES));

RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/core/common/activation_state.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"

Expand Down Expand Up @@ -103,33 +103,34 @@ void ChromeSubresourceFilterClient::ShowNotification() {
}
}

subresource_filter::ActivationLevel
subresource_filter::mojom::ActivationLevel
ChromeSubresourceFilterClient::OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::ActivationLevel initial_activation_level,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) {
DCHECK(navigation_handle->IsInMainFrame());

subresource_filter::ActivationLevel effective_activation_level =
subresource_filter::mojom::ActivationLevel effective_activation_level =
initial_activation_level;
if (activated_via_devtools_) {
effective_activation_level = subresource_filter::ActivationLevel::ENABLED;
effective_activation_level =
subresource_filter::mojom::ActivationLevel::kEnabled;
*decision = subresource_filter::ActivationDecision::FORCED_ACTIVATION;
}

const GURL& url(navigation_handle->GetURL());
if (url.SchemeIsHTTPOrHTTPS()) {
settings_manager_->ResetSiteMetadataBasedOnActivation(
url, effective_activation_level ==
subresource_filter::ActivationLevel::ENABLED);
subresource_filter::mojom::ActivationLevel::kEnabled);
}

if (settings_manager_->GetSitePermission(url) == CONTENT_SETTING_ALLOW) {
if (effective_activation_level ==
subresource_filter::ActivationLevel::ENABLED) {
subresource_filter::mojom::ActivationLevel::kEnabled) {
*decision = subresource_filter::ActivationDecision::URL_WHITELISTED;
}
return subresource_filter::ActivationLevel::DISABLED;
return subresource_filter::mojom::ActivationLevel::kDisabled;
}
return effective_activation_level;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ class ChromeSubresourceFilterClient

// SubresourceFilterClient:
void ShowNotification() override;
subresource_filter::ActivationLevel OnPageActivationComputed(
subresource_filter::mojom::ActivationLevel OnPageActivationComputed(
content::NavigationHandle* navigation_handle,
subresource_filter::ActivationLevel initial_activation_level,
subresource_filter::mojom::ActivationLevel initial_activation_level,
subresource_filter::ActivationDecision* decision) override;

// Should be called by devtools in response to a protocol command to enable ad
Expand Down
20 changes: 12 additions & 8 deletions chrome/browser/subresource_filter/ruleset_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
auto ruleset_handle =
std::make_unique<VerifiedRuleset::Handle>(service->ruleset_dealer());
AsyncDocumentSubresourceFilter::InitializationParams params(
GURL("https://example.com/"), ActivationLevel::ENABLED, false);
GURL("https://example.com/"), mojom::ActivationLevel::kEnabled, false);

testing::TestActivationStateCallbackReceiver receiver;
AsyncDocumentSubresourceFilter filter(ruleset_handle.get(), std::move(params),
receiver.GetCallback());
receiver.WaitForActivationDecision();
receiver.ExpectReceivedOnce(ActivationState(ActivationLevel::ENABLED));
receiver.ExpectReceivedOnce(
ActivationState(mojom::ActivationLevel::kEnabled));
histogram_tester.ExpectUniqueSample(kIndexedRulesetVerifyHistogram,
VerifyStatus::kPassValidChecksum, 1);
}
Expand All @@ -101,13 +102,14 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, NoRuleset_NoActivation) {
auto ruleset_handle =
std::make_unique<VerifiedRuleset::Handle>(service->ruleset_dealer());
AsyncDocumentSubresourceFilter::InitializationParams params(
GURL("https://example.com/"), ActivationLevel::ENABLED, false);
GURL("https://example.com/"), mojom::ActivationLevel::kEnabled, false);

testing::TestActivationStateCallbackReceiver receiver;
AsyncDocumentSubresourceFilter filter(ruleset_handle.get(), std::move(params),
receiver.GetCallback());
receiver.WaitForActivationDecision();
receiver.ExpectReceivedOnce(ActivationState(ActivationLevel::DISABLED));
receiver.ExpectReceivedOnce(
ActivationState(mojom::ActivationLevel::kDisabled));
histogram_tester.ExpectTotalCount(kIndexedRulesetVerifyHistogram, 0);
}

Expand Down Expand Up @@ -139,13 +141,14 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, InvalidRuleset_Checksum) {
auto ruleset_handle =
std::make_unique<VerifiedRuleset::Handle>(service->ruleset_dealer());
AsyncDocumentSubresourceFilter::InitializationParams params(
GURL("https://example.com/"), ActivationLevel::ENABLED, false);
GURL("https://example.com/"), mojom::ActivationLevel::kEnabled, false);

testing::TestActivationStateCallbackReceiver receiver;
AsyncDocumentSubresourceFilter filter(ruleset_handle.get(), std::move(params),
receiver.GetCallback());
receiver.WaitForActivationDecision();
receiver.ExpectReceivedOnce(ActivationState(ActivationLevel::DISABLED));
receiver.ExpectReceivedOnce(
ActivationState(mojom::ActivationLevel::kDisabled));
RulesetVerificationStatus dealer_status = GetRulesetVerification();
EXPECT_EQ(RulesetVerificationStatus::kCorrupt, dealer_status);
// If AdTagging is enabled, then the initial SetRuleset will trigger
Expand Down Expand Up @@ -188,13 +191,14 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
auto ruleset_handle =
std::make_unique<VerifiedRuleset::Handle>(service->ruleset_dealer());
AsyncDocumentSubresourceFilter::InitializationParams params(
GURL("https://example.com/"), ActivationLevel::ENABLED, false);
GURL("https://example.com/"), mojom::ActivationLevel::kEnabled, false);

testing::TestActivationStateCallbackReceiver receiver;
AsyncDocumentSubresourceFilter filter(ruleset_handle.get(), std::move(params),
receiver.GetCallback());
receiver.WaitForActivationDecision();
receiver.ExpectReceivedOnce(ActivationState(ActivationLevel::DISABLED));
receiver.ExpectReceivedOnce(
ActivationState(mojom::ActivationLevel::kDisabled));
RulesetVerificationStatus dealer_status = GetRulesetVerification();
EXPECT_EQ(RulesetVerificationStatus::kCorrupt, dealer_status);
histogram_tester.ExpectUniqueSample(kIndexedRulesetVerifyHistogram,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
#include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_state.h"
#include "components/subresource_filter/core/common/common_features.h"
#include "components/subresource_filter/core/common/scoped_timers.h"
#include "components/subresource_filter/core/common/test_ruleset_creator.h"
#include "components/subresource_filter/core/common/test_ruleset_utils.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "components/url_pattern_index/proto/rules.pb.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
Expand Down Expand Up @@ -150,7 +150,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterListInsertingBrowserTest,
ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
"suffix-that-does-not-match-anything"));

Configuration config(subresource_filter::ActivationLevel::ENABLED,
Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::SUBRESOURCE_FILTER);
ResetConfiguration(std::move(config));
Expand Down Expand Up @@ -182,7 +182,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterListInsertingBrowserTest,
ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
"suffix-that-does-not-match-anything"));

Configuration config(subresource_filter::ActivationLevel::ENABLED,
Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::BETTER_ADS);
ResetConfiguration(std::move(config));
Expand Down Expand Up @@ -317,7 +317,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
web_contents()->SetDelegate(&console_observer);

Configuration config(
subresource_filter::ActivationLevel::DISABLED,
subresource_filter::mojom::ActivationLevel::kDisabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::PHISHING_INTERSTITIAL);
ResetConfiguration(std::move(config));
Expand All @@ -342,7 +342,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
web_contents()->SetDelegate(&console_observer);

Configuration config(
subresource_filter::ActivationLevel::DRYRUN,
subresource_filter::mojom::ActivationLevel::kDryRun,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::PHISHING_INTERSTITIAL);
ResetConfiguration(std::move(config));
Expand Down Expand Up @@ -709,7 +709,8 @@ void ExpectHistogramsAreRecordedForTestFrameSet(

tester.ExpectUniqueSample(
kDocumentLoadActivationLevel,
static_cast<base::Histogram::Sample>(ActivationLevel::ENABLED), 6);
static_cast<base::Histogram::Sample>(mojom::ActivationLevel::kEnabled),
6);

EXPECT_THAT(tester.GetAllSamples(kSubresourceLoadsTotal),
::testing::ElementsAre(base::Bucket(0, 3), base::Bucket(2, 3)));
Expand Down Expand Up @@ -783,7 +784,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
// Although SubresourceFilterAgents still record the activation decision.
tester.ExpectUniqueSample(
kDocumentLoadActivationLevel,
static_cast<base::Histogram::Sample>(ActivationLevel::DISABLED), 6);
static_cast<base::Histogram::Sample>(mojom::ActivationLevel::kDisabled),
6);
}

IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@

#include "chrome/browser/subresource_filter/subresource_filter_test_harness.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_list.h"
#include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

using subresource_filter::ActivationList;
using subresource_filter::ActivationScope;
using subresource_filter::ActivationLevel;
using subresource_filter::mojom::ActivationLevel;

using FlattenedConfig =
std::tuple<ActivationScope, ActivationList, ActivationLevel>;
Expand Down Expand Up @@ -75,6 +75,6 @@ INSTANTIATE_TEST_CASE_P(
ActivationList::SOCIAL_ENG_ADS_INTERSTITIAL,
ActivationList::PHISHING_INTERSTITIAL,
ActivationList::SUBRESOURCE_FILTER),
::testing::Values(ActivationLevel::ENABLED,
ActivationLevel::DISABLED,
ActivationLevel::DRYRUN)));
::testing::Values(ActivationLevel::kEnabled,
ActivationLevel::kDisabled,
ActivationLevel::kDryRun)));
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterListInsertingBrowserTest,
ConfigureURLWithWarning(url,
{safe_browsing::SubresourceFilterType::BETTER_ADS});

Configuration config(subresource_filter::ActivationLevel::ENABLED,
Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::BETTER_ADS);
ResetConfiguration(std::move(config));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class SubresourceFilterPopupBrowserTest
public:
void SetUpOnMainThread() override {
SubresourceFilterBrowserTest::SetUpOnMainThread();
Configuration config(subresource_filter::ActivationLevel::ENABLED,
Configuration config(subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::BETTER_ADS);
ResetConfiguration(std::move(config));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h"
#include "components/subresource_filter/core/browser/ruleset_service.h"
#include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_list.h"
#include "components/subresource_filter/core/common/test_ruleset_creator.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "content/public/browser/navigation_throttle.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_renderer_host.h"
Expand All @@ -47,7 +47,7 @@ void SubresourceFilterTestHarness::SetUp() {

// Ensure correct features.
scoped_configuration_.ResetConfiguration(subresource_filter::Configuration(
subresource_filter::ActivationLevel::ENABLED,
subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::SUBRESOURCE_FILTER));

Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/subresource_filter/subresource_filter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
#include "components/subresource_filter/core/common/activation_decision.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_list.h"
#include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/core/common/load_policy.h"
#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
#include "content/public/browser/devtools_agent_host.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -73,7 +73,7 @@ TEST_F(SubresourceFilterTest, ActivationToDryRun_ClearsSiteMetadata) {
// If the site later activates as DRYRUN due to e.g. a configuration change,
// it should also be removed from the metadata.
scoped_configuration().ResetConfiguration(subresource_filter::Configuration(
subresource_filter::ActivationLevel::DRYRUN,
subresource_filter::mojom::ActivationLevel::kDryRun,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::SUBRESOURCE_FILTER));

Expand Down Expand Up @@ -107,7 +107,7 @@ TEST_F(SubresourceFilterTest, SimpleAllowedLoad_WithObserver) {
subresource_filter::TestSubresourceFilterObserver observer(web_contents());
SimulateNavigateAndCommit(url, main_rfh());

EXPECT_EQ(subresource_filter::ActivationLevel::ENABLED,
EXPECT_EQ(subresource_filter::mojom::ActivationLevel::kEnabled,
observer.GetPageActivation(url).value());

GURL allowed_url("https://example.test/foo");
Expand All @@ -126,7 +126,7 @@ TEST_F(SubresourceFilterTest, SimpleDisallowedLoad_WithObserver) {
subresource_filter::TestSubresourceFilterObserver observer(web_contents());
SimulateNavigateAndCommit(url, main_rfh());

EXPECT_EQ(subresource_filter::ActivationLevel::ENABLED,
EXPECT_EQ(subresource_filter::mojom::ActivationLevel::kEnabled,
observer.GetPageActivation(url).value());

GURL disallowed_url(SubresourceFilterTest::kDefaultDisallowedUrl);
Expand Down Expand Up @@ -283,7 +283,7 @@ TEST_F(SubresourceFilterTest, NotifySafeBrowsing) {

TEST_F(SubresourceFilterTest, WarningSite_NoMetadata) {
subresource_filter::Configuration config(
subresource_filter::ActivationLevel::ENABLED,
subresource_filter::mojom::ActivationLevel::kEnabled,
subresource_filter::ActivationScope::ACTIVATION_LIST,
subresource_filter::ActivationList::BETTER_ADS);
scoped_configuration().ResetConfiguration(std::move(config));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ ActivationStateComputingNavigationThrottle::CreateForSubframe(
VerifiedRuleset::Handle* ruleset_handle,
const ActivationState& parent_activation_state) {
DCHECK(!navigation_handle->IsInMainFrame());
DCHECK_NE(ActivationLevel::DISABLED,
DCHECK_NE(mojom::ActivationLevel::kDisabled,
parent_activation_state.activation_level);
DCHECK(ruleset_handle);
return base::WrapUnique(new ActivationStateComputingNavigationThrottle(
Expand All @@ -59,7 +59,8 @@ void ActivationStateComputingNavigationThrottle::
VerifiedRuleset::Handle* ruleset_handle,
const ActivationState& page_activation_state) {
DCHECK(navigation_handle()->IsInMainFrame());
DCHECK_NE(ActivationLevel::DISABLED, page_activation_state.activation_level);
DCHECK_NE(mojom::ActivationLevel::kDisabled,
page_activation_state.activation_level);
parent_activation_state_ = page_activation_state;
ruleset_handle_ = ruleset_handle;
}
Expand Down
Loading

0 comments on commit 91113db

Please sign in to comment.