Commit f5e3d066 authored by Andy Paicu's avatar Andy Paicu Committed by Commit Bot

Removed kModalPermissionPrompts feature

It's been over a year since modal permission prompts have been enabled
by default. The usage is now very low so it's time to removed them.
https://uma.googleplex.com/histograms?sid=19aaae5bcd78f725505eebc7e91172ce
Usage in stable less than 0.0003%

Bug: 935900
Change-Id: Ifecfacec485fc1a18fccbf7d219209d3727946b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1757939Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689420}
parent d0e5c1f6
......@@ -75,7 +75,6 @@ import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeApplication;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.IntentHandler;
......@@ -158,8 +157,8 @@ public class CustomTabActivityTest {
private static final int NUM_CHROME_MENU_ITEMS = 5;
private static final String TEST_PAGE = "/chrome/test/data/android/google.html";
private static final String TEST_PAGE_2 = "/chrome/test/data/android/test.html";
private static final String GEOLOCATION_PAGE =
"/chrome/test/data/geolocation/geolocation_on_load.html";
private static final String POPUP_PAGE =
"/chrome/test/data/popup_blocker/popup-window-open.html";
private static final String SELECT_POPUP_PAGE = "/chrome/test/data/android/select.html";
private static final String FRAGMENT_TEST_PAGE = "/chrome/test/data/android/fragment.html";
private static final String TARGET_BLANK_TEST_PAGE =
......@@ -815,19 +814,16 @@ public class CustomTabActivityTest {
/**
* Test whether a custom tab can be reparented to a new activity while showing an infobar.
*
* TODO(timloh): Use a different InfoBar type once we only use modals for permission prompts.
*/
@Test
@SmallTest
@DisableFeatures(ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@RetryOnFailure
public void testTabReparentingInfoBar() throws InterruptedException {
LocationSettingsTestUtil.setSystemLocationSettingEnabled(true);
mCustomTabActivityTestRule.startCustomTabActivityWithIntent(
CustomTabsTestUtils.createMinimalCustomTabIntent(
InstrumentationRegistry.getTargetContext(),
mTestServer.getURL(GEOLOCATION_PAGE)));
mTestServer.getURL(POPUP_PAGE)));
CriteriaHelper.pollUiThread(
() -> isInfoBarSizeOne(mCustomTabActivityTestRule.getActivity().getActivityTab()));
......
......@@ -28,7 +28,6 @@ import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.WebContentsFactory;
import org.chromium.chrome.browser.datareduction.DataReductionPromoUtils;
......@@ -38,7 +37,6 @@ import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.InfoBarTestAnimationListener;
import org.chromium.chrome.test.util.InfoBarUtil;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.chrome.test.util.browser.LocationSettingsTestUtil;
import org.chromium.content_public.browser.UiThreadTaskTraits;
......@@ -62,8 +60,6 @@ public class InfoBarTest {
private static final long MAX_TIMEOUT = scaleTimeout(2000);
private static final int CHECK_INTERVAL = 500;
private static final String GEOLOCATION_PAGE =
"/chrome/test/data/geolocation/geolocation_on_load.html";
private static final String POPUP_PAGE =
"/chrome/test/data/popup_blocker/popup-window-open.html";
private static final String HELLO_WORLD_URL = UrlUtils.encodeHtmlDataUri("<html>"
......@@ -253,46 +249,17 @@ public class InfoBarTest {
}
/**
* Verify Geolocation creates an InfoBar.
*
* TODO(timloh): Remove this once we only use modals for permission prompts.
* Verify Popups create an InfoBar and that it's destroyed when navigating back.
*/
@Test
@MediumTest
@DisableFeatures(ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@Feature({"Browser", "Main"})
@RetryOnFailure
public void testInfoBarForGeolocation() throws InterruptedException, TimeoutException {
LocationSettingsTestUtil.setSystemLocationSettingEnabled(true);
mActivityTestRule.loadUrl(mTestServer.getURL(GEOLOCATION_PAGE));
mListener.addInfoBarAnimationFinished("InfoBar not added");
// Make sure it has OK/Cancel buttons.
List<InfoBar> infoBars = mActivityTestRule.getInfoBars();
Assert.assertEquals("Wrong infobar count", 1, infoBars.size());
Assert.assertTrue(InfoBarUtil.hasPrimaryButton(infoBars.get(0)));
Assert.assertTrue(InfoBarUtil.hasSecondaryButton(infoBars.get(0)));
mActivityTestRule.loadUrl(HELLO_WORLD_URL);
mListener.removeInfoBarAnimationFinished("InfoBar not removed.");
Assert.assertTrue("Wrong infobar count", mActivityTestRule.getInfoBars().isEmpty());
}
/**
* Verify Geolocation creates an InfoBar and that it's destroyed when navigating back.
*
* TODO(timloh): Use a different InfoBar type once we only use modals for permission prompts.
*/
@Test
@MediumTest
@DisableFeatures(ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@Feature({"Browser"})
@RetryOnFailure
public void testInfoBarForGeolocationDisappearsOnBack()
throws InterruptedException, TimeoutException {
LocationSettingsTestUtil.setSystemLocationSettingEnabled(true);
mActivityTestRule.loadUrl(HELLO_WORLD_URL);
mActivityTestRule.loadUrl(mTestServer.getURL(GEOLOCATION_PAGE));
mActivityTestRule.loadUrl(mTestServer.getURL(POPUP_PAGE));
mListener.addInfoBarAnimationFinished("InfoBar not added.");
Assert.assertEquals("Wrong infobar count", 1, mActivityTestRule.getInfoBars().size());
......@@ -567,19 +534,16 @@ public class InfoBarTest {
/**
* Verify InfoBarContainers swap the WebContents they are monitoring properly.
*
* TODO(timloh): Use a different InfoBar type once we only use modals for permission prompts.
*/
@Test
@MediumTest
@DisableFeatures(ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@Feature({"Browser", "Main"})
@RetryOnFailure
public void testInfoBarContainerSwapsWebContents()
throws InterruptedException, TimeoutException {
// Add an infobar.
LocationSettingsTestUtil.setSystemLocationSettingEnabled(true);
mActivityTestRule.loadUrl(mTestServer.getURL(GEOLOCATION_PAGE));
mActivityTestRule.loadUrl(mTestServer.getURL(POPUP_PAGE));
mListener.addInfoBarAnimationFinished("InfoBar not added");
Assert.assertEquals("Wrong infobar count", 1, mActivityTestRule.getInfoBars().size());
......@@ -598,7 +562,7 @@ public class InfoBarTest {
// Revisiting the original page should make the InfoBar reappear.
InfoBarTestAnimationListener addListener = new InfoBarTestAnimationListener();
mActivityTestRule.getInfoBarContainer().addAnimationListener(addListener);
mActivityTestRule.loadUrl(mTestServer.getURL(GEOLOCATION_PAGE));
mActivityTestRule.loadUrl(mTestServer.getURL(POPUP_PAGE));
addListener.addInfoBarAnimationFinished("InfoBar not added");
Assert.assertEquals("Wrong infobar count", 1, mActivityTestRule.getInfoBars().size());
}
......
......@@ -12,6 +12,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
......@@ -68,9 +69,9 @@ public class GeolocationTest {
*/
@Test
@MediumTest
@CommandLineFlags.Add("disable-features=" + PermissionTestRule.MODAL_FLAG)
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@Feature({"Location", "Main"})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testGeolocationPlumbingAllowedInfoBar() throws Exception {
runTest("initiate_getCurrentPosition()", 1, false, false);
}
......@@ -81,7 +82,6 @@ public class GeolocationTest {
*/
@Test
@MediumTest
@CommandLineFlags.Add("enable-features=" + PermissionTestRule.MODAL_FLAG)
@Feature({"Location", "Main"})
public void testGeolocationPlumbingAllowedDialog() throws Exception {
runTest("initiate_getCurrentPosition()", 1, true, true);
......@@ -94,7 +94,6 @@ public class GeolocationTest {
*/
@Test
@MediumTest
@CommandLineFlags.Add("enable-features=" + PermissionTestRule.MODAL_FLAG)
@Feature({"Location", "Main"})
public void testGeolocationPlumbingAllowedDialogNoGesture() throws Exception {
runTest("initiate_getCurrentPosition()", 1, false, true);
......@@ -106,9 +105,9 @@ public class GeolocationTest {
*/
@Test
@MediumTest
@CommandLineFlags.Add("disable-features=" + PermissionTestRule.MODAL_FLAG)
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@Feature({"Location"})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testGeolocationWatchInfoBar() throws Exception {
runTest("initiate_watchPosition()", 2, false, false);
}
......@@ -119,7 +118,6 @@ public class GeolocationTest {
*/
@Test
@MediumTest
@CommandLineFlags.Add("enable-features=" + PermissionTestRule.MODAL_FLAG)
@Feature({"Location"})
public void testGeolocationWatchDialog() throws Exception {
runTest("initiate_watchPosition()", 2, true, true);
......
......@@ -12,6 +12,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
......@@ -60,8 +61,9 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "disable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testMicrophonePermissionsPlumbingInfoBar() throws Exception {
testMediaPermissionsPlumbing("Mic count:", "initiate_getMicrophone()", 1, false, false);
}
......@@ -73,7 +75,7 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "enable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
public void testMicrophoneMediaPermissionsPlumbingDialog() throws Exception {
testMediaPermissionsPlumbing("Mic count:", "initiate_getMicrophone()", 1, true, true);
}
......@@ -85,8 +87,9 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "disable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testCameraPermissionsPlumbingInfoBar() throws Exception {
testMediaPermissionsPlumbing("Camera count:", "initiate_getCamera()", 1, false, false);
}
......@@ -99,7 +102,7 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "enable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
public void testCameraPermissionsPlumbingDialog() throws Exception {
testMediaPermissionsPlumbing("Camera count:", "initiate_getCamera()", 1, false, true);
}
......@@ -112,8 +115,9 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "disable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testCombinedPermissionsPlumbing() throws Exception {
testMediaPermissionsPlumbing("Combined count:", "initiate_getCombined()", 1, false, false);
}
......@@ -126,7 +130,7 @@ public class MediaTest {
@Test
@MediumTest
@Feature({"MediaPermissions", "Main"})
@CommandLineFlags.Add({FAKE_DEVICE, "enable-features=" + PermissionTestRule.MODAL_FLAG})
@CommandLineFlags.Add({FAKE_DEVICE})
public void testCombinedPermissionsPlumbingDialog() throws Exception {
testMediaPermissionsPlumbing("Combined count:", "initiate_getCombined()", 1, true, true);
}
......
......@@ -57,7 +57,6 @@ public class PermissionNavigationTest {
@Test
@MediumTest
@Feature({"Permissions"})
@CommandLineFlags.Add("enable-features=" + PermissionTestRule.MODAL_FLAG)
public void testNavigationDismissesModalPermissionPrompt() throws Exception {
mPermissionRule.setUpUrl(TEST_FILE);
mPermissionRule.runJavaScriptCodeInCurrentTab("requestGeolocationPermission()");
......
......@@ -12,7 +12,6 @@ import org.junit.runners.model.Statement;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.infobar.InfoBar;
import org.chromium.chrome.browser.modaldialog.ModalDialogTestUtils;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
......@@ -50,8 +49,6 @@ import java.util.concurrent.ExecutionException;
* JS call with a gesture, and whether an infobar or a dialog is expected.
*/
public class PermissionTestRule extends ChromeActivityTestRule<ChromeActivity> {
public static final String MODAL_FLAG = ChromeFeatureList.MODAL_PERMISSION_PROMPTS;
private InfoBarTestAnimationListener mListener;
private EmbeddedTestServer mTestServer;
......
......@@ -12,6 +12,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
......@@ -58,8 +59,8 @@ public class QuotaTest {
@Test
@MediumTest
@Feature({"QuotaPermissions"})
@CommandLineFlags.Add("disable-features=" + PermissionTestRule.MODAL_FLAG)
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testQuotaShowsInfobar() throws Exception {
testQuotaPermissionsPlumbing("initiate_requestQuota(1024)", 1, false, false);
}
......
......@@ -21,11 +21,11 @@ import org.junit.runner.RunWith;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ContentSettingsType;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
......@@ -117,8 +117,8 @@ public class SiteSettingsPreferencesTest {
@Test
@SmallTest
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@CommandLineFlags.Add("disable-features=" + ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@Feature({"Preferences"})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testSetAllowLocationEnabled() throws Exception {
setAllowLocation(true);
InfoBarTestAnimationListener listener = setInfoBarAnimationListener();
......@@ -139,8 +139,8 @@ public class SiteSettingsPreferencesTest {
@Test
@SmallTest
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@CommandLineFlags.Add("disable-features=" + ChromeFeatureList.MODAL_PERMISSION_PROMPTS)
@Feature({"Preferences"})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testSetAllowLocationNotEnabled() throws Exception {
setAllowLocation(false);
......@@ -610,8 +610,8 @@ public class SiteSettingsPreferencesTest {
@Test
@SmallTest
@Feature({"Preferences"})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM,
"disable-features=" + ChromeFeatureList.MODAL_PERMISSION_PROMPTS})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testMicBlocked() throws Exception {
setGlobalToggleForCategory(SiteSettingsCategory.Type.MICROPHONE, false);
......@@ -641,8 +641,8 @@ public class SiteSettingsPreferencesTest {
@SmallTest
@Feature({"Preferences"})
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM,
"disable-features=" + ChromeFeatureList.MODAL_PERMISSION_PROMPTS})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testCameraNotBlocked() throws Exception {
setEnableCamera(true);
......@@ -668,8 +668,8 @@ public class SiteSettingsPreferencesTest {
@SmallTest
@Feature({"Preferences"})
@Restriction({ChromeRestriction.RESTRICTION_TYPE_REQUIRES_TOUCH})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM,
"disable-features=" + ChromeFeatureList.MODAL_PERMISSION_PROMPTS})
@CommandLineFlags.Add({ContentSwitches.USE_FAKE_DEVICE_FOR_MEDIA_STREAM})
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testMicNotBlocked() throws Exception {
setEnableCamera(true);
......
......@@ -186,7 +186,7 @@ public class PushMessagingTest implements PushMessagingServiceObserver.Listener
@Test
@MediumTest
@Feature({"Browser", "PushMessaging"})
@CommandLineFlags.Add("disable-features=ModalPermissionPrompts")
@DisabledTest(message = "Modals are now enabled and test needs to be reworked crbug.com/935900")
public void testPushPermissionGranted() throws InterruptedException, TimeoutException {
// Notifications permission should initially be prompt.
Assert.assertEquals("\"default\"", runScriptBlocking("Notification.permission"));
......
......@@ -41,11 +41,6 @@ void PermissionDialogDelegate::Create(
new PermissionDialogDelegate(tab, permission_prompt);
}
// static
bool PermissionDialogDelegate::ShouldShowDialog() {
return base::FeatureList::IsEnabled(features::kModalPermissionPrompts);
}
void PermissionDialogDelegate::CreateJavaDelegate(JNIEnv* env,
TabAndroid* tab) {
base::android::ScopedJavaLocalRef<jstring> primaryButtonText =
......
......@@ -35,10 +35,6 @@ class PermissionDialogDelegate : public content::WebContentsObserver {
static void Create(content::WebContents* web_contents,
PermissionPromptAndroid* permission_prompt);
// Returns true if we should show the user a modal permission prompt rather
// than an infobar.
static bool ShouldShowDialog();
// JNI methods.
void Accept(JNIEnv* env, const JavaParamRef<jobject>& obj);
void Cancel(JNIEnv* env, const JavaParamRef<jobject>& obj);
......
......@@ -23,18 +23,7 @@ PermissionPromptAndroid::PermissionPromptAndroid(
: web_contents_(web_contents), delegate_(delegate) {
DCHECK(web_contents);
if (PermissionDialogDelegate::ShouldShowDialog()) {
PermissionDialogDelegate::Create(web_contents_, this);
return;
}
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents_);
if (!infobar_service)
return;
GroupedPermissionInfoBarDelegate::Create(weak_factory_.GetWeakPtr(),
infobar_service);
PermissionDialogDelegate::Create(web_contents_, this);
}
PermissionPromptAndroid::~PermissionPromptAndroid() {}
......
......@@ -485,11 +485,6 @@ const base::Feature kOnConnectNative{"OnConnectNative",
base::FEATURE_DISABLED_BY_DEFAULT};
#endif
// Enables or disables modal permission prompts.
// TODO(https://crbug.com/935900): Remove this.
const base::Feature kModalPermissionPrompts{"ModalPermissionPrompts",
base::FEATURE_ENABLED_BY_DEFAULT};
// Enables the use of native notification centers instead of using the Message
// Center for displaying the toasts. The feature is hardcoded to enabled for
// Chrome OS.
......
......@@ -307,9 +307,6 @@ COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kManagedGuestSessionNotification;
#endif
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kModalPermissionPrompts;
#if !defined(OS_ANDROID)
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kOnConnectNative;
......
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