Commit 0d5cdec6 authored by Aaron Krajeski's avatar Aaron Krajeski Committed by Commit Bot

Reland "Add managed text view in Security settings page."

This reverts commit 5abb4e4d.

Reason for revert: crbug.com/1112723 was the real culprit

Original change's description:
> Revert "Add managed text view in Security settings page."
> 
> This reverts commit 835d50e1.
> 
> Reason for revert: Linux CFI failures https://ci.chromium.org/p/chromium/builders/ci/Linux%20CFI
> 
> Original change's description:
> > Add managed text view in Security settings page.
> > 
> > Screenshot: http://screen/zaaBRbPGsTd
> > 
> > Bug: 1097310
> > Change-Id: I26b76f2071d4e2ee5b4fc5ff04868b24bed88a8c
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316829
> > Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
> > Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
> > Reviewed-by: Varun Khaneja <vakh@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#794260}
> 
> TBR=vakh@chromium.org,chouinard@chromium.org,xinghuilu@chromium.org
> 
> Change-Id: I065975a7f0ee2fbb6d04b22c36adc12bc8502d13
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1097310
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333685
> Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
> Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#794520}

TBR=vakh@chromium.org,aaronhk@chromium.org,chouinard@chromium.org,xinghuilu@chromium.org

# Not skipping CQ checks because this is a reland.

Bug: 1097310
Change-Id: I4d1c5b5050ad570b411c2843d84f2acb4769e30d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337113Reviewed-by: default avatarAaron Krajeski <aaronhk@chromium.org>
Reviewed-by: default avatarXinghui Lu <xinghuilu@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794664}
parent e789b31d
......@@ -10,7 +10,8 @@
android:key="safe_browsing_section"
android:title="@string/prefs_section_safe_browsing_title"/>
<!-- TODO(crbug.com/1097310): Add managed text view for Safe Browsing preference. -->
<org.chromium.components.browser_ui.settings.TextMessagePreference
android:key="text_managed" />
<org.chromium.chrome.browser.safe_browsing.settings.RadioButtonGroupSafeBrowsingPreference
android:key="safe_browsing_radio_button_group"
......
......@@ -59,6 +59,14 @@ public final class SafeBrowsingBridge {
SafeBrowsingBridgeJni.get().setSafeBrowsingState(state);
}
/**
* @return Whether the Safe Browsing preference is managed. It can be managed by either
* the SafeBrowsingEnabled policy(legacy) or the SafeBrowsingProtectionLevel policy(new).
*/
public static boolean isSafeBrowsingManaged() {
return SafeBrowsingBridgeJni.get().isSafeBrowsingManaged();
}
/**
* @return Whether there is a Google account to use for the leak detection check.
*/
......@@ -76,5 +84,6 @@ public final class SafeBrowsingBridge {
int getSafeBrowsingState();
void setSafeBrowsingState(@SafeBrowsingState int state);
boolean hasAccountForLeakCheckRequest();
boolean isSafeBrowsingManaged();
}
}
......@@ -15,6 +15,8 @@ import androidx.preference.Preference;
import androidx.preference.PreferenceViewHolder;
import org.chromium.chrome.browser.safe_browsing.SafeBrowsingState;
import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
import org.chromium.components.browser_ui.settings.ManagedPreferencesUtils;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescription;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescriptionAndAuxButton;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescriptionLayout;
......@@ -52,6 +54,7 @@ public class RadioButtonGroupSafeBrowsingPreference extends Preference
private @SafeBrowsingState int mSafeBrowsingState;
private boolean mIsEnhancedProtectionEnabled;
private OnSafeBrowsingModeDetailsRequested mSafeBrowsingModeDetailsRequestedListener;
private ManagedPreferenceDelegate mManagedPrefDelegate;
public RadioButtonGroupSafeBrowsingPreference(Context context, AttributeSet attrs) {
super(context, attrs);
......@@ -112,6 +115,16 @@ public class RadioButtonGroupSafeBrowsingPreference extends Preference
}
mStandardProtection.setChecked(mSafeBrowsingState == SafeBrowsingState.STANDARD_PROTECTION);
mNoProtection.setChecked(mSafeBrowsingState == SafeBrowsingState.NO_SAFE_BROWSING);
// If Safe Browsing is managed, disable the radio button group, but keep the aux buttons
// enabled to disclose information.
if (mManagedPrefDelegate.isPreferenceClickDisabledByPolicy(this)) {
groupLayout.setEnabled(false);
if (mIsEnhancedProtectionEnabled) {
mEnhancedProtection.setAuxButtonEnabled(true);
}
mStandardProtection.setAuxButtonEnabled(true);
}
}
@Override
......@@ -139,6 +152,14 @@ public class RadioButtonGroupSafeBrowsingPreference extends Preference
mSafeBrowsingModeDetailsRequestedListener = listener;
}
/**
* Sets the ManagedPreferenceDelegate which will determine whether this preference is managed.
*/
public void setManagedPreferenceDelegate(ManagedPreferenceDelegate delegate) {
mManagedPrefDelegate = delegate;
ManagedPreferencesUtils.initPreference(mManagedPrefDelegate, this);
}
@VisibleForTesting
public @SafeBrowsingState int getSafeBrowsingStateForTesting() {
return mSafeBrowsingState;
......
......@@ -12,9 +12,12 @@ import androidx.preference.PreferenceFragmentCompat;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.safe_browsing.SafeBrowsingBridge;
import org.chromium.chrome.browser.safe_browsing.SafeBrowsingState;
import org.chromium.chrome.browser.settings.ChromeManagedPreferenceDelegate;
import org.chromium.chrome.browser.settings.FragmentSettingsLauncher;
import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
import org.chromium.components.browser_ui.settings.SettingsUtils;
import org.chromium.components.browser_ui.settings.TextMessagePreference;
/**
* Fragment containing security settings.
......@@ -23,7 +26,9 @@ public class SecuritySettingsFragment extends PreferenceFragmentCompat
implements FragmentSettingsLauncher,
RadioButtonGroupSafeBrowsingPreference.OnSafeBrowsingModeDetailsRequested {
@VisibleForTesting
public static final String PREF_SAFE_BROWSING = "safe_browsing_radio_button_group";
static final String PREF_TEXT_MANAGED = "text_managed";
@VisibleForTesting
static final String PREF_SAFE_BROWSING = "safe_browsing_radio_button_group";
// An instance of SettingsLauncher that is used to launch Safe Browsing subsections.
private SettingsLauncher mSettingsLauncher;
......@@ -33,18 +38,26 @@ public class SecuritySettingsFragment extends PreferenceFragmentCompat
SettingsUtils.addPreferencesFromResource(this, R.xml.security_preferences);
getActivity().setTitle(R.string.prefs_security_title);
ManagedPreferenceDelegate managedPreferenceDelegate = createManagedPreferenceDelegate();
RadioButtonGroupSafeBrowsingPreference safeBrowsingPreference =
findPreference(PREF_SAFE_BROWSING);
safeBrowsingPreference.init(SafeBrowsingBridge.getSafeBrowsingState(),
ChromeFeatureList.isEnabled(
ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED));
safeBrowsingPreference.setSafeBrowsingModeDetailsRequestedListener(this);
safeBrowsingPreference.setManagedPreferenceDelegate(managedPreferenceDelegate);
safeBrowsingPreference.setOnPreferenceChangeListener((preference, newValue) -> {
@SafeBrowsingState
int newState = (int) newValue;
SafeBrowsingBridge.setSafeBrowsingState(newState);
return true;
});
TextMessagePreference textManaged = findPreference(PREF_TEXT_MANAGED);
textManaged.setManagedPreferenceDelegate(managedPreferenceDelegate);
textManaged.setVisible(managedPreferenceDelegate.isPreferenceClickDisabledByPolicy(
safeBrowsingPreference));
}
@Override
......@@ -64,4 +77,16 @@ public class SecuritySettingsFragment extends PreferenceFragmentCompat
public void setSettingsLauncher(SettingsLauncher settingsLauncher) {
mSettingsLauncher = settingsLauncher;
}
private ChromeManagedPreferenceDelegate createManagedPreferenceDelegate() {
return preference -> {
String key = preference.getKey();
if (PREF_TEXT_MANAGED.equals(key) || PREF_SAFE_BROWSING.equals(key)) {
return SafeBrowsingBridge.isSafeBrowsingManaged();
} else {
assert false : "Should not be reached.";
}
return false;
};
}
}
......@@ -16,22 +16,25 @@ import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
import org.chromium.chrome.browser.safe_browsing.SafeBrowsingBridge;
import org.chromium.chrome.browser.safe_browsing.SafeBrowsingState;
import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.settings.TextMessagePreference;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescription;
import org.chromium.components.browser_ui.widget.RadioButtonWithDescriptionAndAuxButton;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.policy.test.annotations.Policies;
/**
* Tests for {@link SecuritySettingsFragment}.
*/
@RunWith(BaseJUnit4ClassRunner.class)
@RunWith(ChromeJUnit4ClassRunner.class)
// clang-format off
@Features.EnableFeatures({ChromeFeatureList.SAFE_BROWSING_SECURITY_SECTION_UI})
public class SecuritySettingsFragmentTest {
......@@ -55,11 +58,11 @@ public class SecuritySettingsFragmentTest {
private SecuritySettingsFragment mSecuritySettingsFragment;
private RadioButtonGroupSafeBrowsingPreference mSafeBrowsingPreference;
private TextMessagePreference mManagedTextPreference;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
launchSettingsActivity();
}
private void launchSettingsActivity() {
......@@ -67,8 +70,11 @@ public class SecuritySettingsFragmentTest {
mSecuritySettingsFragment = mTestRule.getFragment();
mSafeBrowsingPreference = mSecuritySettingsFragment.findPreference(
SecuritySettingsFragment.PREF_SAFE_BROWSING);
mManagedTextPreference = mSecuritySettingsFragment.findPreference(
SecuritySettingsFragment.PREF_TEXT_MANAGED);
Assert.assertNotNull(
"Safe Browsing preference should not be null.", mSafeBrowsingPreference);
Assert.assertNotNull("Text managed preference should not be null.", mManagedTextPreference);
}
@Test
......@@ -76,6 +82,7 @@ public class SecuritySettingsFragmentTest {
@Feature({"SafeBrowsing"})
@Features.EnableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
public void testOnStartup() {
launchSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
@SafeBrowsingState
int currentState = SafeBrowsingBridge.getSafeBrowsingState();
......@@ -90,6 +97,7 @@ public class SecuritySettingsFragmentTest {
getStandardProtectionButton().isChecked());
Assert.assertEquals(ASSERT_RADIO_BUTTON_CHECKED, no_protection_checked,
getNoProtectionButton().isChecked());
Assert.assertFalse(mManagedTextPreference.isVisible());
});
}
......@@ -98,7 +106,9 @@ public class SecuritySettingsFragmentTest {
@Feature({"SafeBrowsing"})
@Features.EnableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
public void testCheckRadioButtons() {
launchSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse(mManagedTextPreference.isVisible());
// Click the Enhanced Protection button.
getEnhancedProtectionButton().onClick(null);
Assert.assertEquals(ASSERT_SAFE_BROWSING_STATE_RADIO_BUTTON_GROUP,
......@@ -144,6 +154,7 @@ public class SecuritySettingsFragmentTest {
@Feature({"SafeBrowsing"})
@Features.DisableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
public void testEnhancedProtectionDisabled() {
launchSettingsActivity();
Assert.assertNull(getEnhancedProtectionButton());
}
......@@ -152,6 +163,7 @@ public class SecuritySettingsFragmentTest {
@Feature({"SafeBrowsing"})
@Features.EnableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
public void testEnhancedProtectionAuxButtonClicked() {
launchSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mSecuritySettingsFragment.setSettingsLauncher(mSettingsLauncher);
getEnhancedProtectionButton().getAuxButtonForTests().performClick();
......@@ -166,6 +178,7 @@ public class SecuritySettingsFragmentTest {
@Feature({"SafeBrowsing"})
@Features.EnableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
public void testStandardProtectionAuxButtonClicked() {
launchSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mSecuritySettingsFragment.setSettingsLauncher(mSettingsLauncher);
getStandardProtectionButton().getAuxButtonForTests().performClick();
......@@ -175,6 +188,28 @@ public class SecuritySettingsFragmentTest {
});
}
@Test
@SmallTest
@Feature({"SafeBrowsing"})
@Features.EnableFeatures(ChromeFeatureList.SAFE_BROWSING_ENHANCED_PROTECTION_ENABLED)
@Policies.Add({ @Policies.Item(key = "SafeBrowsingEnabled", string = "true") })
public void testSafeBrowsingManaged() {
TestThreadUtils.runOnUiThreadBlocking(
() -> { ChromeBrowserInitializer.getInstance().handleSynchronousStartup(); });
launchSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertTrue(SafeBrowsingBridge.isSafeBrowsingManaged());
Assert.assertTrue(mManagedTextPreference.isVisible());
Assert.assertFalse(getEnhancedProtectionButton().isEnabled());
Assert.assertFalse(getStandardProtectionButton().isEnabled());
Assert.assertFalse(getNoProtectionButton().isEnabled());
Assert.assertEquals(SafeBrowsingState.STANDARD_PROTECTION, getSafeBrowsingState());
// To disclose information, aux buttons should be enabled under managed mode.
Assert.assertTrue(getEnhancedProtectionButton().getAuxButtonForTests().isEnabled());
Assert.assertTrue(getStandardProtectionButton().getAuxButtonForTests().isEnabled());
});
}
private @SafeBrowsingState int getSafeBrowsingState() {
return mSafeBrowsingPreference.getSafeBrowsingStateForTesting();
}
......
......@@ -66,6 +66,10 @@ static void JNI_SafeBrowsingBridge_SetSafeBrowsingState(JNIEnv* env,
GetPrefService(), static_cast<SafeBrowsingState>(state));
}
static jboolean JNI_SafeBrowsingBridge_IsSafeBrowsingManaged(JNIEnv* env) {
return safe_browsing::IsSafeBrowsingPolicyManaged(*GetPrefService());
}
static jboolean JNI_SafeBrowsingBridge_HasAccountForLeakCheckRequest(
JNIEnv* env) {
signin::IdentityManager* identity_manager =
......
......@@ -185,6 +185,11 @@ bool IsExtendedReportingPolicyManaged(const PrefService& prefs) {
return prefs.IsManagedPreference(prefs::kSafeBrowsingScoutReportingEnabled);
}
bool IsSafeBrowsingPolicyManaged(const PrefService& prefs) {
return prefs.IsManagedPreference(prefs::kSafeBrowsingEnabled) ||
prefs.IsManagedPreference(prefs::kSafeBrowsingEnhanced);
}
void RecordExtendedReportingMetrics(const PrefService& prefs) {
// This metric tracks the extended browsing opt-in based on whichever setting
// the user is currently seeing. It tells us whether extended reporting is
......
......@@ -291,6 +291,11 @@ bool IsExtendedReportingEnabled(const PrefService& prefs);
// enterprise policy, meaning the user can't change it.
bool IsExtendedReportingPolicyManaged(const PrefService& prefs);
// Return whether the Safe Browsing preference is managed. It can be managed by
// either the SafeBrowsingEnabled policy(legacy) or the
// SafeBrowsingProtectionLevel policy(new).
bool IsSafeBrowsingPolicyManaged(const PrefService& prefs);
// Updates UMA metrics about Safe Browsing Extended Reporting states.
void RecordExtendedReportingMetrics(const PrefService& prefs);
......
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