Downloads: Added transient flag to download item and download database

components/history/core/browser/download_row.h

Index: components/history/core/browser/download_row.h
diff --git a/components/history/core/browser/download_row.h b/components/history/core/browser/download_row.h
index cb0f0cc83862d4f9ce3d742460486b3de100291f..0a1252a42f3e2b18a14d5c921eb4809216987ff5 100644
--- a/components/history/core/browser/download_row.h
+++ b/components/history/core/browser/download_row.h
@@ -47,6 +47,7 @@ struct DownloadRow {
               const std::string& guid,
               bool download_opened,
               base::Time last_access,
+              bool transient,

[brettw, 2017/03/29 22:54:13]

This constructor has gotten absurd. I took too long on this CL already so I
don't want to block it any more. But can you do a follow-up to delete it and
just have callers set the members by name? It looks like there are 10 calls
total to this constructor (which you're necessarily updating in this patch) so
this should be an easy job.

There are a bunch of other instances of this (functions with 27 arguments for
DownloadItems) that you had to update in this change that I'm also dismayed by,
but I won't put fixing all of them on you. Do consider it though! :)

[shaktisahu, 2017/03/29 23:46:47]

I agree, it is really big constructor! But, in my opinion, having one
constructor instead of setters make it easy not to forget a particular member
especially since this struct directly maps to one row to the database. wdyt?

[brettw, 2017/03/30 16:44:43]

I agree there is some benefit as you decribe, but there is also a danger that
goes with this type of signature where arguments get mixed up. The signature has
"URL, URL, URL, URL, string, string, string, time, time, string, string" and
it's impossible to tell at the call site if you're putting the right things in
the right places. With struct assignments at least things have names.

Coupled with the growing unwieldyness of the constructor, I think we should
change it.

               const std::string& ext_id,
               const std::string& ext_name,
               const std::vector<DownloadSliceInfo>& download_slice_info);
@@ -138,6 +139,9 @@ struct DownloadRow {
   // The time when the download was last accessed.
   base::Time last_access_time;
 
+  // Whether this download is transient.

[brettw, 2017/03/29 22:54:13]

Can this comment define what transient means? I'm thinking "deleted when it's
done and not shown in the UI".

[shaktisahu, 2017/03/29 23:46:47]

Done.

+  bool transient;
+
   // The id and name of the extension that created this download.
   std::string by_ext_id;
   std::string by_ext_name;