Commit f529d2a4 authored by Dominique Fauteux-Chapleau's avatar Dominique Fauteux-Chapleau Committed by Chromium LUCI CQ

Reland "Add testing config for per-profile Connectors"

This is a reland of crrev.com/c/2623812 that was reverted by
crrev.com/c/2628041. The issue was that enabling per-profile in
unrelated tests made for a bad combination with realtime URL checks
(that landed in crrev.com/c/2615779) and resulted in the browser DM
token reading code being used needlessly. The fix to this is to add an
optimization to ConnectorsService::GetDMTokenForRealTimeUrlCheck and
check the main policy value before calling the expensive DM token
reading code. Aside from that change this reland is identical to
crrev.com/c/2623812.

Original change's description:
> Add testing config for per-profile Connectors
>
> A small change in ConnectorsService is required to make this test config
> work. When checking analysis/reporting settings, 2 things are required
> to obtain settings: a policy set to a valid value and a DM token (from
> the browser or the profile). The previous implementation checked the DM
> token first, meaning the blocking BrowserDMTokenStorage::RetrieveDMToken
> call would trigger on multiple tests and crash them (see patchset 1
> failures for examples of this).
>
> The solution is to simply switch the
> order of checks and read the policy first. This is also a valid
> optimization of Connectors logic, as reading prefs is faster than
> potentially reading the DM token from local storage.
>
> Change-Id: Ie1026b57cf7a4fd66663530ec2a77a72f21d6d81
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623812
> Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
> Reviewed-by: Marc-André Decoste <mad@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#843115}

Change-Id: Ie2959239571905251081c39af7cc4b124863890d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628110
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: default avatarXinghui Lu <xinghuilu@chromium.org>
Reviewed-by: default avatarMarc-André Decoste <mad@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843568}
parent 6bf15621
......@@ -128,17 +128,18 @@ base::Optional<ReportingSettings> ConnectorsService::GetReportingSettings(
if (!ConnectorsEnabled())
return base::nullopt;
base::Optional<ReportingSettings> settings =
connectors_manager_->GetReportingSettings(connector);
if (!settings.has_value())
return base::nullopt;
base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
if (!dm_token.has_value())
return base::nullopt;
base::Optional<ReportingSettings> settings =
connectors_manager_->GetReportingSettings(connector);
if (settings.has_value()) {
settings.value().dm_token = dm_token.value().value;
settings.value().per_profile =
dm_token.value().scope == policy::POLICY_SCOPE_USER;
}
settings.value().dm_token = dm_token.value().value;
settings.value().per_profile =
dm_token.value().scope == policy::POLICY_SCOPE_USER;
return settings;
}
......@@ -149,15 +150,16 @@ base::Optional<AnalysisSettings> ConnectorsService::GetAnalysisSettings(
if (!ConnectorsEnabled())
return base::nullopt;
base::Optional<AnalysisSettings> settings =
connectors_manager_->GetAnalysisSettings(url, connector);
if (!settings.has_value())
return base::nullopt;
base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
if (!dm_token.has_value())
return base::nullopt;
base::Optional<AnalysisSettings> settings =
connectors_manager_->GetAnalysisSettings(url, connector);
if (settings.has_value()) {
settings.value().dm_token = dm_token.value().value;
}
settings.value().dm_token = dm_token.value().value;
return settings;
}
......@@ -188,6 +190,12 @@ base::Optional<std::string> ConnectorsService::GetDMTokenForRealTimeUrlCheck()
if (!ConnectorsEnabled())
return base::nullopt;
if (Profile::FromBrowserContext(context_)->GetPrefs()->GetInteger(
prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckMode) ==
safe_browsing::REAL_TIME_CHECK_DISABLED) {
return base::nullopt;
}
base::Optional<DmToken> dm_token =
GetDmToken(prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckScope);
......
......@@ -185,14 +185,10 @@ INSTANTIATE_TEST_CASE_P(
testing::Bool(),
testing::ValuesIn({0, 1, 2})));
class ConnectorsServiceRealtimeURLCheckTest : public ConnectorsServiceTest {
public:
ConnectorsServiceRealtimeURLCheckTest() {
scoped_feature_list_.InitWithFeatures({kEnterpriseConnectorsEnabled}, {});
}
};
TEST_F(ConnectorsServiceTest, RealtimeURLCheck) {
profile_->GetPrefs()->SetInteger(
prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckMode,
safe_browsing::REAL_TIME_CHECK_FOR_MAINFRAME_ENABLED);
profile_->GetPrefs()->SetInteger(
prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckScope,
policy::POLICY_SCOPE_MACHINE);
......
......@@ -66,6 +66,9 @@ class ChromeEnterpriseRealTimeUrlLookupServiceTest : public PlatformTest {
/*is_under_advanced_protection=*/true,
/*is_off_the_record=*/false);
test_profile_->GetPrefs()->SetInteger(
prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckMode,
REAL_TIME_CHECK_FOR_MAINFRAME_ENABLED);
test_profile_->GetPrefs()->SetInteger(
prefs::kSafeBrowsingEnterpriseRealTimeUrlCheckScope,
policy::POLICY_SCOPE_MACHINE);
......
......@@ -7986,7 +7986,7 @@
]
}
],
"WebProtectConnectors": [
"WebProtectPerProfile": [
{
"platforms": [
"chromeos",
......@@ -7998,7 +7998,7 @@
{
"name": "Enabled",
"enable_features": [
"EnterpriseConnectorsEnabled"
"PerProfileConnectorsEnabled"
]
}
]
......
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