Commit 117f07a4 authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

Add a flag to OfflineItem for showing the origin in notifications.

Background Fetch notifications should always show the origin in the
notification. In order to not break the behavior of other download
notifications, a flag was added to OfflineItem to show the origin.

Bug: 774612
Change-Id: I2f08e7e7d71a6520d02443a0961733d173242372
Reviewed-on: https://chromium-review.googlesource.com/1259011Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596265}
parent 4e3fda33
......@@ -126,8 +126,8 @@ public class DownloadBroadcastManager extends Service {
switch (action) {
case ACTION_DOWNLOAD_PAUSE:
mDownloadNotificationService.notifyDownloadPaused(entry.id, entry.fileName, true,
false, entry.isOffTheRecord, entry.isTransient, null, null, true, false,
PendingState.NOT_PENDING);
false, entry.isOffTheRecord, entry.isTransient, null, null, false, true,
false, PendingState.NOT_PENDING);
break;
case ACTION_DOWNLOAD_CANCEL:
......@@ -148,7 +148,7 @@ public class DownloadBroadcastManager extends Service {
mDownloadNotificationService.notifyDownloadPending(entry.id, entry.fileName,
entry.isOffTheRecord, entry.canDownloadWhileMetered, entry.isTransient,
null, null, true, PendingState.PENDING_NETWORK);
null, null, false, true, PendingState.PENDING_NETWORK);
break;
default:
......
......@@ -59,6 +59,7 @@ public final class DownloadInfo {
private final int mPendingState;
@FailState
private final int mFailState;
private final boolean mShouldPromoteOrigin;
private DownloadInfo(Builder builder) {
mUrl = builder.mUrl;
......@@ -97,6 +98,7 @@ public final class DownloadInfo {
mIcon = builder.mIcon;
mPendingState = builder.mPendingState;
mFailState = builder.mFailState;
mShouldPromoteOrigin = builder.mShouldPromoteOrigin;
}
public String getUrl() {
......@@ -226,6 +228,10 @@ public final class DownloadInfo {
return mFailState;
}
public boolean getShouldPromoteOrigin() {
return mShouldPromoteOrigin;
}
/**
* Helper method to build a {@link DownloadInfo} from an {@link OfflineItem}.
* @param item The {@link OfflineItem} to mimic.
......@@ -279,6 +285,7 @@ public final class DownloadInfo {
.setIcon(visuals == null ? null : visuals.icon)
.setPendingState(item.pendingState)
.setFailState(item.failState)
.setShouldPromoteOrigin(item.promoteOrigin)
.build();
}
......@@ -306,6 +313,7 @@ public final class DownloadInfo {
offlineItem.progress = downloadInfo.getProgress();
offlineItem.isDangerous = downloadInfo.getIsDangerous();
offlineItem.failState = downloadInfo.getFailState();
offlineItem.promoteOrigin = downloadInfo.getShouldPromoteOrigin();
switch (downloadInfo.state()) {
case DownloadState.IN_PROGRESS:
offlineItem.state = downloadInfo.isPaused() ? OfflineItemState.PAUSED
......@@ -389,6 +397,7 @@ public final class DownloadInfo {
private int mPendingState;
@FailState
private int mFailState;
private boolean mShouldPromoteOrigin;
public Builder setUrl(String url) {
mUrl = url;
......@@ -545,6 +554,11 @@ public final class DownloadInfo {
return this;
}
public Builder setShouldPromoteOrigin(boolean shouldPromoteOrigin) {
mShouldPromoteOrigin = shouldPromoteOrigin;
return this;
}
public DownloadInfo build() {
return new DownloadInfo(this);
}
......@@ -584,7 +598,8 @@ public final class DownloadInfo {
.setIsParallelDownload(downloadInfo.getIsParallelDownload())
.setIcon(downloadInfo.getIcon())
.setPendingState(downloadInfo.getPendingState())
.setFailState(downloadInfo.getFailState());
.setFailState(downloadInfo.getFailState())
.setShouldPromoteOrigin(downloadInfo.getShouldPromoteOrigin());
return builder;
}
}
......
......@@ -294,7 +294,8 @@ public final class DownloadNotificationFactory {
setSubText(builder,
context.getResources().getString(
R.string.download_notification_incognito_subtext));
} else if (!TextUtils.isEmpty(downloadUpdate.getOriginalUrl())) {
} else if (downloadUpdate.getShouldPromoteOrigin()
&& !TextUtils.isEmpty(downloadUpdate.getOriginalUrl())) {
// Always show the origin URL if available (for normal profiles).
String formattedUrl = UrlFormatter.formatUrlForSecurityDisplayOmitScheme(
downloadUpdate.getOriginalUrl());
......
......@@ -27,6 +27,7 @@ public final class DownloadUpdate {
private final boolean mIsTransient;
private final int mNotificationId;
private final String mOriginalUrl;
private final boolean mShouldPromoteOrigin;
private final Progress mProgress;
private final String mReferrer;
private final long mStartTime;
......@@ -48,6 +49,7 @@ public final class DownloadUpdate {
this.mIsTransient = builder.mIsTransient;
this.mNotificationId = builder.mNotificationId;
this.mOriginalUrl = builder.mOriginalUrl;
this.mShouldPromoteOrigin = builder.mShouldPromoteOrigin;
this.mProgress = builder.mProgress;
this.mReferrer = builder.mReferrer;
this.mStartTime = builder.mStartTime;
......@@ -106,6 +108,10 @@ public final class DownloadUpdate {
return mOriginalUrl;
}
public boolean getShouldPromoteOrigin() {
return mShouldPromoteOrigin;
}
public Progress getProgress() {
return mProgress;
}
......@@ -153,6 +159,7 @@ public final class DownloadUpdate {
private boolean mIsTransient;
private int mNotificationId = -1;
private String mOriginalUrl;
private boolean mShouldPromoteOrigin;
private Progress mProgress;
private String mReferrer;
private long mStartTime;
......@@ -217,6 +224,11 @@ public final class DownloadUpdate {
return this;
}
public Builder setShouldPromoteOrigin(boolean shouldPromoteOrigin) {
this.mShouldPromoteOrigin = shouldPromoteOrigin;
return this;
}
public Builder setProgress(Progress progress) {
this.mProgress = progress;
return this;
......
......@@ -233,13 +233,14 @@ public class SystemDownloadNotifier2 implements DownloadNotifier {
info.getFileName(), info.getProgress(), info.getBytesReceived(),
info.getTimeRemainingInMillis(), notificationInfo.mStartTime,
info.isOffTheRecord(), notificationInfo.mCanDownloadWhileMetered,
info.getIsTransient(), info.getIcon(), info.getOriginalUrl());
info.getIsTransient(), info.getIcon(), info.getOriginalUrl(),
info.getShouldPromoteOrigin());
break;
case NotificationType.PAUSED:
getDownloadNotificationService().notifyDownloadPaused(info.getContentId(),
info.getFileName(), true, false, info.isOffTheRecord(),
info.getIsTransient(), info.getIcon(), info.getOriginalUrl(), false, false,
info.getPendingState());
info.getIsTransient(), info.getIcon(), info.getOriginalUrl(),
info.getShouldPromoteOrigin(), false, false, info.getPendingState());
break;
case NotificationType.SUCCEEDED:
final int notificationId =
......@@ -247,7 +248,8 @@ public class SystemDownloadNotifier2 implements DownloadNotifier {
info.getContentId(), info.getFilePath(), info.getFileName(),
notificationInfo.mSystemDownloadId, info.isOffTheRecord(),
notificationInfo.mIsSupportedMimeType, info.getIsOpenable(),
info.getIcon(), info.getOriginalUrl(), info.getReferrer(),
info.getIcon(), info.getOriginalUrl(),
info.getShouldPromoteOrigin(), info.getReferrer(),
info.getBytesTotalSize());
if (info.getIsOpenable()) {
......@@ -259,13 +261,14 @@ public class SystemDownloadNotifier2 implements DownloadNotifier {
case NotificationType.FAILED:
getDownloadNotificationService().notifyDownloadFailed(info.getContentId(),
info.getFileName(), info.getIcon(), info.getOriginalUrl(),
info.isOffTheRecord(), info.getFailState());
info.getShouldPromoteOrigin(), info.isOffTheRecord(), info.getFailState());
break;
case NotificationType.INTERRUPTED:
getDownloadNotificationService().notifyDownloadPaused(info.getContentId(),
info.getFileName(), info.isResumable(), notificationInfo.mIsAutoResumable,
info.isOffTheRecord(), info.getIsTransient(), info.getIcon(),
info.getOriginalUrl(), false, false, notificationInfo.mPendingState);
info.getOriginalUrl(), info.getShouldPromoteOrigin(), false, false,
notificationInfo.mPendingState);
break;
}
}
......
......@@ -125,7 +125,7 @@ public class DownloadNotificationService2Test {
// Download is in-progress.
mDownloadNotificationService.notifyDownloadProgress(ID1, "test",
new Progress(1, 100L, OfflineItemProgressUnit.PERCENTAGE), 100L, 1L, 1L, true, true,
false, null, null);
false, null, null, false);
mDownloadForegroundServiceManager.onServiceConnected();
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
......@@ -136,7 +136,7 @@ public class DownloadNotificationService2Test {
// Download is paused.
mDownloadNotificationService.notifyDownloadPaused(ID1, "test", true /* isResumable*/,
false /* isAutoResumable */, true, false, null, null, false, false,
false /* isAutoResumable */, true, false, null, null, false, false, false,
PendingState.NOT_PENDING);
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
......@@ -147,7 +147,7 @@ public class DownloadNotificationService2Test {
// Download is again in-progress.
mDownloadNotificationService.notifyDownloadProgress(ID1, "test",
new Progress(20, 100L, OfflineItemProgressUnit.PERCENTAGE), 100L, 1L, 1L, true,
true, false, null, null);
true, false, null, null, false);
mDownloadForegroundServiceManager.onServiceConnected();
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
......@@ -157,7 +157,7 @@ public class DownloadNotificationService2Test {
// Download is successful.
mDownloadNotificationService.notifyDownloadSuccessful(
ID1, "", "test", 1L, true, true, true, null, "", "", 0);
ID1, "", "test", 1L, true, true, true, null, "", false, "", 0);
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
assertFalse(mDownloadForegroundServiceManager.mDownloadUpdateQueue.containsKey(
notificationId1));
......@@ -172,7 +172,7 @@ public class DownloadNotificationService2Test {
// Download is in-progress.
mDownloadNotificationService.notifyDownloadProgress(ID1, "test",
new Progress(1, 100L, OfflineItemProgressUnit.PERCENTAGE), 100L, 1L, 1L, true, true,
false, null, null);
false, null, null, false);
mDownloadForegroundServiceManager.onServiceConnected();
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
......@@ -183,7 +183,7 @@ public class DownloadNotificationService2Test {
// Download is interrupted and now is pending.
mDownloadNotificationService.notifyDownloadPaused(ID1, "test", true /* isResumable */,
true /* isAutoResumable */, true, false, null, null, false, false,
true /* isAutoResumable */, true, false, null, null, false, false, false,
PendingState.PENDING_NETWORK);
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
assertTrue(mDownloadForegroundServiceManager.mDownloadUpdateQueue.containsKey(
......@@ -207,7 +207,7 @@ public class DownloadNotificationService2Test {
// Download is in-progress.
mDownloadNotificationService.notifyDownloadProgress(ID1, "test",
new Progress(1, 100L, OfflineItemProgressUnit.PERCENTAGE), 100L, 1L, 1L, true, true,
false, null, null);
false, null, null, false);
mDownloadForegroundServiceManager.onServiceConnected();
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
......@@ -218,7 +218,7 @@ public class DownloadNotificationService2Test {
// Download is interrupted but because it is not resumable, fails.
mDownloadNotificationService.notifyDownloadPaused(ID1, "test", false /* isResumable*/,
true /* isAutoResumable */, true, false, null, null, false, false,
true /* isAutoResumable */, true, false, null, null, false, false, false,
PendingState.PENDING_NETWORK);
assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
assertFalse(mDownloadForegroundServiceManager.mDownloadUpdateQueue.containsKey(
......
......@@ -64,13 +64,14 @@ public class MockDownloadNotificationService2 extends DownloadNotificationServic
public int notifyDownloadSuccessful(final ContentId id, final String filePath,
final String fileName, final long systemDownloadId, final boolean isOffTheRecord,
final boolean isSupportedMimeType, final boolean isOpenable, final Bitmap icon,
final String originalUrl, final String referrer, final long totalBytes) {
final String originalUrl, final boolean shouldPromoteOrigin, final String referrer,
final long totalBytes) {
return ThreadUtils.runOnUiThreadBlockingNoException(
()
-> MockDownloadNotificationService2.super.notifyDownloadSuccessful(id,
filePath, fileName, systemDownloadId, isOffTheRecord,
isSupportedMimeType, isOpenable, icon, originalUrl, referrer,
totalBytes));
isSupportedMimeType, isOpenable, icon, originalUrl,
shouldPromoteOrigin, referrer, totalBytes));
}
@Override
......@@ -78,34 +79,36 @@ public class MockDownloadNotificationService2 extends DownloadNotificationServic
final Progress progress, final long bytesReceived, final long timeRemainingInMillis,
final long startTime, final boolean isOffTheRecord,
final boolean canDownloadWhileMetered, final boolean isTransient, final Bitmap icon,
final String originalUrl) {
final String originalUrl, final boolean shouldPromoteOrigin) {
ThreadUtils.runOnUiThreadBlocking(
()
-> MockDownloadNotificationService2.super.notifyDownloadProgress(id,
fileName, progress, bytesReceived, timeRemainingInMillis, startTime,
isOffTheRecord, canDownloadWhileMetered, isTransient, icon,
originalUrl));
originalUrl, shouldPromoteOrigin));
}
@Override
void notifyDownloadPaused(ContentId id, String fileName, boolean isResumable,
boolean isAutoResumable, boolean isOffTheRecord, boolean isTransient, Bitmap icon,
final String originalUrl, boolean hasUserGesture, boolean forceRebuild,
@PendingState int pendingState) {
final String originalUrl, final boolean shouldPromoteOrigin, boolean hasUserGesture,
boolean forceRebuild, @PendingState int pendingState) {
ThreadUtils.runOnUiThreadBlocking(
()
-> MockDownloadNotificationService2.super.notifyDownloadPaused(id, fileName,
isResumable, isAutoResumable, isOffTheRecord, isTransient, icon,
originalUrl, hasUserGesture, forceRebuild, pendingState));
originalUrl, shouldPromoteOrigin, hasUserGesture, forceRebuild,
pendingState));
}
@Override
public void notifyDownloadFailed(final ContentId id, final String fileName, final Bitmap icon,
final String originalUrl, boolean isOffTheRecord, @FailState int failState) {
final String originalUrl, final boolean shouldPromoteOrigin, boolean isOffTheRecord,
@FailState int failState) {
ThreadUtils.runOnUiThreadBlocking(
()
-> MockDownloadNotificationService2.super.notifyDownloadFailed(
id, fileName, icon, originalUrl, isOffTheRecord, failState));
-> MockDownloadNotificationService2.super.notifyDownloadFailed(id, fileName,
icon, originalUrl, shouldPromoteOrigin, isOffTheRecord, failState));
}
@Override
......
......@@ -105,7 +105,7 @@ void BackgroundFetchDelegateImpl::JobDetails::UpdateOfflineItem() {
offline_items_collection::OfflineItemProgressUnit::PERCENTAGE;
offline_item.title = fetch_description->title;
offline_item.promote_origin = true;
offline_item.is_transient = true;
offline_item.is_resumable = true;
......
......@@ -241,8 +241,7 @@ GURL OfflineItemModel::GetOriginalURL() const {
}
bool OfflineItemModel::ShouldPromoteOrigin() const {
// TODO(crbug.com/774612): Add a field in OfflineItem to make this decision.
return true;
return offline_item_ && offline_item_->promote_origin;
}
#if !defined(OS_ANDROID)
......
......@@ -75,6 +75,7 @@ public class OfflineItem implements Cloneable {
public boolean isSuggested;
public boolean isAccelerated;
public boolean refreshVisuals;
public boolean promoteOrigin;
// Content Metadata.
public long totalSizeBytes;
......@@ -121,6 +122,7 @@ public class OfflineItem implements Cloneable {
clone.isSuggested = isSuggested;
clone.isAccelerated = isAccelerated;
clone.refreshVisuals = refreshVisuals;
clone.promoteOrigin = promoteOrigin;
clone.totalSizeBytes = totalSizeBytes;
clone.externallyRemoved = externallyRemoved;
clone.creationTimeMs = creationTimeMs;
......
......@@ -45,10 +45,10 @@ public final class OfflineItemBridge {
private static OfflineItem createOfflineItemAndMaybeAddToList(ArrayList<OfflineItem> list,
String nameSpace, String id, String title, String description,
@OfflineItemFilter int filter, boolean isTransient, boolean isSuggested,
boolean isAccelerated, boolean refreshVisuals, long totalSizeBytes,
boolean externallyRemoved, long creationTimeMs, long lastAccessedTimeMs,
boolean isOpenable, String filePath, String mimeType, String pageUrl,
String originalUrl, boolean isOffTheRecord, @OfflineItemState int state,
boolean isAccelerated, boolean refreshVisuals, boolean promoteOrigin,
long totalSizeBytes, boolean externallyRemoved, long creationTimeMs,
long lastAccessedTimeMs, boolean isOpenable, String filePath, String mimeType,
String pageUrl, String originalUrl, boolean isOffTheRecord, @OfflineItemState int state,
@PendingState int pendingState, boolean isResumable, boolean allowMetered,
long receivedBytes, long progressValue, long progressMax,
@OfflineItemProgressUnit int progressUnit, long timeRemainingMs, boolean isDangerous) {
......@@ -62,6 +62,7 @@ public final class OfflineItemBridge {
item.isSuggested = isSuggested;
item.isAccelerated = isAccelerated;
item.refreshVisuals = refreshVisuals;
item.promoteOrigin = promoteOrigin;
item.totalSizeBytes = totalSizeBytes;
item.externallyRemoved = externallyRemoved;
item.creationTimeMs = creationTimeMs;
......
......@@ -33,10 +33,10 @@ JNI_OfflineItemBridge_createOfflineItemAndMaybeAddToList(
ConvertUTF8ToJavaString(env, item.title),
ConvertUTF8ToJavaString(env, item.description),
static_cast<jint>(item.filter), item.is_transient, item.is_suggested,
item.is_accelerated, item.refresh_visuals, item.total_size_bytes,
item.externally_removed, item.creation_time.ToJavaTime(),
item.last_accessed_time.ToJavaTime(), item.is_openable,
ConvertUTF8ToJavaString(env, item.file_path.value()),
item.is_accelerated, item.refresh_visuals, item.promote_origin,
item.total_size_bytes, item.externally_removed,
item.creation_time.ToJavaTime(), item.last_accessed_time.ToJavaTime(),
item.is_openable, ConvertUTF8ToJavaString(env, item.file_path.value()),
ConvertUTF8ToJavaString(env, item.mime_type),
ConvertUTF8ToJavaString(env, item.page_url.spec()),
ConvertUTF8ToJavaString(env, item.original_url.spec()),
......
......@@ -44,6 +44,7 @@ OfflineItem::OfflineItem()
is_suggested(false),
is_accelerated(false),
refresh_visuals(false),
promote_origin(false),
total_size_bytes(0),
externally_removed(false),
is_openable(false),
......@@ -73,6 +74,7 @@ bool OfflineItem::operator==(const OfflineItem& offline_item) const {
is_suggested == offline_item.is_suggested &&
is_accelerated == offline_item.is_accelerated &&
refresh_visuals == offline_item.refresh_visuals &&
promote_origin == offline_item.promote_origin &&
total_size_bytes == offline_item.total_size_bytes &&
externally_removed == offline_item.externally_removed &&
creation_time == offline_item.creation_time &&
......
......@@ -119,6 +119,9 @@ struct OfflineItem {
// Whether there are new visuals available.
bool refresh_visuals;
// Whether the origin should be displayed.
bool promote_origin;
// TODO(dtrainor): Build out custom per-item icon support.
// Content Metadata.
......
......@@ -23,6 +23,7 @@ std::ostream& operator<<(std::ostream& os, const OfflineItem& item) {
os << ", is_suggested: " << item.is_suggested;
os << ", is_accelerated: " << item.is_accelerated;
os << ", refresh_visuals: " << item.refresh_visuals;
os << ", promote_origin: " << item.promote_origin;
os << ", total_size_bytes: " << item.total_size_bytes;
os << ", externally_removed: " << item.externally_removed;
os << ", creation_time: " << item.creation_time;
......
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