Add OTR status to SupportsPlusAddresses function
This is the first of two CLs, in an attempt to minimize review
time for the calling team. This is purely an interface change;
the next round will involve actually reacting to the new param.
Change-Id: Ia01dc6ae4fc927326a731caa87877814c932702e
Bug: 308988053
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5002690
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Kirubel Aklilu <kaklilu@chromium.org>
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1219620}
diff --git a/chrome/browser/plus_addresses/plus_address_browsertest.cc b/chrome/browser/plus_addresses/plus_address_browsertest.cc
index 1219465..a336ffd 100644
--- a/chrome/browser/plus_addresses/plus_address_browsertest.cc
+++ b/chrome/browser/plus_addresses/plus_address_browsertest.cc
@@ -61,5 +61,6 @@
GetActiveWebContents()->GetBrowserContext());
EXPECT_NE(plus_address_service, nullptr);
EXPECT_TRUE(plus_address_service->SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
diff --git a/components/autofill/core/browser/browser_autofill_manager.cc b/components/autofill/core/browser/browser_autofill_manager.cc
index 2788df4..1c0ef59 100644
--- a/components/autofill/core/browser/browser_autofill_manager.cc
+++ b/components/autofill/core/browser/browser_autofill_manager.cc
@@ -3808,7 +3808,8 @@
client().GetPlusAddressService();
if (!plus_address_service ||
!plus_address_service->SupportsPlusAddresses(
- client().GetLastCommittedPrimaryMainFrameOrigin())) {
+ client().GetLastCommittedPrimaryMainFrameOrigin(),
+ client().IsOffTheRecord())) {
return absl::nullopt;
}
absl::optional<std::string> maybe_address =
diff --git a/components/autofill/core/browser/browser_autofill_manager_unittest.cc b/components/autofill/core/browser/browser_autofill_manager_unittest.cc
index 6715492f..89c369a 100644
--- a/components/autofill/core/browser/browser_autofill_manager_unittest.cc
+++ b/components/autofill/core/browser/browser_autofill_manager_unittest.cc
@@ -253,7 +253,10 @@
// and does not trigger any UI elements.
class MockPlusAddressService : public plus_addresses::PlusAddressService {
public:
- MOCK_METHOD(bool, SupportsPlusAddresses, (url::Origin), (override));
+ MOCK_METHOD(bool,
+ SupportsPlusAddresses,
+ (url::Origin, bool is_off_the_record),
+ (override));
};
class MockAutofillClient : public TestAutofillClient {
diff --git a/components/plus_addresses/plus_address_service.cc b/components/plus_addresses/plus_address_service.cc
index c6ee57f..70a421ee 100644
--- a/components/plus_addresses/plus_address_service.cc
+++ b/components/plus_addresses/plus_address_service.cc
@@ -58,8 +58,11 @@
}
}
-bool PlusAddressService::SupportsPlusAddresses(url::Origin origin) {
+bool PlusAddressService::SupportsPlusAddresses(url::Origin origin,
+ bool is_off_the_record) {
// TODO(b/295187452): Also check `origin` here.
+ // TODO(b/308988053): implement OTR behavior. Only existing can be offered in
+ // that mode.
return is_enabled();
}
diff --git a/components/plus_addresses/plus_address_service.h b/components/plus_addresses/plus_address_service.h
index 332d2b25..87908ac 100644
--- a/components/plus_addresses/plus_address_service.h
+++ b/components/plus_addresses/plus_address_service.h
@@ -51,7 +51,8 @@
// Virtual to allow overriding the behavior in tests. This allows external
// tests (e.g., those in autofill that depend on this class) to substitute
// their own behavior.
- virtual bool SupportsPlusAddresses(url::Origin origin);
+ virtual bool SupportsPlusAddresses(url::Origin origin,
+ bool is_off_the_record);
// Get a plus address, if one exists, for the passed-in origin. Note that all
// plus address activity is scoped to eTLD+1. This class owns the conversion
// of `origin` to its eTLD+1 form.
diff --git a/components/plus_addresses/plus_address_service_unittest.cc b/components/plus_addresses/plus_address_service_unittest.cc
index 483522a..a1cfc10 100644
--- a/components/plus_addresses/plus_address_service_unittest.cc
+++ b/components/plus_addresses/plus_address_service_unittest.cc
@@ -81,7 +81,8 @@
// By default, the `SupportsPlusAddresses` function should return `false`.
PlusAddressService service;
EXPECT_FALSE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
// Ensure `SupportsPlusAddresses` is false without a server URL.
@@ -91,7 +92,8 @@
base::test::ScopedFeatureList scoped_feature_list{plus_addresses::kFeature};
PlusAddressService service;
EXPECT_FALSE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
// Tests for the label overrides. These tests are not in the enabled/disabled
@@ -655,7 +657,8 @@
{signin::ConsentLevel::kSignin});
PlusAddressService service(identity_test_env.identity_manager());
EXPECT_FALSE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
TEST_F(PlusAddressServiceDisabledTest, DisabledFeatureLabel) {
@@ -682,7 +685,8 @@
// `false`.
PlusAddressService service;
EXPECT_FALSE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
TEST_F(PlusAddressServiceEnabledTest, NoSignedInUser) {
@@ -691,7 +695,8 @@
signin::IdentityTestEnvironment identity_test_env;
PlusAddressService service(identity_test_env.identity_manager());
EXPECT_FALSE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
TEST_F(PlusAddressServiceEnabledTest, FullySupported) {
@@ -702,7 +707,8 @@
{signin::ConsentLevel::kSignin});
PlusAddressService service(identity_test_env.identity_manager());
EXPECT_TRUE(service.SupportsPlusAddresses(
- url::Origin::Create(GURL("https://test.example"))));
+ url::Origin::Create(GURL("https://test.example")),
+ /*is_off_the_record=*/false));
}
TEST_F(PlusAddressServiceEnabledTest, DefaultLabel) {
@@ -775,7 +781,7 @@
// Verify behaviors expected when service is enabled.
url::Origin site = url::Origin::Create(GURL("https://foo.com"));
service.SavePlusAddress(site, "plus@plus.plus");
- EXPECT_TRUE(service.SupportsPlusAddresses(site));
+ EXPECT_TRUE(service.SupportsPlusAddresses(site, /*is_off_the_record=*/false));
EXPECT_TRUE(service.GetPlusAddress(site));
EXPECT_EQ(service.GetPlusAddress(site).value(), "plus@plus.plus");
EXPECT_TRUE(service.IsPlusAddress("plus@plus.plus"));
@@ -784,7 +790,8 @@
EXPECT_FALSE(service.is_enabled());
// Ensure that the local data is cleared on disabling.
- EXPECT_FALSE(service.SupportsPlusAddresses(site));
+ EXPECT_FALSE(
+ service.SupportsPlusAddresses(site, /*is_off_the_record=*/false));
EXPECT_FALSE(service.IsPlusAddress("plus@plus.plus"));
}
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)
@@ -797,7 +804,7 @@
// Verify behaviors expected when service is enabled.
url::Origin site = url::Origin::Create(GURL("https://foo.com"));
service.SavePlusAddress(site, "plus@plus.plus");
- EXPECT_TRUE(service.SupportsPlusAddresses(site));
+ EXPECT_TRUE(service.SupportsPlusAddresses(site, /*is_off_the_record=*/false));
EXPECT_TRUE(service.GetPlusAddress(site));
EXPECT_EQ(service.GetPlusAddress(site).value(), "plus@plus.plus");
EXPECT_TRUE(service.IsPlusAddress("plus@plus.plus"));
@@ -821,7 +828,8 @@
EXPECT_FALSE(service.is_enabled());
// Ensure that the local data is cleared on disabling.
- EXPECT_FALSE(service.SupportsPlusAddresses(site));
+ EXPECT_FALSE(
+ service.SupportsPlusAddresses(site, /*is_off_the_record=*/false));
EXPECT_FALSE(service.IsPlusAddress("plus@plus.plus"));
}
