Skip to content

Commit

Permalink
Clean up kMarkHttpAs experiment logic
Browse files Browse the repository at this point in the history
Bug: 1015626
Change-Id: I92d5c225677760bf0a5dc4c3c1381db05dd36697
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552626
Commit-Queue: Livvie Lin <livvielin@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Chris Thompson <cthomp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833366}
  • Loading branch information
livvielin authored and Chromium LUCI CQ committed Dec 3, 2020
1 parent 2e0f3ed commit 8a40489
Show file tree
Hide file tree
Showing 14 changed files with 16 additions and 518 deletions.
27 changes: 0 additions & 27 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1484,17 +1484,6 @@ const FeatureEntry::FeatureVariation kOmniboxBubbleUrlSuggestionsVariations[] =
nullptr,
}};

const FeatureEntry::FeatureParam kMarkHttpAsDangerous[] = {
{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterDangerous}};
const FeatureEntry::FeatureParam kMarkHttpAsWarningAndDangerousOnFormEdits[] = {
{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::
kMarkHttpAsParameterWarningAndDangerousOnFormEdits}};
const FeatureEntry::FeatureParam kMarkHttpAsDangerWarning[] = {
{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterDangerWarning}};

// The "Enabled" state for this feature is "0" and representing setting A.
const FeatureEntry::FeatureParam kTabHoverCardsSettingB[] = {
{features::kTabHoverCardsFeatureParameterName, "1"}};
Expand All @@ -1505,15 +1494,6 @@ const FeatureEntry::FeatureVariation kTabHoverCardsFeatureVariations[] = {
{"B", kTabHoverCardsSettingB, base::size(kTabHoverCardsSettingB), nullptr},
{"C", kTabHoverCardsSettingC, base::size(kTabHoverCardsSettingC), nullptr}};

const FeatureEntry::FeatureVariation kMarkHttpAsFeatureVariations[] = {
{"(mark as actively dangerous)", kMarkHttpAsDangerous,
base::size(kMarkHttpAsDangerous), nullptr},
{"(mark with a Not Secure warning and dangerous on form edits)",
kMarkHttpAsWarningAndDangerousOnFormEdits,
base::size(kMarkHttpAsWarningAndDangerousOnFormEdits), nullptr},
{"(mark with a grey triangle icon)", kMarkHttpAsDangerWarning,
base::size(kMarkHttpAsDangerWarning), nullptr}};

const FeatureEntry::FeatureParam kPromoBrowserCommandUnknownCommandParam[] = {
{features::kPromoBrowserCommandIdParam, "0"}};
const FeatureEntry::FeatureParam
Expand Down Expand Up @@ -4609,13 +4589,6 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kEnableNetworkLoggingToFileDescription, kOsAll,
SINGLE_VALUE_TYPE(network::switches::kLogNetLog)},

{"enable-mark-http-as", flag_descriptions::kMarkHttpAsName,
flag_descriptions::kMarkHttpAsDescription, kOsAll,
FEATURE_WITH_PARAMS_VALUE_TYPE(
security_state::features::kMarkHttpAsFeature,
kMarkHttpAsFeatureVariations,
"HTTPReallyBadFinal")},

{"enable-web-authentication-cable-v2-support",
flag_descriptions::kEnableWebAuthenticationCableV2SupportName,
flag_descriptions::kEnableWebAuthenticationCableV2SupportDescription,
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1377,9 +1377,6 @@ const char kLogJsConsoleMessagesDescription[] =
"Enable logging JS console messages in system logs, please note that they "
"may contain PII.";

const char kMarkHttpAsName[] = "Mark non-secure origins as non-secure";
const char kMarkHttpAsDescription[] = "Change the UI treatment for HTTP pages";

const char kMediaHistoryName[] = "Enable Media History";
const char kMediaHistoryDescription[] =
"Enables Media History which records data around media playbacks on "
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -814,12 +814,6 @@ extern const char kLoadMediaRouterComponentExtensionDescription[];
extern const char kLogJsConsoleMessagesName[];
extern const char kLogJsConsoleMessagesDescription[];

extern const char kMarkHttpAsName[];
extern const char kMarkHttpAsDescription[];
extern const char kMarkHttpAsDangerous[];
extern const char kMarkHttpAsWarning[];
extern const char kMarkHttpAsWarningAndDangerousOnFormEdits[];

extern const char kMediaHistoryName[];
extern const char kMediaHistoryDescription[];

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ const base::Feature* kFeaturesExposedToJava[] = {
&reading_list::switches::kReadLater,
&safe_browsing::kEnhancedProtection,
&safe_browsing::kSafeBrowsingSectionUIAndroid,
&security_state::features::kMarkHttpAsFeature,
&signin::kMobileIdentityConsistency,
&signin::kMobileIdentityConsistencyVar,
&switches::kDecoupleSyncFromAndroidMasterSync,
Expand Down
43 changes: 5 additions & 38 deletions chrome/browser/secure_origin_allowlist_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,40 +166,7 @@ IN_PROC_BROWSER_TEST_P(SecureOriginAllowlistBrowsertest, Simple) {
}
}

class SecureOriginAllowlistBrowsertestWithMarkHttpDangerous
: public SecureOriginAllowlistBrowsertest {
public:
SecureOriginAllowlistBrowsertestWithMarkHttpDangerous() {
// TODO(crbug.com/917693): Remove this forced feature/param when the feature
// fully launches.
feature_list_.InitAndEnableFeatureWithParameters(
security_state::features::kMarkHttpAsFeature,
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterDangerous}});
}

private:
base::test::ScopedFeatureList feature_list_;
};

INSTANTIATE_TEST_SUITE_P(All,
SecureOriginAllowlistBrowsertestWithMarkHttpDangerous,
testing::Values(TestVariant::kNone,
TestVariant::kCommandline,
// The legacy policy isn't defined on ChromeOS or Android, so skip tests that
// use it on those platforms.
#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
TestVariant::kPolicyOld,
TestVariant::kPolicyOldAndNew,
#endif
TestVariant::kPolicy,
TestVariant::kPolicy2,
TestVariant::kPolicy3));

// Tests that allowlisted insecure origins are correctly set as security level
// NONE instead of the default level DANGEROUS.
IN_PROC_BROWSER_TEST_P(SecureOriginAllowlistBrowsertestWithMarkHttpDangerous,
SecurityIndicators) {
IN_PROC_BROWSER_TEST_P(SecureOriginAllowlistBrowsertest, SecurityIndicators) {
ui_test_utils::NavigateToURL(
browser(),
embedded_test_server()->GetURL(
Expand All @@ -210,15 +177,15 @@ IN_PROC_BROWSER_TEST_P(SecureOriginAllowlistBrowsertestWithMarkHttpDangerous,

if (GetParam() == TestVariant::kPolicyOldAndNew) {
// When both policies are set, the new policy overrides the old policy.
EXPECT_EQ(security_state::DANGEROUS, helper->GetSecurityLevel());
EXPECT_EQ(security_state::WARNING, helper->GetSecurityLevel());
ui_test_utils::NavigateToURL(
browser(),
embedded_test_server()->GetURL(
"otherexample.com", "/secure_origin_allowlist_browsertest.html"));
EXPECT_EQ(security_state::NONE, helper->GetSecurityLevel());
} else {
EXPECT_EQ(ExpectSecureContext() ? security_state::NONE
: security_state::DANGEROUS,
helper->GetSecurityLevel());
EXPECT_EQ(
ExpectSecureContext() ? security_state::NONE : security_state::WARNING,
helper->GetSecurityLevel());
}
}
Loading

0 comments on commit 8a40489

Please sign in to comment.