Commit 754477b8 authored by rhalavati@chromium.org's avatar rhalavati@chromium.org Committed by Commit Bot

Remove user data from Android incognito media notifications.

Android notifications are stored in Android logs. User data are removed
from media notifications in incognito mode to avoid leaking this data.
The change is behind a disabled by default switch and is previously
reviewed in: crrev.com/c/586868

Bug: 629887
Change-Id: Icbe2ba7b7b670e59f78dd53e282bc326631c9a9c
Reviewed-on: https://chromium-review.googlesource.com/c/1196367
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596541}
parent 2cf21db7
...@@ -42,6 +42,7 @@ import org.chromium.base.VisibleForTesting; ...@@ -42,6 +42,7 @@ import org.chromium.base.VisibleForTesting;
import org.chromium.blink.mojom.MediaSessionAction; import org.chromium.blink.mojom.MediaSessionAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.AppHooks; import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.notifications.ChromeNotificationBuilder; import org.chromium.chrome.browser.notifications.ChromeNotificationBuilder;
import org.chromium.chrome.browser.notifications.NotificationBuilderFactory; import org.chromium.chrome.browser.notifications.NotificationBuilderFactory;
import org.chromium.chrome.browser.notifications.NotificationConstants; import org.chromium.chrome.browser.notifications.NotificationConstants;
...@@ -1070,11 +1071,15 @@ public class MediaNotificationManager { ...@@ -1070,11 +1071,15 @@ public class MediaNotificationManager {
private void setMediaStyleLayoutForNotificationBuilder(ChromeNotificationBuilder builder) { private void setMediaStyleLayoutForNotificationBuilder(ChromeNotificationBuilder builder) {
setMediaStyleNotificationText(builder); setMediaStyleNotificationText(builder);
// Notifications in incognito shouldn't show an icon to avoid leaking information.
boolean hideUserData = mMediaNotificationInfo.isPrivate
&& ChromeFeatureList.isEnabled(
ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
if (!mMediaNotificationInfo.supportsPlayPause()) { if (!mMediaNotificationInfo.supportsPlayPause()) {
// Non-playback (Cast) notification will not use MediaStyle, so not // Non-playback (Cast) notification will not use MediaStyle, so not
// setting the large icon is fine. // setting the large icon is fine.
builder.setLargeIcon(null); builder.setLargeIcon(null);
} else if (mMediaNotificationInfo.notificationLargeIcon != null) { } else if (mMediaNotificationInfo.notificationLargeIcon != null && !hideUserData) {
builder.setLargeIcon(mMediaNotificationInfo.notificationLargeIcon); builder.setLargeIcon(mMediaNotificationInfo.notificationLargeIcon);
} else if (!isRunningAtLeastN()) { } else if (!isRunningAtLeastN()) {
if (mDefaultNotificationLargeIcon == null if (mDefaultNotificationLargeIcon == null
...@@ -1133,6 +1138,21 @@ public class MediaNotificationManager { ...@@ -1133,6 +1138,21 @@ public class MediaNotificationManager {
} }
private void setMediaStyleNotificationText(ChromeNotificationBuilder builder) { private void setMediaStyleNotificationText(ChromeNotificationBuilder builder) {
boolean hideUserData = mMediaNotificationInfo.isPrivate
&& ChromeFeatureList.isEnabled(
ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
if (hideUserData) {
// Notifications in incognito shouldn't show what is playing to avoid leaking
// information.
builder.setContentTitle(
getContext().getResources().getString(R.string.media_notification_incognito));
if (isRunningAtLeastN()) {
builder.setSubText(
getContext().getResources().getString(R.string.notification_incognito_tab));
}
return;
}
builder.setContentTitle(mMediaNotificationInfo.metadata.getTitle()); builder.setContentTitle(mMediaNotificationInfo.metadata.getTitle());
String artistAndAlbumText = getArtistAndAlbumText(mMediaNotificationInfo.metadata); String artistAndAlbumText = getArtistAndAlbumText(mMediaNotificationInfo.metadata);
if (isRunningAtLeastN() || !artistAndAlbumText.isEmpty()) { if (isRunningAtLeastN() || !artistAndAlbumText.isEmpty()) {
......
...@@ -3309,6 +3309,12 @@ To obtain new licenses, connect to the internet and play your downloaded content ...@@ -3309,6 +3309,12 @@ To obtain new licenses, connect to the internet and play your downloaded content
<message name="IDS_MEDIA_NOTIFICATION_LINK_TEXT" desc="Url of the current tab. The notification will display this text for the user to identify the tab to return to."> <message name="IDS_MEDIA_NOTIFICATION_LINK_TEXT" desc="Url of the current tab. The notification will display this text for the user to identify the tab to return to.">
Touch to return to <ph name="URL_OF_THE_CURRENT_TAB">%1$s<ex>https://apprtc.appspot.com</ex></ph> Touch to return to <ph name="URL_OF_THE_CURRENT_TAB">%1$s<ex>https://apprtc.appspot.com</ex></ph>
</message> </message>
<message name="IDS_MEDIA_NOTIFICATION_INCOGNITO" desc="Text used as a placeholder for media notifications content, when notification is shown from incognito tab.">
A site is playing media
</message>
<message name="IDS_NOTIFICATION_INCOGNITO_TAB" desc="Text used as notifications source when the notification is from incognito tabs.">
Incognito tab
</message>
<message name="IDS_SCREEN_CAPTURE_NOTIFICATION_TEXT" desc="Text to be shown as a notification when screen capture is in progress."> <message name="IDS_SCREEN_CAPTURE_NOTIFICATION_TEXT" desc="Text to be shown as a notification when screen capture is in progress.">
<ph name="URL_OF_THE_CURRENT_TAB">%1$s<ex>https://apprtc.appspot.com</ex></ph> is sharing your screen <ph name="URL_OF_THE_CURRENT_TAB">%1$s<ex>https://apprtc.appspot.com</ex></ph> is sharing your screen
</message> </message>
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.media.ui; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.media.ui;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
...@@ -24,6 +25,7 @@ import org.robolectric.shadows.ShadowNotification; ...@@ -24,6 +25,7 @@ import org.robolectric.shadows.ShadowNotification;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.content_public.common.MediaMetadata; import org.chromium.content_public.common.MediaMetadata;
/** /**
...@@ -48,11 +50,23 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM ...@@ -48,11 +50,23 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM
ShadowNotification shadowNotification = Shadows.shadowOf(notification); ShadowNotification shadowNotification = Shadows.shadowOf(notification);
assertEquals("title", shadowNotification.getContentTitle()); boolean userDataIsHidden = info.isPrivate
assertEquals("artist - album", shadowNotification.getContentText()); && ChromeFeatureList.isEnabled(
if (hasNApis()) { ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
assertEquals("https://example.com/", if (userDataIsHidden) {
notification.extras.getString(Notification.EXTRA_SUB_TEXT)); assertNotEquals("title", shadowNotification.getContentTitle());
assertNotEquals("artist - album", shadowNotification.getContentText());
if (hasNApis()) {
assertNull(notification.extras.getString(Notification.EXTRA_SUB_TEXT));
}
} else {
assertEquals("title", shadowNotification.getContentTitle());
assertEquals("artist - album", shadowNotification.getContentText());
if (hasNApis()) {
assertEquals("https://example.com/",
notification.extras.getString(Notification.EXTRA_SUB_TEXT));
}
} }
} }
...@@ -68,8 +82,16 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM ...@@ -68,8 +82,16 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM
ShadowNotification shadowNotification = Shadows.shadowOf(notification); ShadowNotification shadowNotification = Shadows.shadowOf(notification);
assertEquals(info.metadata.getTitle(), shadowNotification.getContentTitle()); boolean userDataIsHidden = info.isPrivate
assertEquals(info.origin, shadowNotification.getContentText()); && ChromeFeatureList.isEnabled(
ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
if (userDataIsHidden) {
assertNotEquals(info.metadata.getTitle(), shadowNotification.getContentTitle());
assertNull(shadowNotification.getContentText());
} else {
assertEquals(info.metadata.getTitle(), shadowNotification.getContentTitle());
assertEquals(info.origin, shadowNotification.getContentText());
}
if (hasNApis()) { if (hasNApis()) {
assertEquals(null, notification.extras.getString(Notification.EXTRA_SUB_TEXT)); assertEquals(null, notification.extras.getString(Notification.EXTRA_SUB_TEXT));
} }
...@@ -87,10 +109,23 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM ...@@ -87,10 +109,23 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM
ShadowNotification shadowNotification = Shadows.shadowOf(notification); ShadowNotification shadowNotification = Shadows.shadowOf(notification);
assertEquals(info.metadata.getTitle(), shadowNotification.getContentTitle()); boolean userDataIsHidden = info.isPrivate
assertEquals("", shadowNotification.getContentText()); && ChromeFeatureList.isEnabled(
if (hasNApis()) { ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
assertEquals(info.origin, notification.extras.getString(Notification.EXTRA_SUB_TEXT)); if (userDataIsHidden) {
assertNotEquals(info.metadata.getTitle(), shadowNotification.getContentTitle());
assertNull(shadowNotification.getContentText());
if (hasNApis()) {
assertNotEquals(
info.origin, notification.extras.getString(Notification.EXTRA_SUB_TEXT));
}
} else {
assertEquals(info.metadata.getTitle(), shadowNotification.getContentTitle());
assertEquals("", shadowNotification.getContentText());
if (hasNApis()) {
assertEquals(
info.origin, notification.extras.getString(Notification.EXTRA_SUB_TEXT));
}
} }
} }
...@@ -103,7 +138,14 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM ...@@ -103,7 +138,14 @@ public class MediaNotificationManagerNotificationTest extends MediaNotificationM
Notification notification = updateNotificationBuilderAndBuild(info); Notification notification = updateNotificationBuilderAndBuild(info);
if (hasNApis()) { if (hasNApis()) {
assertTrue(largeIcon.sameAs(iconToBitmap(notification.getLargeIcon()))); boolean userDataIsHidden = info.isPrivate
&& ChromeFeatureList.isEnabled(
ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS);
if (userDataIsHidden) {
assertNull(notification.getLargeIcon());
} else {
assertTrue(largeIcon.sameAs(iconToBitmap(notification.getLargeIcon())));
}
} }
} }
......
...@@ -35,10 +35,13 @@ import org.chromium.base.CommandLine; ...@@ -35,10 +35,13 @@ import org.chromium.base.CommandLine;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.AppHooks; import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.AppHooksImpl; import org.chromium.chrome.browser.AppHooksImpl;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.media.ui.MediaNotificationManager.ListenerService; import org.chromium.chrome.browser.media.ui.MediaNotificationManager.ListenerService;
import org.chromium.chrome.browser.notifications.NotificationUmaTracker; import org.chromium.chrome.browser.notifications.NotificationUmaTracker;
import org.chromium.content_public.common.MediaMetadata; import org.chromium.content_public.common.MediaMetadata;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
/** /**
...@@ -137,6 +140,11 @@ public class MediaNotificationManagerTestBase { ...@@ -137,6 +140,11 @@ public class MediaNotificationManagerTestBase {
// Init the command line to avoid assertion failure in |SysUtils#isLowEndDevice()|. // Init the command line to avoid assertion failure in |SysUtils#isLowEndDevice()|.
CommandLine.init(null); CommandLine.init(null);
// Init ChromeFeaturesList to avoid assertion failure in
// MediaNotificationManagerNotificationTest.
Map<String, Boolean> testFeatures = new HashMap<>();
testFeatures.put(ChromeFeatureList.HIDE_USER_DATA_FROM_INCOGNITO_NOTIFICATIONS, true);
ChromeFeatureList.setTestFeatures(testFeatures);
} }
@After @After
......
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