Display formatted origin at top of download notification when provided

The origin will be set if provided to the OfflineItem a non-incognito profile.
screenshot: https://lh3.googleusercontent.com/-BCLL-YuqB28/W6j_XbF542I/AAAAAAAABLA/Mq7jMbOdDdAPPZh-rfFVbDx7JIfLTKlEACL0BGAYYCw/h261/2018-09-24.png

Bug: 774612
Change-Id: I34aa8fa51195443a242e4bd6e068d2d408737fbb
Reviewed-on: https://chromium-review.googlesource.com/1240304
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596260}

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastManager.java

@@ -126,7 +126,7 @@ public class DownloadBroadcastManager extends Service {
         switch (action) {
             case ACTION_DOWNLOAD_PAUSE:
                 mDownloadNotificationService.notifyDownloadPaused(entry.id, entry.fileName, true,
-                        false, entry.isOffTheRecord, entry.isTransient, null, true, false,
+                        false, entry.isOffTheRecord, entry.isTransient, null, null, true, false,
                         PendingState.NOT_PENDING);
                 break;
 
@@ -148,7 +148,7 @@ public class DownloadBroadcastManager extends Service {
 
                 mDownloadNotificationService.notifyDownloadPending(entry.id, entry.fileName,
                         entry.isOffTheRecord, entry.canDownloadWhileMetered, entry.isTransient,
-                        null, true, PendingState.PENDING_NETWORK);
+                        null, null, true, PendingState.PENDING_NETWORK);
                 break;
 
             default:

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java

@@ -27,6 +27,7 @@ import android.content.Intent;
 import android.os.Build;
 import android.os.Bundle;
 import android.support.v4.app.NotificationCompat;
+import android.text.TextUtils;
 
 import com.google.ipc.invalidation.util.Preconditions;
 
@@ -36,9 +37,11 @@ import org.chromium.chrome.browser.notifications.ChromeNotificationBuilder;
 import org.chromium.chrome.browser.notifications.NotificationBuilderFactory;
 import org.chromium.chrome.browser.notifications.NotificationConstants;
 import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions;
+import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.components.offline_items_collection.ContentId;
 import org.chromium.components.offline_items_collection.LegacyHelpers;
 import org.chromium.components.offline_items_collection.PendingState;
+import org.chromium.components.url_formatter.UrlFormatter;
 
 /**
  * Creates and updates notifications related to downloads.
@@ -47,6 +50,10 @@ public final class DownloadNotificationFactory {
     // Limit file name to 25 characters. TODO(qinmin): use different limit for different devices?
     public static final int MAX_FILE_NAME_LENGTH = 25;
 
+    // Limit the origin length so that the eTLD+1 cannot be hidden. If the origin exceeds this
+    // length the eTLD+1 is extracted and shown.
+    public static final int MAX_ORIGIN_LENGTH = 40;
+
     /**
      * Builds a downloads notification based on the status of the download and its information. All
      * changes to this function should consider the difference between normal profile and off the
@@ -282,11 +289,22 @@ public final class DownloadNotificationFactory {
                             downloadHomeIntent, PendingIntent.FLAG_UPDATE_CURRENT));
         }
 
-        // A sub text to inform the users that they are using incognito mode.
         if (downloadUpdate.getIsOffTheRecord()) {
+            // A sub text to inform the users that they are using incognito mode.
             setSubText(builder,
                     context.getResources().getString(
                             R.string.download_notification_incognito_subtext));
+        } else if (!TextUtils.isEmpty(downloadUpdate.getOriginalUrl())) {
+            // Always show the origin URL if available (for normal profiles).
+            String formattedUrl = UrlFormatter.formatUrlForSecurityDisplayOmitScheme(
+                    downloadUpdate.getOriginalUrl());
+
+            if (formattedUrl.length() > MAX_ORIGIN_LENGTH) {
+                // The origin is too long. Strip down to eTLD+1.
+                formattedUrl = UrlUtilities.getDomainAndRegistry(
+                        downloadUpdate.getOriginalUrl(), false /* includePrivateRegistries */);
+            }
+            setSubText(builder, formattedUrl);
         }
 
         return builder.build();

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java

@@ -168,13 +168,14 @@ public class DownloadNotificationService2 {
      * @param isTransient             Whether or not clicking on the download should launch
      *                                downloads home.
      * @param icon                    A {@link Bitmap} to be used as the large icon for display.
+     * @param originalUrl             The original url of the downloaded file.
      */
     @VisibleForTesting
     public void notifyDownloadProgress(ContentId id, String fileName, Progress progress,
             long bytesReceived, long timeRemainingInMillis, long startTime, boolean isOffTheRecord,
-            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon) {
+            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon, String originalUrl) {
         updateActiveDownloadNotification(id, fileName, progress, timeRemainingInMillis, startTime,
-                isOffTheRecord, canDownloadWhileMetered, isTransient, icon, false,
+                isOffTheRecord, canDownloadWhileMetered, isTransient, icon, originalUrl, false,
                 PendingState.NOT_PENDING);
     }
 
@@ -187,14 +188,15 @@ public class DownloadNotificationService2 {
      * @param isTransient             Whether or not clicking on the download should launch
      *                                downloads home.
      * @param icon                    A {@link Bitmap} to be used as the large icon for display.
+     * @param originalUrl             The original url of the downloaded file.
      * @param pendingState            Reason download is pending.
      */
     void notifyDownloadPending(ContentId id, String fileName, boolean isOffTheRecord,
-            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon,
+            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon, String originalUrl,
             boolean hasUserGesture, @PendingState int pendingState) {
         updateActiveDownloadNotification(id, fileName, Progress.createIndeterminateProgress(), 0, 0,
-                isOffTheRecord, canDownloadWhileMetered, isTransient, icon, hasUserGesture,
-                pendingState);
+                isOffTheRecord, canDownloadWhileMetered, isTransient, icon, originalUrl,
+                hasUserGesture, pendingState);
     }
 
     /**
@@ -211,11 +213,12 @@ public class DownloadNotificationService2 {
      * @param isTransient             Whether or not clicking on the download should launch
      *                                downloads home.
      * @param icon                    A {@link Bitmap} to be used as the large icon for display.
+     * @param originalUrl             The original url of the downloaded file.
      * @param pendingState            Reason download is pending.
      */
     private void updateActiveDownloadNotification(ContentId id, String fileName, Progress progress,
             long timeRemainingInMillis, long startTime, boolean isOffTheRecord,
-            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon,
+            boolean canDownloadWhileMetered, boolean isTransient, Bitmap icon, String originalUrl,
             boolean hasUserGesture, @PendingState int pendingState) {
         int notificationId = getNotificationId(id);
         Context context = ContextUtils.getApplicationContext();
@@ -229,6 +232,7 @@ public class DownloadNotificationService2 {
                                                 .setIsOffTheRecord(isOffTheRecord)
                                                 .setIsTransient(isTransient)
                                                 .setIcon(icon)
+                                                .setOriginalUrl(originalUrl)
                                                 .setNotificationId(notificationId)
                                                 .setPendingState(pendingState)
                                                 .build();
@@ -289,18 +293,21 @@ public class DownloadNotificationService2 {
      * @param isOffTheRecord  Whether the download is off the record.
      * @param isTransient     Whether or not clicking on the download should launch downloads home.
      * @param icon            A {@link Bitmap} to be used as the large icon for display.
+     * @param originalUrl     The original url of the downloaded file.
      * @param forceRebuild    Whether the notification was forcibly relaunched.
      * @param pendingState    Reason download is pending.
      */
     @VisibleForTesting
     void notifyDownloadPaused(ContentId id, String fileName, boolean isResumable,
             boolean isAutoResumable, boolean isOffTheRecord, boolean isTransient, Bitmap icon,
-            boolean hasUserGesture, boolean forceRebuild, @PendingState int pendingState) {
+            String originalUrl, boolean hasUserGesture, boolean forceRebuild,
+            @PendingState int pendingState) {
         DownloadSharedPreferenceEntry entry =
                 mDownloadSharedPreferenceHelper.getDownloadSharedPreferenceEntry(id);
         if (!isResumable) {
             // TODO(cmsy): Use correct FailState.
-            notifyDownloadFailed(id, fileName, icon, isOffTheRecord, FailState.CANNOT_DOWNLOAD);
+            notifyDownloadFailed(
+                    id, fileName, icon, originalUrl, isOffTheRecord, FailState.CANNOT_DOWNLOAD);
             return;
         }
         // If download is already paused, do nothing.
@@ -309,7 +316,7 @@ public class DownloadNotificationService2 {
         // If download is interrupted due to network disconnection, show download pending state.
         if (isAutoResumable || pendingState != PendingState.NOT_PENDING) {
             notifyDownloadPending(id, fileName, isOffTheRecord, canDownloadWhileMetered,
-                    isTransient, icon, hasUserGesture, pendingState);
+                    isTransient, icon, originalUrl, hasUserGesture, pendingState);
             stopTrackingInProgressDownload(id);
             return;
         }
@@ -322,6 +329,7 @@ public class DownloadNotificationService2 {
                                                 .setIsOffTheRecord(isOffTheRecord)
                                                 .setIsTransient(isTransient)
                                                 .setIcon(icon)
+                                                .setOriginalUrl(originalUrl)
                                                 .setNotificationId(notificationId)
                                                 .build();
 
@@ -395,11 +403,12 @@ public class DownloadNotificationService2 {
      * @param id             The {@link ContentId} of the download.
      * @param fileName       Filename of the download.
      * @param icon           A {@link Bitmap} to be used as the large icon for display.
+     * @param originalUrl    The original url of the downloaded file.
      * @param isOffTheRecord If the profile is off the record.
      * @param failState      Reason why download failed.
      */
     @VisibleForTesting
-    public void notifyDownloadFailed(ContentId id, String fileName, Bitmap icon,
+    public void notifyDownloadFailed(ContentId id, String fileName, Bitmap icon, String originalUrl,
             boolean isOffTheRecord, @FailState int failState) {
         // If the download is not in history db, fileName could be empty. Get it from
         // SharedPreferences.
@@ -519,7 +528,7 @@ public class DownloadNotificationService2 {
             if (!canResumeDownload(ContextUtils.getApplicationContext(), entry)) continue;
             if (mDownloadsInProgress.contains(entry.id)) continue;
             notifyDownloadPending(entry.id, entry.fileName, entry.isOffTheRecord,
-                    entry.canDownloadWhileMetered, entry.isTransient, null, false,
+                    entry.canDownloadWhileMetered, entry.isTransient, null, null, false,
                     PendingState.PENDING_NETWORK);
 
             Intent intent = new Intent();
@@ -659,8 +668,9 @@ public class DownloadNotificationService2 {
                 // paused notification, with the updated notification id..
                 notifyDownloadPaused(updatedEntry.id, updatedEntry.fileName, true /* isResumable */,
                         updatedEntry.isAutoResumable, updatedEntry.isOffTheRecord,
-                        updatedEntry.isTransient, null /* icon */, true /* hasUserGesture */,
-                        true /* forceRebuild */, PendingState.NOT_PENDING);
+                        updatedEntry.isTransient, null /* icon */, null /* originalUrl */,
+                        true /* hasUserGesture */, true /* forceRebuild */,
+                        PendingState.NOT_PENDING);
                 return;
             }
         }
@@ -675,7 +685,7 @@ public class DownloadNotificationService2 {
             // if native is still working and it triggers an update, then the service will be
             // restarted.
             notifyDownloadPaused(entry.id, entry.fileName, true, true, false, entry.isTransient,
-                    null, false, false, PendingState.PENDING_NETWORK);
+                    null, null, false, false, PendingState.PENDING_NETWORK);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier2.java

@@ -233,12 +233,12 @@ public class SystemDownloadNotifier2 implements DownloadNotifier {
                         info.getFileName(), info.getProgress(), info.getBytesReceived(),
                         info.getTimeRemainingInMillis(), notificationInfo.mStartTime,
                         info.isOffTheRecord(), notificationInfo.mCanDownloadWhileMetered,
-                        info.getIsTransient(), info.getIcon());
+                        info.getIsTransient(), info.getIcon(), info.getOriginalUrl());
                 break;
             case NotificationType.PAUSED:
                 getDownloadNotificationService().notifyDownloadPaused(info.getContentId(),
                         info.getFileName(), true, false, info.isOffTheRecord(),
-                        info.getIsTransient(), info.getIcon(), false, false,
+                        info.getIsTransient(), info.getIcon(), info.getOriginalUrl(), false, false,
                         info.getPendingState());
                 break;
             case NotificationType.SUCCEEDED:
@@ -258,14 +258,14 @@ public class SystemDownloadNotifier2 implements DownloadNotifier {
                 break;
             case NotificationType.FAILED:
                 getDownloadNotificationService().notifyDownloadFailed(info.getContentId(),
-                        info.getFileName(), info.getIcon(), info.isOffTheRecord(),
-                        info.getFailState());
+                        info.getFileName(), info.getIcon(), info.getOriginalUrl(),
+                        info.isOffTheRecord(), info.getFailState());
                 break;
             case NotificationType.INTERRUPTED:
                 getDownloadNotificationService().notifyDownloadPaused(info.getContentId(),
                         info.getFileName(), info.isResumable(), notificationInfo.mIsAutoResumable,
-                        info.isOffTheRecord(), info.getIsTransient(), info.getIcon(), false, false,
-                        notificationInfo.mPendingState);
+                        info.isOffTheRecord(), info.getIsTransient(), info.getIcon(),
+                        info.getOriginalUrl(), false, false, notificationInfo.mPendingState);
                 break;
         }
     }

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadNotificationService2Test.java

@@ -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);
+                false, null, null);
         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, false, false,
+                false /* isAutoResumable */, true, false, null, null, 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);
+                true, false, null, null);
         mDownloadForegroundServiceManager.onServiceConnected();
 
         assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
@@ -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);
+                false, null, null);
         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, false, false,
+                true /* isAutoResumable */, true, false, null, null, 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);
+                false, null, null);
         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, false, false,
+                true /* isAutoResumable */, true, false, null, null, false, false,
                 PendingState.PENDING_NETWORK);
         assertEquals(1, mDownloadNotificationService.getNotificationIds().size());
         assertFalse(mDownloadForegroundServiceManager.mDownloadUpdateQueue.containsKey(

chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService2.java

@@ -77,32 +77,35 @@ public class MockDownloadNotificationService2 extends DownloadNotificationServic
     public void notifyDownloadProgress(final ContentId id, final String fileName,
             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 boolean canDownloadWhileMetered, final boolean isTransient, final Bitmap icon,
+            final String originalUrl) {
         ThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> MockDownloadNotificationService2.super.notifyDownloadProgress(id,
                                 fileName, progress, bytesReceived, timeRemainingInMillis, startTime,
-                                isOffTheRecord, canDownloadWhileMetered, isTransient, icon));
+                                isOffTheRecord, canDownloadWhileMetered, isTransient, icon,
+                                originalUrl));
     }
 
     @Override
     void notifyDownloadPaused(ContentId id, String fileName, boolean isResumable,
             boolean isAutoResumable, boolean isOffTheRecord, boolean isTransient, Bitmap icon,
-            boolean hasUserGesture, boolean forceRebuild, @PendingState int pendingState) {
+            final String originalUrl, boolean hasUserGesture, boolean forceRebuild,
+            @PendingState int pendingState) {
         ThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> MockDownloadNotificationService2.super.notifyDownloadPaused(id, fileName,
                                 isResumable, isAutoResumable, isOffTheRecord, isTransient, icon,
-                                hasUserGesture, forceRebuild, pendingState));
+                                originalUrl, hasUserGesture, forceRebuild, pendingState));
     }
 
     @Override
     public void notifyDownloadFailed(final ContentId id, final String fileName, final Bitmap icon,
-            boolean isOffTheRecord, @FailState int failState) {
+            final String originalUrl, boolean isOffTheRecord, @FailState int failState) {
         ThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> MockDownloadNotificationService2.super.notifyDownloadFailed(
-                                id, fileName, icon, isOffTheRecord, failState));
+                                id, fileName, icon, originalUrl, isOffTheRecord, failState));
     }
 
     @Override

chrome/browser/background_fetch/background_fetch_browsertest.cc

@@ -312,14 +312,6 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
   // ---------------------------------------------------------------------------
   // Helper functions.
 
-  // Helper function for getting the expected title given the |developer_title|.
-  std::string GetExpectedTitle(const char* developer_title) {
-    url::Origin origin = url::Origin::Create(https_server_->base_url());
-
-    return base::StringPrintf("%s (%s)", developer_title,
-                              origin.Serialize().c_str());
-  }
-
   // Runs the |script| in the current tab and writes the output to |*result|.
   bool RunScript(const std::string& script, std::string* result) {
     return content::ExecuteScriptAndExtractString(
@@ -503,7 +495,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
   const OfflineItem& offline_item = items[0];
 
   // Verify that the appropriate data is being set.
-  EXPECT_EQ(offline_item.title, GetExpectedTitle(kSingleFileDownloadTitle));
+  EXPECT_EQ(offline_item.title, kSingleFileDownloadTitle);
   EXPECT_EQ(offline_item.filter, OfflineItemFilter::FILTER_OTHER);
   EXPECT_TRUE(offline_item.is_transient);
   EXPECT_TRUE(offline_item.is_resumable);

chrome/browser/background_fetch/background_fetch_delegate_impl.cc

@@ -78,6 +78,7 @@ BackgroundFetchDelegateImpl::JobDetails::JobDetails(
           fetch_description->job_unique_id)),
       fetch_description(std::move(fetch_description)) {
   offline_item.is_off_the_record = is_off_the_record;
+  offline_item.original_url = this->fetch_description->origin.GetURL();
   UpdateOfflineItem();
 }
 
@@ -103,15 +104,7 @@ void BackgroundFetchDelegateImpl::JobDetails::UpdateOfflineItem() {
   offline_item.progress.unit =
       offline_items_collection::OfflineItemProgressUnit::PERCENTAGE;
 
-  if (fetch_description->title.empty()) {
-    offline_item.title = fetch_description->origin.Serialize();
-  } else {
-    // TODO(crbug.com/774612): Make sure that the origin is displayed completely
-    // in all cases so that long titles cannot obscure it.
-    offline_item.title =
-        base::StringPrintf("%s (%s)", fetch_description->title.c_str(),
-                           fetch_description->origin.Serialize().c_str());
-  }
+  offline_item.title = fetch_description->title;
 
   offline_item.is_transient = true;
   offline_item.is_resumable = true;