Commit 238cefd0 authored by Livvie Lin's avatar Livvie Lin Committed by Commit Bot

Re-use security_state::ShouldDowngradeNeutralStyling in Android code

Bug: 1008219
Change-Id: I88472d3884c9c9e8495662c366347fd2f775c3d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1927373
Commit-Queue: Livvie Lin <livvielin@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#717760}
parent 76c21c0b
...@@ -121,7 +121,6 @@ public class FeatureUtilities { ...@@ -121,7 +121,6 @@ public class FeatureUtilities {
cacheSwapPixelFormatToFixConvertFromTranslucentEnabled(); cacheSwapPixelFormatToFixConvertFromTranslucentEnabled();
cacheReachedCodeProfilerTrialGroup(); cacheReachedCodeProfilerTrialGroup();
cacheStartSurfaceEnabled(); cacheStartSurfaceEnabled();
cacheMarkHttpAsDangerWarningEnabled();
if (isHighEndPhone()) cacheGridTabSwitcherEnabled(); if (isHighEndPhone()) cacheGridTabSwitcherEnabled();
if (isHighEndPhone()) cacheTabGroupsAndroidEnabled(); if (isHighEndPhone()) cacheTabGroupsAndroidEnabled();
...@@ -556,36 +555,6 @@ public class FeatureUtilities { ...@@ -556,36 +555,6 @@ public class FeatureUtilities {
ChromeFeatureList.SWAP_PIXEL_FORMAT_TO_FIX_CONVERT_FROM_TRANSLUCENT); ChromeFeatureList.SWAP_PIXEL_FORMAT_TO_FIX_CONVERT_FROM_TRANSLUCENT);
} }
/**
* Cache the value of the flag of whether to use the grey triangle security indicator on
* insecure pages.
*/
public static void cacheMarkHttpAsDangerWarningEnabled() {
String featureParam = "danger-warning";
String enabledFeature = ChromeFeatureList.getFieldTrialParamByFeature(
ChromeFeatureList.MARK_HTTP_AS, "treatment");
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.MARK_HTTP_AS_DANGER_WARNING_KEY,
enabledFeature.equals(featureParam));
}
/**
* @return Whether a grey triangle icon should be used in the omnibox instead of the info icon
* on insecure pages.
*/
public static boolean isMarkHttpAsDangerWarningEnabled() {
return isFlagEnabled(ChromePreferenceKeys.MARK_HTTP_AS_DANGER_WARNING_KEY, false);
}
/**
* Toggles whether experiment for marking insecure connections with a grey triangle
* icon is enabled for testing. Should be reset back to null after the test has finished.
*/
@VisibleForTesting
public static void setMarkHttpAsDangerWarningEnabledForTesting(@Nullable Boolean isEnabled) {
sFlags.put(ChromePreferenceKeys.MARK_HTTP_AS_DANGER_WARNING_KEY, isEnabled);
}
/** /**
* Caches the trial group of the reached code profiler feature to be using on next startup. * Caches the trial group of the reached code profiler feature to be using on next startup.
*/ */
......
...@@ -26,24 +26,24 @@ public class SecurityStateModel { ...@@ -26,24 +26,24 @@ public class SecurityStateModel {
} }
/** /**
* Returns true for a valid URL from a secure origin, e.g., http://localhost, * Returns true for a valid URL with a cryptographic scheme, e.g., HTTPS, WSS.
* file:///home/user/test.html, https://bobpay.com.
* *
* @param url The URL to check. * @param url The URL to check.
* @return Whether the origin of the URL is secure. * @return Whether the scheme of the URL is cryptographic.
*/ */
public static boolean isOriginSecure(String url) { public static boolean isSchemeCryptographic(String url) {
return SecurityStateModelJni.get().isOriginSecure(url); return SecurityStateModelJni.get().isSchemeCryptographic(url);
} }
/** /**
* Returns true for a valid URL with a cryptographic scheme, e.g., HTTPS, WSS. * Returns whether to use a danger icon instead of an info icon in the URL bar.
* *
* @param securityLevel The ConnectionSecurityLevel of the page.
* @param url The URL to check. * @param url The URL to check.
* @return Whether the scheme of the URL is cryptographic. * @return Whether to downgrade the info icon to a danger triangle.
*/ */
public static boolean isSchemeCryptographic(String url) { public static boolean shouldDowngradeNeutralStyling(int securityLevel, String url) {
return SecurityStateModelJni.get().isSchemeCryptographic(url); return SecurityStateModelJni.get().shouldDowngradeNeutralStyling(securityLevel, url);
} }
private SecurityStateModel() {} private SecurityStateModel() {}
...@@ -51,7 +51,7 @@ public class SecurityStateModel { ...@@ -51,7 +51,7 @@ public class SecurityStateModel {
@NativeMethods @NativeMethods
interface Natives { interface Natives {
int getSecurityLevelForWebContents(WebContents webContents); int getSecurityLevelForWebContents(WebContents webContents);
boolean isOriginSecure(String url);
boolean isSchemeCryptographic(String url); boolean isSchemeCryptographic(String url);
boolean shouldDowngradeNeutralStyling(int securityLevel, String url);
} }
} }
...@@ -20,7 +20,6 @@ import org.chromium.chrome.browser.ChromeFeatureList; ...@@ -20,7 +20,6 @@ import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior; import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.dom_distiller.DomDistillerTabUtils; import org.chromium.chrome.browser.dom_distiller.DomDistillerTabUtils;
import org.chromium.chrome.browser.flags.FeatureUtilities;
import org.chromium.chrome.browser.native_page.NativePageFactory; import org.chromium.chrome.browser.native_page.NativePageFactory;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils; import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
...@@ -390,7 +389,6 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope ...@@ -390,7 +389,6 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope
return R.drawable.ic_offline_pin_24dp; return R.drawable.ic_offline_pin_24dp;
} }
boolean featureIsEnabled = FeatureUtilities.isMarkHttpAsDangerWarningEnabled();
String url = getCurrentUrl(); String url = getCurrentUrl();
switch (securityLevel) { switch (securityLevel) {
...@@ -399,8 +397,9 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope ...@@ -399,8 +397,9 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope
// is NONE, but the security indicator should be shown on all devices. // is NONE, but the security indicator should be shown on all devices.
if (mNativeLocationBarModelAndroid != 0 if (mNativeLocationBarModelAndroid != 0
&& SecurityStateModel.isSchemeCryptographic(url)) { && SecurityStateModel.isSchemeCryptographic(url)) {
return featureIsEnabled ? R.drawable.omnibox_not_secure_warning return SecurityStateModel.shouldDowngradeNeutralStyling(securityLevel, url)
: R.drawable.omnibox_info; ? R.drawable.omnibox_not_secure_warning
: R.drawable.omnibox_info;
} }
return isSmallDevice return isSmallDevice
&& (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito()) && (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito())
...@@ -411,7 +410,7 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope ...@@ -411,7 +410,7 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope
if (mNativeLocationBarModelAndroid == 0) { if (mNativeLocationBarModelAndroid == 0) {
return R.drawable.omnibox_info; return R.drawable.omnibox_info;
} }
if (featureIsEnabled && !SecurityStateModel.isOriginSecure(url)) { if (SecurityStateModel.shouldDowngradeNeutralStyling(securityLevel, url)) {
return R.drawable.omnibox_not_secure_warning; return R.drawable.omnibox_not_secure_warning;
} }
return R.drawable.omnibox_info; return R.drawable.omnibox_info;
......
...@@ -26,7 +26,6 @@ import org.robolectric.annotation.Resetter; ...@@ -26,7 +26,6 @@ import org.robolectric.annotation.Resetter;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker; import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.FeatureUtilities;
import org.chromium.chrome.browser.omnibox.LocationBarLayout; import org.chromium.chrome.browser.omnibox.LocationBarLayout;
import org.chromium.chrome.browser.ssl.SecurityStateModel; import org.chromium.chrome.browser.ssl.SecurityStateModel;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
...@@ -63,12 +62,12 @@ public final class ToolbarSecurityIconTest { ...@@ -63,12 +62,12 @@ public final class ToolbarSecurityIconTest {
@Implements(SecurityStateModel.class) @Implements(SecurityStateModel.class)
static class ShadowSecurityStateModel { static class ShadowSecurityStateModel {
private static boolean sSchemeIsCryptographic; private static boolean sSchemeIsCryptographic;
private static boolean sOriginIsSecure; private static boolean sShouldDowngrade;
@Resetter @Resetter
public static void reset() { public static void reset() {
sSchemeIsCryptographic = false; sSchemeIsCryptographic = false;
sOriginIsSecure = false; sShouldDowngrade = false;
} }
@Implementation @Implementation
...@@ -77,16 +76,16 @@ public final class ToolbarSecurityIconTest { ...@@ -77,16 +76,16 @@ public final class ToolbarSecurityIconTest {
} }
@Implementation @Implementation
public static boolean isOriginSecure(String url) { public static boolean shouldDowngradeNeutralStyling(int securityLevel, String url) {
return sOriginIsSecure; return sShouldDowngrade;
} }
public static void setSchemeIsCryptographic(boolean schemeIsCryptographicValue) { public static void setSchemeIsCryptographic(boolean schemeIsCryptographicValue) {
sSchemeIsCryptographic = schemeIsCryptographicValue; sSchemeIsCryptographic = schemeIsCryptographicValue;
} }
public static void setOriginIsSecure(boolean originIsSecureValue) { public static void setShouldDowngradeNeutralStyling(boolean shouldDowngradeValue) {
sOriginIsSecure = originIsSecureValue; sShouldDowngrade = shouldDowngradeValue;
} }
} }
...@@ -103,7 +102,6 @@ public final class ToolbarSecurityIconTest { ...@@ -103,7 +102,6 @@ public final class ToolbarSecurityIconTest {
@After @After
public void tearDown() { public void tearDown() {
ShadowSecurityStateModel.reset(); ShadowSecurityStateModel.reset();
FeatureUtilities.setMarkHttpAsDangerWarningEnabledForTesting(null);
} }
@Test @Test
...@@ -212,8 +210,6 @@ public final class ToolbarSecurityIconTest { ...@@ -212,8 +210,6 @@ public final class ToolbarSecurityIconTest {
@Test @Test
public void testGetSecurityIconResourceForMarkHttpAsDangerWarning() { public void testGetSecurityIconResourceForMarkHttpAsDangerWarning() {
FeatureUtilities.setMarkHttpAsDangerWarningEnabledForTesting(true);
assertEquals(0, assertEquals(0,
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
...@@ -221,8 +217,9 @@ public final class ToolbarSecurityIconTest { ...@@ -221,8 +217,9 @@ public final class ToolbarSecurityIconTest {
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
!IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
// Tests passive mixed content. // Tests passive mixed content, which should be downgraded.
ShadowSecurityStateModel.setSchemeIsCryptographic(true); ShadowSecurityStateModel.setSchemeIsCryptographic(true);
ShadowSecurityStateModel.setShouldDowngradeNeutralStyling(true);
assertEquals(R.drawable.omnibox_not_secure_warning, assertEquals(R.drawable.omnibox_not_secure_warning,
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
...@@ -230,8 +227,9 @@ public final class ToolbarSecurityIconTest { ...@@ -230,8 +227,9 @@ public final class ToolbarSecurityIconTest {
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
!IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
// Tests non-secure connection (e.g. HTTP). // Tests non-secure connection (e.g. HTTP), which should be downgraded.
ShadowSecurityStateModel.setSchemeIsCryptographic(false); ShadowSecurityStateModel.setSchemeIsCryptographic(false);
ShadowSecurityStateModel.setShouldDowngradeNeutralStyling(true);
assertEquals(R.drawable.omnibox_not_secure_warning, assertEquals(R.drawable.omnibox_not_secure_warning,
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
...@@ -239,8 +237,8 @@ public final class ToolbarSecurityIconTest { ...@@ -239,8 +237,8 @@ public final class ToolbarSecurityIconTest {
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
!IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
// Tests non-cryptographic secure origins. // Tests non-cryptographic secure origins, which should not be downgraded.
ShadowSecurityStateModel.setOriginIsSecure(true); ShadowSecurityStateModel.setShouldDowngradeNeutralStyling(false);
assertEquals(R.drawable.omnibox_info, assertEquals(R.drawable.omnibox_info,
mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING, mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW)); IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
......
...@@ -305,12 +305,6 @@ public final class ChromePreferenceKeys { ...@@ -305,12 +305,6 @@ public final class ChromePreferenceKeys {
*/ */
public static final String START_SURFACE_ENABLED_KEY = "start_surface_enabled"; public static final String START_SURFACE_ENABLED_KEY = "start_surface_enabled";
/**
* Whether or not grey triangle icon on non-secure connections feature is enabled.
* Default value is false.
*/
public static final String MARK_HTTP_AS_DANGER_WARNING_KEY = "mark_http_as_danger_warning";
/** /**
* Whether or not the grid tab switcher is enabled. * Whether or not the grid tab switcher is enabled.
* Default value is false. * Default value is false.
...@@ -420,7 +414,6 @@ public final class ChromePreferenceKeys { ...@@ -420,7 +414,6 @@ public final class ChromePreferenceKeys {
NIGHT_MODE_CCT_AVAILABLE_KEY, NIGHT_MODE_CCT_AVAILABLE_KEY,
COMMAND_LINE_ON_NON_ROOTED_ENABLED_KEY, COMMAND_LINE_ON_NON_ROOTED_ENABLED_KEY,
START_SURFACE_ENABLED_KEY, START_SURFACE_ENABLED_KEY,
MARK_HTTP_AS_DANGER_WARNING_KEY,
GRID_TAB_SWITCHER_ENABLED_KEY, GRID_TAB_SWITCHER_ENABLED_KEY,
TAB_GROUPS_ANDROID_ENABLED_KEY, TAB_GROUPS_ANDROID_ENABLED_KEY,
PRIORITIZE_BOOTSTRAP_TASKS_KEY, PRIORITIZE_BOOTSTRAP_TASKS_KEY,
...@@ -530,7 +523,6 @@ public final class ChromePreferenceKeys { ...@@ -530,7 +523,6 @@ public final class ChromePreferenceKeys {
NIGHT_MODE_CCT_AVAILABLE_KEY, NIGHT_MODE_CCT_AVAILABLE_KEY,
COMMAND_LINE_ON_NON_ROOTED_ENABLED_KEY, COMMAND_LINE_ON_NON_ROOTED_ENABLED_KEY,
START_SURFACE_ENABLED_KEY, START_SURFACE_ENABLED_KEY,
MARK_HTTP_AS_DANGER_WARNING_KEY,
GRID_TAB_SWITCHER_ENABLED_KEY, GRID_TAB_SWITCHER_ENABLED_KEY,
TAB_GROUPS_ANDROID_ENABLED_KEY, TAB_GROUPS_ANDROID_ENABLED_KEY,
PRIORITIZE_BOOTSTRAP_TASKS_KEY, PRIORITIZE_BOOTSTRAP_TASKS_KEY,
......
...@@ -30,17 +30,21 @@ jint JNI_SecurityStateModel_GetSecurityLevelForWebContents( ...@@ -30,17 +30,21 @@ jint JNI_SecurityStateModel_GetSecurityLevelForWebContents(
} }
// static // static
jboolean JNI_SecurityStateModel_IsOriginSecure( jboolean JNI_SecurityStateModel_IsSchemeCryptographic(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jstring>& jurl) { const JavaParamRef<jstring>& jurl) {
GURL url(ConvertJavaStringToUTF16(env, jurl)); GURL url(ConvertJavaStringToUTF16(env, jurl));
return url.is_valid() && content::IsOriginSecure(url); return url.is_valid() && url.SchemeIsCryptographic();
} }
// static // static
jboolean JNI_SecurityStateModel_IsSchemeCryptographic( jboolean JNI_SecurityStateModel_ShouldDowngradeNeutralStyling(
JNIEnv* env, JNIEnv* env,
jint jsecurity_level,
const JavaParamRef<jstring>& jurl) { const JavaParamRef<jstring>& jurl) {
GURL url(ConvertJavaStringToUTF16(env, jurl)); GURL url(ConvertJavaStringToUTF16(env, jurl));
return url.is_valid() && url.SchemeIsCryptographic(); auto security_level =
static_cast<security_state::SecurityLevel>(jsecurity_level);
return security_state::ShouldDowngradeNeutralStyling(
security_level, url, base::BindRepeating(&content::IsOriginSecure));
} }
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