Skip to content

Commit

Permalink
[DIPS] move dips_features to //content/public
Browse files Browse the repository at this point in the history
As a first step to moving the entire dips project to //content, move the feature flag to //content.

Bug: 1399545
Change-Id: I1a1b7e69c6fa90b6bdaee6956d0ef9ab43287c85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4770460
Reviewed-by: Joshua Hood <jdh@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Patricia Alfonso <trishalfonso@google.com>
Cr-Commit-Position: refs/heads/main@{#1184377}
  • Loading branch information
Patricia Alfonso authored and Chromium LUCI CQ committed Aug 16, 2023
1 parent 064ec1b commit b601bb0
Show file tree
Hide file tree
Showing 28 changed files with 259 additions and 252 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,6 @@ static_library("browser") {
"dips/dips_cleanup_service_factory.h",
"dips/dips_database.cc",
"dips/dips_database.h",
"dips/dips_features.cc",
"dips/dips_features.h",
"dips/dips_redirect_info.cc",
"dips/dips_redirect_info.h",
"dips/dips_service.cc",
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/companion/core/features.h"
#include "chrome/browser/companion/visual_search/features.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/fast_checkout/fast_checkout_features.h"
#include "chrome/browser/feature_guide/notifications/feature_notification_guide_service.h"
#include "chrome/browser/flag_descriptions.h"
Expand Down Expand Up @@ -9160,7 +9159,7 @@ const FeatureEntry kFeatureEntries[] = {

{"bounce-tracking-mitigations", flag_descriptions::kDIPSName,
flag_descriptions::kDIPSDescription, kOsAll,
FEATURE_WITH_PARAMS_VALUE_TYPE(dips::kFeature, kDIPSVariations, "DIPS")},
FEATURE_WITH_PARAMS_VALUE_TYPE(features::kDIPS, kDIPSVariations, "DIPS")},

#if BUILDFLAG(IS_CHROMEOS_ASH)
{kBorealisBigGlInternalName, flag_descriptions::kBorealisBigGlName,
Expand Down
19 changes: 9 additions & 10 deletions chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "chrome/browser/data_saver/data_saver.h"
#include "chrome/browser/devtools/devtools_window.h"
#include "chrome/browser/devtools/protocol/devtools_protocol_test_support.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_service.h"
#include "chrome/browser/dips/dips_storage.h"
#include "chrome/browser/extensions/extension_service.h"
Expand Down Expand Up @@ -397,7 +396,7 @@ class DevToolsProtocolTest_BounceTrackingMitigations
protected:
void SetUp() override {
scoped_feature_list_.InitAndEnableFeatureWithParameters(
dips::kFeature,
features::kDIPS,
{{"delete", "true"}, {"triggering_action", "stateful_bounce"}});

DevToolsProtocolTest::SetUp();
Expand Down Expand Up @@ -454,13 +453,13 @@ class DIPSStatusDevToolsProtocolTest
: public DevToolsProtocolTest,
public testing::WithParamInterface<std::tuple<bool, bool, std::string>> {
// The fields of `GetParam()` indicate/control the following:
// `std::get<0>(GetParam())` => `dips::kFeature`
// `std::get<1>(GetParam())` => `dips::kDeletionEnabled`
// `std::get<2>(GetParam())` => `dips::kTriggeringAction`
// `std::get<0>(GetParam())` => `features::kDIPS`
// `std::get<1>(GetParam())` => `features::kDIPSDeletionEnabled`
// `std::get<2>(GetParam())` => `features::kDIPSTriggeringAction`
//
// In order for Bounce Tracking Mitigations to take effect, `kFeature` must
// be true/enabled, `kDeletionEnabled` must be true, and `kTriggeringAction`
// must NOT be `none`.
// In order for Bounce Tracking Mitigations to take effect, `features::kDIPS`
// must be true/enabled, `kDeletionEnabled` must be true, and
// `kTriggeringAction` must NOT be `none`.
//
// Note: Bounce Tracking Mitigations issues only report sites that would
// be affected when `kTriggeringAction` is set to 'stateful_bounce'.
Expand All @@ -469,11 +468,11 @@ class DIPSStatusDevToolsProtocolTest
void SetUp() override {
if (std::get<0>(GetParam())) {
scoped_feature_list_.InitAndEnableFeatureWithParameters(
dips::kFeature,
features::kDIPS,
{{"delete", (std::get<1>(GetParam()) ? "true" : "false")},
{"triggering_action", std::get<2>(GetParam())}});
} else {
scoped_feature_list_.InitAndDisableFeature(dips::kFeature);
scoped_feature_list_.InitAndDisableFeature(features::kDIPS);
}

DevToolsProtocolTest::SetUp();
Expand Down
12 changes: 6 additions & 6 deletions chrome/browser/devtools/protocol/system_info_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

#include "chrome/browser/devtools/protocol/system_info_handler.h"

#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_utils.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "content/public/common/dips_utils.h"

SystemInfoHandler::SystemInfoHandler(protocol::UberDispatcher* dispatcher) {
protocol::SystemInfo::Dispatcher::wire(dispatcher, this);
Expand All @@ -19,10 +19,10 @@ protocol::Response SystemInfoHandler::GetFeatureState(
const std::string& in_featureState,
bool* featureEnabled) {
if (in_featureState == "DIPS") {
*featureEnabled =
base::FeatureList::IsEnabled(dips::kFeature) &&
dips::kDeletionEnabled.Get() &&
(dips::kTriggeringAction.Get() != DIPSTriggeringAction::kNone);
*featureEnabled = base::FeatureList::IsEnabled(features::kDIPS) &&
features::kDIPSDeletionEnabled.Get() &&
(features::kDIPSTriggeringAction.Get() !=
content::DIPSTriggeringAction::kNone);
return protocol::Response::Success();
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/dips/dips_bounce_detector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "chrome/browser/3pcd/heuristics/opener_heuristic_tab_helper.h"
#include "chrome/browser/3pcd/heuristics/opener_heuristic_utils.h"
#include "chrome/browser/dips/cookie_access_filter.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_redirect_info.h"
#include "chrome/browser/dips/dips_service.h"
#include "chrome/browser/dips/dips_storage.h"
Expand All @@ -38,6 +37,7 @@
#include "content/public/browser/page.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/content_features.h"
#include "net/cookies/canonical_cookie.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
Expand Down Expand Up @@ -126,7 +126,7 @@ DIPSBounceDetector::DIPSBounceDetector(DIPSBounceDetectorDelegate* delegate,
/*redirect_prefix_count=*/0u),
client_bounce_detection_timer_(
FROM_HERE,
dips::kClientBounceDetectionTimeout.Get(),
features::kDIPSClientBounceDetectionTimeout.Get(),
base::BindRepeating(
&DIPSBounceDetector::OnClientBounceDetectionTimeout,
base::Unretained(this)),
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/dips/dips_bounce_detector.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "base/timer/timer.h"
#include "base/types/optional_ref.h"
#include "chrome/browser/dips/cookie_access_filter.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_redirect_info.h"
#include "chrome/browser/dips/dips_service.h"
#include "chrome/browser/dips/dips_utils.h"
Expand Down
24 changes: 14 additions & 10 deletions chrome/browser/dips/dips_bounce_detector_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
#include "base/test/task_environment.h"
#include "base/time/time.h"
#include "base/types/pass_key.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_service.h"
#include "chrome/browser/dips/dips_test_utils.h"
#include "chrome/browser/dips/dips_utils.h"
#include "components/ukm/test_ukm_recorder.h"
#include "content/public/common/content_features.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -273,10 +273,10 @@ class DIPSBounceDetectorTest : public ::testing::Test {
task_environment_.RunUntilIdle();
}

// Advances the mocked clock by `dips::kClientBounceDetectionTimeout` to
// trigger the closure of the pending redirect chain.
// Advances the mocked clock by `features::kDIPSClientBounceDetectionTimeout`
// to trigger the closure of the pending redirect chain.
void EndPendingRedirectChain() {
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get());
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get());
}

const std::string& URLForNavigationSourceId(ukm::SourceId source_id) {
Expand Down Expand Up @@ -340,13 +340,15 @@ TEST_F(DIPSBounceDetectorTest,
.RedirectTo("http://c.test")
.RedirectTo("http://d.test")
.Finish(true);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get() - base::Seconds(1));
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get() -
base::Seconds(1));
auto mocked_bounce_time_2 = GetCurrentTime();
StartNavigation("http://e.test", kNoUserGesture)
.RedirectTo("http://f.test")
.RedirectTo("http://g.test")
.Finish(true);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get() - base::Seconds(1));
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get() -
base::Seconds(1));
auto mocked_bounce_time_3 = GetCurrentTime();
StartNavigation("http://h.test", kWithUserGesture)
.RedirectTo("http://i.test")
Expand Down Expand Up @@ -389,13 +391,13 @@ TEST_F(DIPSBounceDetectorTest,
TEST_F(DIPSBounceDetectorTest,
DetectStatefulRedirects_After_ClientBounceDetectionTimeout) {
NavigateTo("http://a.test", kWithUserGesture);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get());
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get());
auto mocked_bounce_time_1 = GetCurrentTime();
StartNavigation("http://b.test", kWithUserGesture)
.RedirectTo("http://c.test")
.RedirectTo("http://d.test")
.Finish(true);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get());
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get());
auto mocked_bounce_time_2 = GetCurrentTime();
StartNavigation("http://e.test", kNoUserGesture)
.RedirectTo("http://f.test")
Expand Down Expand Up @@ -526,7 +528,8 @@ TEST_F(DIPSBounceDetectorTest, DetectStatefulRedirect_Server_LateNotification) {
TEST_F(DIPSBounceDetectorTest, DetectStatefulRedirect_Client) {
NavigateTo("http://a.test", kWithUserGesture);
NavigateTo("http://b.test", kWithUserGesture);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get() - base::Seconds(1));
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get() -
base::Seconds(1));
NavigateTo("http://c.test", kNoUserGesture);

auto mocked_bounce_time = GetCurrentTime();
Expand All @@ -545,7 +548,8 @@ TEST_F(DIPSBounceDetectorTest, DetectStatefulRedirect_Client_OnStartUp) {
NavigateTo("http://a.test", kWithUserGesture);
AccessClientCookie(CookieOperation::kRead);
AccessClientCookie(CookieOperation::kChange);
AdvanceDIPSTime(dips::kClientBounceDetectionTimeout.Get() - base::Seconds(1));
AdvanceDIPSTime(features::kDIPSClientBounceDetectionTimeout.Get() -
base::Seconds(1));
NavigateTo("http://b.test", kNoUserGesture);

auto mocked_bounce_time = GetCurrentTime();
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/dips/dips_browser_signin_detector_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "base/test/mock_callback.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_file_util.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_service.h"
#include "chrome/browser/dips/dips_test_utils.h"
#include "chrome/browser/dips/dips_utils.h"
Expand All @@ -31,6 +30,7 @@
#include "components/signin/public/identity_manager/account_managed_status_finder.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_task_environment.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
Expand Down Expand Up @@ -125,7 +125,7 @@ class BrowserSigninDetectorServiceTest : public testing::Test {
base::StrCat({"foo@", kIdentityProviderDomain}), kIdentityProviderDomain};

private:
ScopedInitFeature feature_{dips::kFeature,
ScopedInitFeature feature_{features::kDIPS,
/*enable:*/ true,
/*params:*/ {{"persist_database", "true"}}};
network::TestURLLoaderFactory test_url_loader_factory_;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/dips/dips_cleanup_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
#include "chrome/browser/dips/dips_cleanup_service.h"

#include "chrome/browser/dips/dips_cleanup_service_factory.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_storage.h"
#include "content/public/common/content_features.h"

DIPSCleanupService::DIPSCleanupService(content::BrowserContext* context) {
DCHECK(!base::FeatureList::IsEnabled(dips::kFeature));
DCHECK(!base::FeatureList::IsEnabled(features::kDIPS));
DIPSStorage::DeleteDatabaseFiles(
GetDIPSFilePath(context),
base::BindOnce(&DIPSCleanupService::OnCleanupFinished,
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/dips/dips_cleanup_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#include "base/no_destructor.h"
#include "chrome/browser/dips/dips_cleanup_service.h"
#include "chrome/browser/dips/dips_features.h"
#include "content/public/common/content_features.h"

// static
DIPSCleanupService* DIPSCleanupServiceFactory::GetForBrowserContext(
Expand All @@ -22,7 +22,7 @@ DIPSCleanupServiceFactory* DIPSCleanupServiceFactory::GetInstance() {

/*static*/
ProfileSelections DIPSCleanupServiceFactory::CreateProfileSelections() {
if (!base::FeatureList::IsEnabled(dips::kFeature)) {
if (!base::FeatureList::IsEnabled(features::kDIPS)) {
return ProfileSelections::Builder()
.WithRegular(ProfileSelection::kOriginalOnly)
.WithGuest(ProfileSelection::kNone)
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/dips/dips_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/threading/thread_restrictions.h"
#include "base/time/time.h"
#include "chrome/browser/dips/dips_utils.h"
#include "content/public/common/content_features.h"
#include "sql/database.h"
#include "sql/error_delegate_util.h"
#include "sql/init_status.h"
Expand Down Expand Up @@ -100,7 +101,7 @@ DIPSDatabase::DIPSDatabase(const absl::optional<base::FilePath>& db_path)
sql::DatabaseOptions{.exclusive_locking = true,
.page_size = 4096,
.cache_size = 32})) {
DCHECK(base::FeatureList::IsEnabled(dips::kFeature));
DCHECK(base::FeatureList::IsEnabled(features::kDIPS));
base::AssertLongCPUWorkAllowed();
if (db_path.has_value()) {
DCHECK(!db_path->empty())
Expand Down Expand Up @@ -887,7 +888,8 @@ size_t DIPSDatabase::ClearExpiredRows() {
DCHECK(db_->IsSQLValid(kClearAllExpiredBouncesTableSql));
sql::Statement bounces_statement(
db_->GetCachedStatement(SQL_FROM_HERE, kClearAllExpiredBouncesTableSql));
bounces_statement.BindTime(0, clock_->Now() - dips::kInteractionTtl.Get());
bounces_statement.BindTime(
0, clock_->Now() - features::kDIPSInteractionTtl.Get());
if (!bounces_statement.Run()) {
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/dips/dips_database.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

#include "base/time/clock.h"
#include "base/time/default_clock.h"
#include "chrome/browser/dips/dips_features.h"
#include "chrome/browser/dips/dips_utils.h"
#include "content/public/common/content_features.h"
#include "sql/database.h"
#include "sql/init_status.h"
#include "sql/meta_table.h"
Expand Down Expand Up @@ -74,7 +74,7 @@ class DIPSDatabase {
// This is implicitly `inline`. Don't move its definition to the .cc file.
bool HasExpired(absl::optional<base::Time> time) {
return time.has_value() &&
(time.value() + dips::kInteractionTtl.Get()) < clock_->Now();
(time.value() + features::kDIPSInteractionTtl.Get()) < clock_->Now();
}

absl::optional<StateValue> Read(const std::string& site);
Expand Down

0 comments on commit b601bb0

Please sign in to comment.