Revert "Use grey triangle icon for HTTP when experiment enabled (Android)"

This reverts commit 4430c28a.

Reason for revert: https://crbug.com/1024954

Original change's description:
> Use grey triangle icon for HTTP when experiment enabled (Android)
> 
> Bug: 1008219
> Change-Id: If02a36539fe61e71d1434528ec5af31d3914439b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1889116
> Commit-Queue: Livvie Lin <livvielin@chromium.org>
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714377}

TBR=mthiesse@chromium.org,livvielin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1008219
Change-Id: I3f63150118c21da153c995529848fc92cd18bd52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918103Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Livvie Lin <livvielin@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715482}

chrome/android/java/src/org/chromium/chrome/browser/flags/FeatureUtilities.java

@@ -138,12 +138,6 @@ public class FeatureUtilities {
      */
     private 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.
-     */
-    private static final String MARK_HTTP_AS_DANGER_WARNING = "mark_http_as_danger_warning";
-
     /**
      * Whether or not the grid tab switcher is enabled.
      * Default value is false.
@@ -248,7 +242,6 @@ public class FeatureUtilities {
         cacheReachedCodeProfilerTrialGroup();
         cacheStartSurfaceEnabled();
         cacheClickToCallOpenDialerDirectlyEnabled();
-        cacheMarkHttpAsDangerWarningEnabled();
 
         if (isHighEndPhone()) cacheGridTabSwitcherEnabled();
         if (isHighEndPhone()) cacheTabGroupsAndroidEnabled();
@@ -695,35 +688,6 @@ public class FeatureUtilities {
         sFlags.put(CLICK_TO_CALL_OPEN_DIALER_DIRECTLY_KEY, isEnabled);
     }
 
-    /**
-     * 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(
-                MARK_HTTP_AS_DANGER_WARNING, 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(MARK_HTTP_AS_DANGER_WARNING, 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(MARK_HTTP_AS_DANGER_WARNING, isEnabled);
-    }
-
     /**
      * Caches the trial group of the reached code profiler feature to be using on next startup.
      */

chrome/android/java/src/org/chromium/chrome/browser/ssl/SecurityStateModel.java

@@ -25,33 +25,10 @@ public class SecurityStateModel {
         return SecurityStateModelJni.get().getSecurityLevelForWebContents(webContents);
     }
 
-    /**
-     * Returns true for a valid URL from a secure origin, e.g., http://localhost,
-     * file:///home/user/test.html, https://bobpay.com.
-     *
-     * @param url The URL to check.
-     * @return Whether the origin of the URL is secure.
-     */
-    public static boolean isOriginSecure(String url) {
-        return SecurityStateModelJni.get().isOriginSecure(url);
-    }
-
-    /**
-     * Returns true for a valid URL with a cryptographic scheme, e.g., HTTPS, WSS.
-     *
-     * @param url The URL to check.
-     * @return Whether the scheme of the URL is cryptographic.
-     */
-    public static boolean isSchemeCryptographic(String url) {
-        return SecurityStateModelJni.get().isSchemeCryptographic(url);
-    }
-
     private SecurityStateModel() {}
 
     @NativeMethods
     interface Natives {
         int getSecurityLevelForWebContents(WebContents webContents);
-        boolean isOriginSecure(String url);
-        boolean isSchemeCryptographic(String url);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java

@@ -20,7 +20,6 @@ import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
 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.ntp.NewTabPage;
 import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
@@ -29,7 +28,6 @@ import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils;
 import org.chromium.chrome.browser.omnibox.UrlBarData;
 import org.chromium.chrome.browser.previews.PreviewsAndroidBridge;
 import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.ssl.SecurityStateModel;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TrustedCdn;
 import org.chromium.chrome.browser.ui.styles.ChromeColors;
@@ -390,30 +388,14 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope
             return R.drawable.ic_offline_pin_24dp;
         }
 
-        boolean featureIsEnabled = FeatureUtilities.isMarkHttpAsDangerWarningEnabled();
-        String url = getCurrentUrl();
-
         switch (securityLevel) {
             case ConnectionSecurityLevel.NONE:
-                // For HTTPS sites with passive mixed content, ConnectionSecurityLevel
-                // is NONE, but the security indicator should be shown on all devices.
-                if (mNativeLocationBarModelAndroid != 0
-                        && SecurityStateModel.isSchemeCryptographic(url)) {
-                    return featureIsEnabled ? R.drawable.omnibox_not_secure_warning
-                                            : R.drawable.omnibox_info;
-                }
                 return isSmallDevice
                                 && (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito())
                                         || getNewTabPageForCurrentTab() != null)
                         ? 0
                         : R.drawable.omnibox_info;
             case ConnectionSecurityLevel.WARNING:
-                if (mNativeLocationBarModelAndroid == 0) {
-                    return R.drawable.omnibox_info;
-                }
-                if (featureIsEnabled && !SecurityStateModel.isOriginSecure(url)) {
-                    return R.drawable.omnibox_not_secure_warning;
-                }
                 return R.drawable.omnibox_info;
             case ConnectionSecurityLevel.DANGEROUS:
                 return R.drawable.omnibox_not_secure_warning;

chrome/android/junit/src/org/chromium/chrome/browser/toolbar/ToolbarSecurityIconTest.java

@@ -4,31 +4,22 @@
 package org.chromium.chrome.browser.toolbar;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
 import android.content.res.Resources;
 
-import org.junit.After;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
-import org.robolectric.annotation.Implementation;
-import org.robolectric.annotation.Implements;
-import org.robolectric.annotation.Resetter;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.flags.FeatureUtilities;
 import org.chromium.chrome.browser.omnibox.LocationBarLayout;
-import org.chromium.chrome.browser.ssl.SecurityStateModel;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
 
@@ -36,7 +27,7 @@ import org.chromium.components.security_state.ConnectionSecurityLevel;
  * Unit tests for {@link LocationBarLayout} class.
  */
 @RunWith(BaseRobolectricTestRunner.class)
-@Config(manifest = Config.NONE, shadows = {ToolbarSecurityIconTest.ShadowSecurityStateModel.class})
+@Config(manifest = Config.NONE)
 public final class ToolbarSecurityIconTest {
     private static final boolean IS_SMALL_DEVICE = true;
     private static final boolean IS_OFFLINE_PAGE = true;
@@ -44,10 +35,6 @@ public final class ToolbarSecurityIconTest {
     private static final int[] SECURITY_LEVELS = new int[] {ConnectionSecurityLevel.NONE,
             ConnectionSecurityLevel.WARNING, ConnectionSecurityLevel.DANGEROUS,
             ConnectionSecurityLevel.SECURE, ConnectionSecurityLevel.EV_SECURE};
-    private static final long NATIVE_PTR = 1;
-
-    @Rule
-    public JniMocker mocker = new JniMocker();
 
     @Mock
     private Tab mTab;
@@ -55,55 +42,14 @@ public final class ToolbarSecurityIconTest {
     private Context mContext;
     @Mock
     private Resources mResources;
-    @Mock
-    LocationBarModel.Natives mNativeMocks;
 
     private LocationBarModel mLocationBarModel;
 
-    @Implements(SecurityStateModel.class)
-    static class ShadowSecurityStateModel {
-        private static boolean sSchemeIsCryptographic;
-        private static boolean sOriginIsSecure;
-
-        @Resetter
-        public static void reset() {
-            sSchemeIsCryptographic = false;
-            sOriginIsSecure = false;
-        }
-
-        @Implementation
-        public static boolean isSchemeCryptographic(String url) {
-            return sSchemeIsCryptographic;
-        }
-
-        @Implementation
-        public static boolean isOriginSecure(String url) {
-            return sOriginIsSecure;
-        }
-
-        public static void setSchemeIsCryptographic(boolean schemeIsCryptographicValue) {
-            sSchemeIsCryptographic = schemeIsCryptographicValue;
-        }
-
-        public static void setOriginIsSecure(boolean originIsSecureValue) {
-            sOriginIsSecure = originIsSecureValue;
-        }
-    }
-
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mocker.mock(LocationBarModelJni.TEST_HOOKS, mNativeMocks);
         doReturn(mResources).when(mContext).getResources();
-        when(mNativeMocks.init(any())).thenReturn(NATIVE_PTR);
         mLocationBarModel = new LocationBarModel(mContext);
-        mLocationBarModel.initializeWithNative();
-    }
-
-    @After
-    public void tearDown() {
-        ShadowSecurityStateModel.reset();
-        FeatureUtilities.setMarkHttpAsDangerWarningEnabledForTesting(null);
     }
 
     @Test
@@ -163,15 +109,6 @@ public final class ToolbarSecurityIconTest {
                 mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
                         !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
 
-        // Tests passive mixed content.
-        ShadowSecurityStateModel.setSchemeIsCryptographic(true);
-        assertEquals(R.drawable.omnibox_info,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-        assertEquals(R.drawable.omnibox_info,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-
         assertEquals(R.drawable.omnibox_info,
                 mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
                         IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
@@ -209,43 +146,4 @@ public final class ToolbarSecurityIconTest {
                 mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.EV_SECURE,
                         !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
     }
-
-    @Test
-    public void testGetSecurityIconResourceForMarkHttpAsDangerWarning() {
-        FeatureUtilities.setMarkHttpAsDangerWarningEnabledForTesting(true);
-
-        assertEquals(0,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-        assertEquals(R.drawable.omnibox_info,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-
-        // Tests passive mixed content.
-        ShadowSecurityStateModel.setSchemeIsCryptographic(true);
-        assertEquals(R.drawable.omnibox_not_secure_warning,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-        assertEquals(R.drawable.omnibox_not_secure_warning,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.NONE,
-                        !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-
-        // Tests non-secure connection (e.g. HTTP).
-        ShadowSecurityStateModel.setSchemeIsCryptographic(false);
-        assertEquals(R.drawable.omnibox_not_secure_warning,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
-                        IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-        assertEquals(R.drawable.omnibox_not_secure_warning,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
-                        !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-
-        // Tests non-cryptographic secure origins.
-        ShadowSecurityStateModel.setOriginIsSecure(true);
-        assertEquals(R.drawable.omnibox_info,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
-                        IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-        assertEquals(R.drawable.omnibox_info,
-                mLocationBarModel.getSecurityIconResource(ConnectionSecurityLevel.WARNING,
-                        !IS_SMALL_DEVICE, !IS_OFFLINE_PAGE, !IS_PREVIEW));
-    }
 }

chrome/browser/ssl/security_state_model_android.cc

@@ -2,17 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/android/jni_string.h"
-#include "base/android/scoped_java_ref.h"
 #include "base/logging.h"
 #include "chrome/android/chrome_jni_headers/SecurityStateModel_jni.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "components/security_state/core/security_state.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/common/origin_util.h"
-#include "url/gurl.h"
 
-using base::android::ConvertJavaStringToUTF16;
 using base::android::JavaParamRef;
 
 // static
@@ -28,19 +23,3 @@ jint JNI_SecurityStateModel_GetSecurityLevelForWebContents(
   DCHECK(helper);
   return helper->GetSecurityLevel();
 }
-
-// static
-jboolean JNI_SecurityStateModel_IsOriginSecure(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& jurl) {
-  GURL url(ConvertJavaStringToUTF16(env, jurl));
-  return url.is_valid() && content::IsOriginSecure(url);
-}
-
-// static
-jboolean JNI_SecurityStateModel_IsSchemeCryptographic(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& jurl) {
-  GURL url(ConvertJavaStringToUTF16(env, jurl));
-  return url.is_valid() && url.SchemeIsCryptographic();
-}