Expose final download URL (actual url after redirects) in the extension API.

chrome/common/extensions/api/downloads.idl

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Use the <code>chrome.downloads</code> API to programmatically initiate,
 // monitor, manipulate, and search for downloads.
 [permissions=downloads]
 namespace downloads {
   [inline_doc] dictionary HeaderNameValuePair {
     // Name of the HTTP header.
@@ skipping 117 matching lines @@
   //     <dd>The download completed successfully.</dd>
   // </dl>
   enum State {in_progress, interrupted, complete};
 
   // The state of the process of downloading a file.
   dictionary DownloadItem {
     // An identifier that is persistent across browser sessions.
     long id;
 
     // Absolute URL.
+    // URL that this download started from (before any redirects).

[asargent_no_longer_on_chrome, 2016/06/16 22:11:41]

nit: in the autogenerated documentation at
https://developer.chrome.com/extensions/downloads#type-DownloadItem, this will
read as one awkward paragraph like this: 

"Absolute URL. URL that this download started from (before any redirects)."

You should rewrite it to either a single sentence or a pair of sentences. E.g.
something like:


"The absolute URL that this download started from, before any redirects."

or

"Absolute URL. This will be the URL that this download started from, before any
redirects."

[mharanczyk, 2016/06/23 10:33:21]

Done.

     DOMString url;
 
     // Absolute URL.
+    // Actual download URL (final url in chain after all redirects).

[asargent_no_longer_on_chrome, 2016/06/16 22:11:40]

same nit here

[mharanczyk, 2016/06/23 10:33:21]

Done.

+    DOMString finalUrl;
+
+    // Absolute URL.
     DOMString referrer;
 
     // Absolute local path.
     DOMString filename;
 
     // False if this download is recorded in the history, true if it is not
     // recorded.
     boolean incognito;
 
     // Indication of whether this download is thought to be safe or known to be
@@ skipping 105 matching lines @@
     double? totalBytesLess;
 
     // Limits results to $(ref:DownloadItem) whose
     // <code>filename</code> matches the given regular expression.
     DOMString? filenameRegex;
 
     // Limits results to $(ref:DownloadItem) whose
     // <code>url</code> matches the given regular expression.
     DOMString? urlRegex;
 
+    // Limits results to $(ref:DownloadItem) whose
+    // <code>finalUrl</code> matches the given regular expression.
+    DOMString? finalUrlRegex;
+
     // The maximum number of matching $(ref:DownloadItem) returned. Defaults to
     // 1000. Set to 0 in order to return all matching $(ref:DownloadItem). See
     // $(ref:search) for how to page through results.
     long? limit;
 
     // Set elements of this array to $(ref:DownloadItem) properties in order to
     // sort search results. For example, setting
     // <code>orderBy=['startTime']</code> sorts the $(ref:DownloadItem) by their
     // start time in ascending order. To specify descending order, prefix with a
     // hyphen: '-startTime'.
     DOMString[]? orderBy;
 
     // The <code>id</code> of the $(ref:DownloadItem) to query.
     long? id;
 
     // Absolute URL.
+    // URL that this download started from (before any redirects).
     DOMString? url;
 
+    // Absolute URL.
+    // Actual download URL (final url in chain after all redirects).

[asargent_no_longer_on_chrome, 2016/06/16 22:11:41]

same nit here

[mharanczyk, 2016/06/23 10:33:21]

Done.

+    DOMString? finalUrl;
+
     // Absolute local path.
     DOMString? filename;
 
     // Indication of whether this download is thought to be safe or known to be
     // suspicious.
     DangerType? danger;
 
     // The file's MIME type.
     DOMString? mime;
 
@@ skipping 45 matching lines @@
 
   // Encapsulates a change in a DownloadItem.
   [inline_doc] dictionary DownloadDelta {
     // The <code>id</code> of the $(ref:DownloadItem)
     // that changed.
     long id;
 
     // The change in <code>url</code>, if any.
     StringDelta? url;
 
+    // The change in <code>finalUrl</code>, if any.

[asargent_no_longer_on_chrome, 2016/06/16 22:11:41]

It would be useful to mention here how finalUrl can change, so that it shows up
in the documentation for developers. Not having looked at your implementation
yet, as a developer consuming the API I'd hope that the only possibility would
be that finalUrl would start out empty (so would not be present in any
DownloadDelta's) until the download has reached the final url and we are
guaranteed no more redirects are possible, and then finalUrl would be non-empty
in a DownloadDelta and we're guaranteed it will never change again. 

[asanka, 2016/06/22 19:05:13]

Neither url nor finalUrl can change for a download after it's created. Same goes
for mime, and startTime.

[mharanczyk, 2016/06/23 10:33:21]

So in what way should be this handled?
1. Just remove finalUrl and leave this part of api a bit inconsistent.
2. Remove all the data that cannot appear here pointed by asanka. This will in
theory change the api that should be stable but in practice since the data is
optional and never was present it should cause no harm to remove it.
3. Just leave it as it is now for api consistency even though it won't be
present.
Please advise.

[asargent_no_longer_on_chrome, 2016/06/23 16:35:52]

I think it's fine to remove any optional elements from this dictionary if they
have never, and can never, be present. 

[mharanczyk, 2016/06/28 13:15:32]

As it turn out those delta fields are actually set up in very specific cases.
The onChanged event with all the data from created event can be sent to
extension for downloads already existing before onChanged event listener
registration or when dangerous download will transition from temporary to not
temporary.
Otherwise those properties are reported in onCreated event and will never be
present in onChanged afaict.

I've added additional explanation to documentation stating those situations and
mentioning that above download properties may not change after it starts.
benjhayden@, asargent@, please take another look.

[asanka, 2016/06/28 13:39:45]

FTR, both of those behaviors are incorrect. Reporting all the fields during
onChanged for some downloads isn't useful and masks the material change that
occurred in the download. Also, I'd argue that the url, finalUrl and mime fields
are more important for evaluating a dangerous download than a safe one. I
suggest we address these later.

+    StringDelta? finalUrl;
+
     // The change in <code>filename</code>, if any.
     StringDelta? filename;
 
     // The change in <code>danger</code>, if any.
     StringDelta? danger;
 
     // The change in <code>mime</code>, if any.
     StringDelta? mime;
 
     // The change in <code>startTime</code>, if any.
@@ skipping 185 matching lines @@
     // <code>suggestion</code> object to <code>suggest</code> wins. In order to
     // avoid confusion regarding which extension will win, users should not
     // install extensions that may conflict. If the download is initiated by
     // $(ref:download) and the target filename is known before the MIME type and
     // tentative filename have been determined, pass <code>filename</code> to
     // $(ref:download) instead.
     [maxListeners=1] static void onDeterminingFilename(
         DownloadItem downloadItem, SuggestFilenameCallback suggest);
   };
 };