Added member data to classes to support download resumption.

content/browser/download/download_create_info.h

 // Copyright (c) 2011 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.
 
 #ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_
 #pragma once
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #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"
 
 // Used for informing the download manager of a new download, since we don't
 // want to pass |DownloadItem|s between threads.
 struct CONTENT_EXPORT DownloadCreateInfo {
   DownloadCreateInfo(const FilePath& path,
                      const GURL& url,
                      const base::Time& start_time,
@@ skipping 53 matching lines @@
   std::string content_disposition;
 
   // The mime type string from the response header (may be overridden).
   std::string mime_type;
 
   // The value of the content type header sent with the downloaded item.  It
   // may be different from |mime_type|, which may be set based on heuristics
   // 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;
 
   // The original name for a dangerous download.
   FilePath original_name;
 
   // The charset of the referring page where the download request comes from.
   // It's used to construct a suggested filename.
   std::string referrer_charset;
 
   // The download file save info.
   DownloadSaveInfo save_info;
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_CREATE_INFO_H_