Commit d9fc6113 authored by Peter Beverloo's avatar Peter Beverloo Committed by Commit Bot

Remove the command line flags to toggle custom notification layouts

They practically have zero usage and are a footgun for most users as
custom layouts are entirely untested on modern versions of Android. By
removing the command line flags, we simplify our Web Notification
display style to:

  Android J, K, L, M: custom layouts
  Android N, O; big image notification: default layout

BUG=

Change-Id: Ia1b47681fe67ebc66e9ba856fa5d2df82bdb2e07
Reviewed-on: https://chromium-review.googlesource.com/747422Reviewed-by: default avatarAnita Woodruff <awdf@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512911}
parent 3108fc39
...@@ -125,20 +125,6 @@ public abstract class ChromeSwitches { ...@@ -125,20 +125,6 @@ public abstract class ChromeSwitches {
*/ */
public static final String ENABLE_HUNG_RENDERER_INFOBAR = "enable-hung-renderer-infobar"; public static final String ENABLE_HUNG_RENDERER_INFOBAR = "enable-hung-renderer-infobar";
/**
* Enables Web Notification custom layouts.
* Native switch - switches::kEnableWebNotificationCustomLayouts
*/
public static final String ENABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS =
"enable-web-notification-custom-layouts";
/**
* Disables Web Notification custom layouts.
* Native switch - switches::kDisableWebNotificationCustomLayouts
*/
public static final String DISABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS =
"disable-web-notification-custom-layouts";
/** /**
* Determines which of the Herb prototypes is being tested. * Determines which of the Herb prototypes is being tested.
* See about:flags for descriptions. * See about:flags for descriptions.
......
...@@ -4,3 +4,4 @@ per-file SingleTabActivity.java=dominickn@chromium.org ...@@ -4,3 +4,4 @@ per-file SingleTabActivity.java=dominickn@chromium.org
# This is For simple changes of adding/removing features. For more structural # This is For simple changes of adding/removing features. For more structural
# changes, use the normal OWNERS rules. # changes, use the normal OWNERS rules.
per-file ChromeFeatureList.java=* per-file ChromeFeatureList.java=*
per-file ChromeSwitches.java=*
...@@ -21,7 +21,6 @@ import android.text.TextUtils; ...@@ -21,7 +21,6 @@ import android.text.TextUtils;
import android.text.style.StyleSpan; import android.text.style.StyleSpan;
import org.chromium.base.BuildInfo; import org.chromium.base.BuildInfo;
import org.chromium.base.CommandLine;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
...@@ -31,7 +30,6 @@ import org.chromium.base.metrics.RecordHistogram; ...@@ -31,7 +30,6 @@ import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.init.ChromeBrowserInitializer; import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions; import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions;
import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager; import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager;
...@@ -701,8 +699,6 @@ public class NotificationPlatformBridge { ...@@ -701,8 +699,6 @@ public class NotificationPlatformBridge {
* Determines whether to use standard notification layouts, using NotificationCompat.Builder, * Determines whether to use standard notification layouts, using NotificationCompat.Builder,
* or custom layouts using Chrome's own templates. * or custom layouts using Chrome's own templates.
* *
* The --{enable,disable}-web-notification-custom-layouts command line flags take precedence.
*
* Normally a standard layout is used on Android N+, and a custom layout is used on older * Normally a standard layout is used on Android N+, and a custom layout is used on older
* versions of Android. But if the notification has a content image, there isn't enough room for * versions of Android. But if the notification has a content image, there isn't enough room for
* the Site Settings button to go on its own line when showing an image, nor is there enough * the Site Settings button to go on its own line when showing an image, nor is there enough
...@@ -713,20 +709,7 @@ public class NotificationPlatformBridge { ...@@ -713,20 +709,7 @@ public class NotificationPlatformBridge {
*/ */
@VisibleForTesting @VisibleForTesting
static boolean useCustomLayouts(boolean hasImage) { static boolean useCustomLayouts(boolean hasImage) {
CommandLine commandLine = CommandLine.getInstance(); return Build.VERSION.SDK_INT < Build.VERSION_CODES.N && !hasImage;
if (commandLine.hasSwitch(ChromeSwitches.ENABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS)) {
return true;
}
if (commandLine.hasSwitch(ChromeSwitches.DISABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS)) {
return false;
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
return false;
}
if (hasImage) {
return false;
}
return true;
} }
/** /**
......
...@@ -5,21 +5,16 @@ ...@@ -5,21 +5,16 @@
package org.chromium.chrome.browser.notifications; package org.chromium.chrome.browser.notifications;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import android.app.Notification; import android.app.Notification;
import android.os.Build;
import org.junit.After;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.CommandLine;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions; import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions;
import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager; import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager;
import org.chromium.testing.local.LocalRobolectricTestRunner; import org.chromium.testing.local.LocalRobolectricTestRunner;
...@@ -32,52 +27,6 @@ import java.util.Arrays; ...@@ -32,52 +27,6 @@ import java.util.Arrays;
@RunWith(LocalRobolectricTestRunner.class) @RunWith(LocalRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
public class NotificationPlatformBridgeUnitTest { public class NotificationPlatformBridgeUnitTest {
@After
public void tearDown() {
// Clean up static state for subsequent tests.
CommandLine.reset();
}
@Test
@Feature({"Browser", "Notifications"})
public void testUseCustomLayouts() {
CommandLine.init(null);
CommandLine.getInstance().appendSwitch(
ChromeSwitches.ENABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS);
assertTrue(NotificationPlatformBridge.useCustomLayouts(false /* hasImage */));
assertTrue(NotificationPlatformBridge.useCustomLayouts(true /* hasImage */));
CommandLine.reset();
CommandLine.init(null);
CommandLine.getInstance().appendSwitch(
ChromeSwitches.DISABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS);
assertFalse(NotificationPlatformBridge.useCustomLayouts(false /* hasImage */));
assertFalse(NotificationPlatformBridge.useCustomLayouts(true /* hasImage */));
CommandLine.reset();
// Enable flag takes precedence over disable flag (arbitrarily).
CommandLine.init(null);
CommandLine.getInstance().appendSwitch(
ChromeSwitches.ENABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS);
CommandLine.getInstance().appendSwitch(
ChromeSwitches.DISABLE_WEB_NOTIFICATION_CUSTOM_LAYOUTS);
assertTrue(NotificationPlatformBridge.useCustomLayouts(false /* hasImage */));
assertTrue(NotificationPlatformBridge.useCustomLayouts(true /* hasImage */));
CommandLine.reset();
CommandLine.init(null);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
// Without comand line flags, custom layouts are always disabled on Nougat+.
assertFalse(NotificationPlatformBridge.useCustomLayouts(false /* hasImage */));
assertFalse(NotificationPlatformBridge.useCustomLayouts(true /* hasImage */));
} else {
// On older versions of Android, custom layouts are enabled unless an image is provided.
assertTrue(NotificationPlatformBridge.useCustomLayouts(false /* hasImage */));
assertFalse(NotificationPlatformBridge.useCustomLayouts(true /* hasImage */));
}
CommandLine.reset();
}
/** /**
* Verifies that the getOriginFromTag method returns the origin for valid input, and null for * Verifies that the getOriginFromTag method returns the origin for valid input, and null for
* invalid input. * invalid input.
......
...@@ -2372,14 +2372,6 @@ const FeatureEntry kFeatureEntries[] = { ...@@ -2372,14 +2372,6 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kCrosRegionsModeDescription, kOsCrOS, flag_descriptions::kCrosRegionsModeDescription, kOsCrOS,
MULTI_VALUE_TYPE(kCrosRegionsModeChoices)}, MULTI_VALUE_TYPE(kCrosRegionsModeChoices)},
#endif // OS_CHROMEOS #endif // OS_CHROMEOS
#if defined(OS_ANDROID)
{"enable-web-notification-custom-layouts",
flag_descriptions::kEnableWebNotificationCustomLayoutsName,
flag_descriptions::kEnableWebNotificationCustomLayoutsDescription,
kOsAndroid,
ENABLE_DISABLE_VALUE_TYPE(switches::kEnableWebNotificationCustomLayouts,
switches::kDisableWebNotificationCustomLayouts)},
#endif // OS_ANDROID
#if defined(OS_WIN) #if defined(OS_WIN)
{"enable-appcontainer", flag_descriptions::kEnableAppcontainerName, {"enable-appcontainer", flag_descriptions::kEnableAppcontainerName,
flag_descriptions::kEnableAppcontainerDescription, kOsWin, flag_descriptions::kEnableAppcontainerDescription, kOsWin,
......
...@@ -523,12 +523,6 @@ const char kEnableWasmStreamingName[] = ...@@ -523,12 +523,6 @@ const char kEnableWasmStreamingName[] =
const char kEnableWasmStreamingDescription[] = const char kEnableWasmStreamingDescription[] =
"WebAssembly.{compile|instantiate} taking a Response as parameter."; "WebAssembly.{compile|instantiate} taking a Response as parameter.";
const char kEnableWebNotificationCustomLayoutsName[] =
"Enable custom layouts for Web Notifications.";
const char kEnableWebNotificationCustomLayoutsDescription[] =
"Enable custom layouts for Web Notifications. They will have subtle layout "
"improvements that are otherwise not possible.";
const char kExpensiveBackgroundTimerThrottlingName[] = const char kExpensiveBackgroundTimerThrottlingName[] =
"Throttle expensive background timers"; "Throttle expensive background timers";
const char kExpensiveBackgroundTimerThrottlingDescription[] = const char kExpensiveBackgroundTimerThrottlingDescription[] =
......
...@@ -336,9 +336,6 @@ extern const char kEnableZeroSuggestRedirectToChromeDescription[]; ...@@ -336,9 +336,6 @@ extern const char kEnableZeroSuggestRedirectToChromeDescription[];
extern const char kEnableWasmStreamingName[]; extern const char kEnableWasmStreamingName[];
extern const char kEnableWasmStreamingDescription[]; extern const char kEnableWasmStreamingDescription[];
extern const char kEnableWebNotificationCustomLayoutsName[];
extern const char kEnableWebNotificationCustomLayoutsDescription[];
extern const char kExpensiveBackgroundTimerThrottlingName[]; extern const char kExpensiveBackgroundTimerThrottlingName[];
extern const char kExpensiveBackgroundTimerThrottlingDescription[]; extern const char kExpensiveBackgroundTimerThrottlingDescription[];
......
...@@ -261,10 +261,6 @@ const char kDisablePushApiBackgroundMode[] = "disable-push-api-background-mode"; ...@@ -261,10 +261,6 @@ const char kDisablePushApiBackgroundMode[] = "disable-push-api-background-mode";
const char kDisableSearchGeolocationDisclosure[] = const char kDisableSearchGeolocationDisclosure[] =
"disable-search-geolocation-disclosure"; "disable-search-geolocation-disclosure";
// Disables Web Notification custom layouts.
const char kDisableWebNotificationCustomLayouts[] =
"disable-web-notification-custom-layouts";
// Some tests seem to require the application to close when the last // Some tests seem to require the application to close when the last
// browser window is closed. Thus, we need a switch to force this behavior // browser window is closed. Thus, we need a switch to force this behavior
// for ChromeOS Aura, disable "zero window mode". // for ChromeOS Aura, disable "zero window mode".
...@@ -383,10 +379,6 @@ const char kEnableSiteSettings[] = "enable-site-settings"; ...@@ -383,10 +379,6 @@ const char kEnableSiteSettings[] = "enable-site-settings";
// Enables user control over muting tab audio from the tab strip. // Enables user control over muting tab audio from the tab strip.
const char kEnableTabAudioMuting[] = "enable-tab-audio-muting"; const char kEnableTabAudioMuting[] = "enable-tab-audio-muting";
// Enables Web Notification custom layouts.
const char kEnableWebNotificationCustomLayouts[] =
"enable-web-notification-custom-layouts";
// If the WebRTC logging private API is active, enables WebRTC event logging. // If the WebRTC logging private API is active, enables WebRTC event logging.
const char kEnableWebRtcEventLoggingFromExtension[] = const char kEnableWebRtcEventLoggingFromExtension[] =
"enable-webrtc-event-logging-from-extension"; "enable-webrtc-event-logging-from-extension";
......
...@@ -90,7 +90,6 @@ extern const char kDisablePrintPreview[]; ...@@ -90,7 +90,6 @@ extern const char kDisablePrintPreview[];
extern const char kDisablePromptOnRepost[]; extern const char kDisablePromptOnRepost[];
extern const char kDisablePushApiBackgroundMode[]; extern const char kDisablePushApiBackgroundMode[];
extern const char kDisableSearchGeolocationDisclosure[]; extern const char kDisableSearchGeolocationDisclosure[];
extern const char kDisableWebNotificationCustomLayouts[];
extern const char kDisableZeroBrowsersOpenForTests[]; extern const char kDisableZeroBrowsersOpenForTests[];
extern const char kDiskCacheDir[]; extern const char kDiskCacheDir[];
extern const char kDiskCacheSize[]; extern const char kDiskCacheSize[];
...@@ -122,7 +121,6 @@ extern const char kEnablePrintPreviewRegisterPromos[]; ...@@ -122,7 +121,6 @@ extern const char kEnablePrintPreviewRegisterPromos[];
extern const char kEnablePushApiBackgroundMode[]; extern const char kEnablePushApiBackgroundMode[];
extern const char kEnableSiteSettings[]; extern const char kEnableSiteSettings[];
extern const char kEnableTabAudioMuting[]; extern const char kEnableTabAudioMuting[];
extern const char kEnableWebNotificationCustomLayouts[];
extern const char kEnableWebRtcEventLoggingFromExtension[]; extern const char kEnableWebRtcEventLoggingFromExtension[];
extern const char kExtensionContentVerification[]; extern const char kExtensionContentVerification[];
extern const char kExtensionContentVerificationBootstrap[]; extern const char kExtensionContentVerificationBootstrap[];
......
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