Added member data to classes to support download resumption.

content/browser/download/download_create_info.h

Index: content/browser/download/download_create_info.h
diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h
index 16088911c628c082a16d947aa101bf01fd478054..3882c8f20aad5676369338b61bf47d2adcbbf1e6 100644
--- a/content/browser/download/download_create_info.h
+++ b/content/browser/download/download_create_info.h
@@ -13,6 +13,7 @@
 #include "base/file_path.h"
 #include "base/time.h"
 #include "content/browser/download/download_file.h"
+#include "content/browser/download/interrupt_reasons.h"
 #include "content/common/content_export.h"
 #include "content/public/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
@@ -86,9 +87,21 @@ struct CONTENT_EXPORT DownloadCreateInfo {
   // which may look at the file extension and first few bytes of the file.
   std::string original_mime_type;
 
+  // For restarting a download, the modification time of the file.
+  std::string last_modified;

[Randy Smith (Not in Mondays), 2011/10/31 18:46:43]

Why is this a string rather than a base::Time or something like it?  

[ahendrickson, 2011/11/13 21:15:20]

I wanted to return to the server the exact Last-Modified string that we received
in the initial download.  Will base::Time guarantee that it's the save after
converting back?  Or, can we guarantee that the formatting doesn't matter?

[Randy Smith (Not in Mondays), 2011/11/15 18:35:25]

Would you do a little investigation of this issue?  I understand the
requirement, and it's probably needed, but I'm not ecstatic about it;
last_modified as a string smells wrong.  Maybe we want to abstract a bit more
and just pass in an (optional) conditional for the server, and format it on the
UI side?

[ahendrickson, 2011/11/16 15:41:08]

In order to use base::Time, we would be converting from a string to Time, then
reconverting it when saving to the history and when restarting a download.

It seems like a lot of extra work when what we need is the original string.

On 2011/11/15 18:35:25, rdsmith wrote:

[Randy Smith (Not in Mondays), 2011/11/16 18:29:27]

Ok, but include a comment explaining the code smell.  And if we come up with a
use case for the actual time in the code, I'll start pushing on this issue
again.

[ahendrickson, 2011/11/19 20:18:03]

Done.

+
+  // For restarting a download, the ETAG of the file.
+  std::string etag;
+
+  // Is continuing a download.
+  bool continued_download;

[Randy Smith (Not in Mondays), 2011/10/31 18:46:43]

What's the use case for this field?  Would it make more sense to just pass the
offset from the beginning of the file of the data being received?  I could
imagine that as being more flexible for more purposes in the future (e.g. an IDM
like functionality that had multiple requests fetching different parts of the
file).

[ahendrickson, 2011/11/13 21:15:20]

We do have the offset for the data being received.  However, I can see
potentially wanting to distinguish multiple partial requests from a 'continue
download' scenario -- for example, in determining whether or not to
automatically resume.

I can remove this variable if you feel strongly about it.
 

[Randy Smith (Not in Mondays), 2011/11/15 18:35:25]

Sure, but I would imagine that distinction would be best done in management
code, i.e. in DownloadItem on the UI thread.  
I feel pretty strongly about starting simple and adding complexity only when
it's needed, and it doesn't sound like there's a nailed down use case at the
moment?

[ahendrickson, 2011/11/16 15:41:08]

Done.

+
+  // Server error code, if any.
+  InterruptReason server_interrupt_reason;

[Randy Smith (Not in Mondays), 2011/10/31 18:46:43]

What's the use case for this field?  DownloadCreateInfo is for passing data from
IO->FILE/UI threads, and we already pass interruption reasons via
OnResponseCompleted.

[ahendrickson, 2011/11/13 21:15:20]

This is so we can pass in interrupt reasons that are caused by response codes
from the server.

[Randy Smith (Not in Mondays), 2011/11/15 18:35:25]

Could you flesh out that use case?  We already have a pathway for passing error
codes from the IO thread machinery to the rest of the download system
(OnResponseCompleted()); why not use that?

[ahendrickson, 2011/11/16 15:41:08]

Done.

+
   // True if we should display the 'save as...' UI and prompt the user
   // for the download location.
-  // False if the UI should be supressed and the download performed to the
+  // False if the UI should be suppressed and the download performed to the
   // default location.
   bool prompt_user_for_save_location;