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

chrome/common/extensions/api/downloads.idl

Index: chrome/common/extensions/api/downloads.idl
diff --git a/chrome/common/extensions/api/downloads.idl b/chrome/common/extensions/api/downloads.idl
index 0381c887f0cdfc65ce7e821dcf600e25a71fdd8a..4cd1b627d6444c6cea94e352a1cb936cbdeba646 100644
--- a/chrome/common/extensions/api/downloads.idl
+++ b/chrome/common/extensions/api/downloads.idl
@@ -135,9 +135,14 @@ namespace downloads {
     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.
@@ -263,6 +268,10 @@ namespace downloads {
     // <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.
@@ -279,8 +288,13 @@ namespace downloads {
     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;
 
@@ -346,6 +360,9 @@ namespace downloads {
     // 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;