Added net logging to DownloadItem.

content/browser/download/download_net_log_parameters.h

Index: content/browser/download/download_net_log_parameters.h
diff --git a/content/browser/download/download_net_log_parameters.h b/content/browser/download/download_net_log_parameters.h
index 031cea039242cc0cf2434a4d945cc96a490cc12d..bcb7470139bcddac02d13e6ea9809ecd80f7d1be 100644
--- a/content/browser/download/download_net_log_parameters.h
+++ b/content/browser/download/download_net_log_parameters.h
@@ -8,11 +8,156 @@
 
 #include <string>
 
+#include "content/public/browser/download_item.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 namespace download_net_logs {
 
+enum DownloadType {
+  SRC_NEW_DOWNLOAD,
+  SRC_HISTORY_IMPORT,
+  SRC_SAVE_PAGE_AS
+};
+
+// NetLog parameters when a DownloadItem is activated.
+class ItemActivatedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemActivatedParameters(DownloadType download_type,
+                          int64 id,
+                          const std::string& original_url,
+                          const std::string& final_url,
+                          const std::string& intermediate_name,
+                          const std::string& suggested_name,

[Randy Smith (Not in Mondays), 2012/02/05 23:49:15]

The names of these parameters seem misleading to me, *especially* if one if
familiar with the downloads system.  My reading of the code (which I'm not
certain about--please feel free to dispute) is that "intermediate_name" will be
null for regular downloads (it's the "full path", which I think is only set for
history and save as), and that suggested_name is null for non-regular downloads
and will only sometimes be set for regular downloads (programmatic downloads
that specify the file that they want to write to and maybe (though I don't
remember the code, the comment indicates this may happen) if the right header is
set in the URL request.  Could you do some combination of changing the names to
make more sense and/or commenting exactly what these names will be in the
different cases?

(The right place for the comment is probably in the event type list file--I just
read this one first.)

[ahendrickson, 2012/02/06 21:39:20]

Done.

+                          content::DownloadItem::SafetyState safety_state,

[Randy Smith (Not in Mondays), 2012/02/05 23:49:15]

In what circumstances will this have information in it?  

[ahendrickson, 2012/02/06 21:39:20]

In the case of a History download, I believe.

[Randy Smith (Not in Mondays), 2012/02/06 22:34:59]

Hmmm.  I'm having a hard time imagining a situation in which the safety state of
a history download would help us debug an issue.  But if you can, I'm good with
it.

[Randy Smith (Not in Mondays), 2012/02/07 15:09:45]

When you switched to DownloadDangerType I checked the code, and it doesn't look
to me like we persist *either* SafetyState or DownloadDangerType.  I ask again,
when will this have useful information in it?  Unless I'm misreading the code,
it ain't for the history download.  Can the DownloadItem ever be initialized
with danger_type with some information in it?  (Asanka might know if the URL
check happens before DownloadItem creation, or you could trace the code.)

[ahendrickson, 2012/02/07 17:18:35]

We will persist both when download resumption is implemented.

For the other constructors, it will always be initialized the same way.

+                          int64 start_offset);
+  virtual ~ItemActivatedParameters();
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const DownloadType type_;
+  const int64 id_;
+  const std::string original_url_;
+  const std::string final_url_;
+  const std::string intermediate_filename_;
+  const std::string suggested_final_filename_;
+  const content::DownloadItem::SafetyState safety_state_;
+  const int64 start_offset_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemActivatedParameters);
+};
+
+// NetLog parameters when a DownloadItem is checked for danger.
+class ItemCheckedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemCheckedParameters(content::DownloadItem::SafetyState safety_state);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const content::DownloadItem::SafetyState safety_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemCheckedParameters);
+};
+
+// NetLog parameters when a DownloadItem is added to the history database.
+class ItemInHistoryParameters : public net::NetLog::EventParameters {
+ public:
+  ItemInHistoryParameters(int64 handle);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const int64 db_handle_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemInHistoryParameters);
+};
+
+// NetLog parameters when a DownloadItem is updated.
+class ItemUpdatedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemUpdatedParameters(int64 bytes_so_far, const std::string& hash_state);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const int64 bytes_so_far_;
+  const std::string hash_state_;

[Randy Smith (Not in Mondays), 2012/02/05 23:49:15]

What's the value in logging the hash state?  (Ditto below.)

[ahendrickson, 2012/02/06 21:39:20]

Removed.

+
+  DISALLOW_COPY_AND_ASSIGN(ItemUpdatedParameters);
+};
+
+// NetLog parameters when a DownloadItem is renamed.
+class ItemRenamedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemRenamedParameters(
+      const std::string& old_filename, const std::string& new_filename);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const std::string old_filename_;
+  const std::string new_filename_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemRenamedParameters);
+};
+
+// NetLog parameters when a DownloadItem is interrupted.
+class ItemInterruptedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemInterruptedParameters(InterruptReason reason,
+                            int64 bytes_so_far,
+                            const std::string& hash_state);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const InterruptReason reason_;
+  const int64 bytes_so_far_;
+  const std::string hash_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemInterruptedParameters);
+};
+
+// NetLog parameters when a DownloadItem is resumed.
+class ItemResumedParameters : public net::NetLog::EventParameters {

[Randy Smith (Not in Mondays), 2012/02/05 23:49:15]

This doesn't look like it's used in this CL.  If so, can we delay putting it in
until use?

[ahendrickson, 2012/02/06 21:39:20]

Done.

+ public:
+  ItemResumedParameters(bool user_initiated,
+                        InterruptReason reason,
+                        int64 bytes_so_far,
+                        const std::string& hash_state);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const bool user_initiated_;
+  const InterruptReason reason_;
+  const int64 bytes_so_far_;
+  const std::string hash_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemResumedParameters);
+};
+
+// NetLog parameters when a DownloadItem is finished.
+class ItemFinishedParameters : public net::NetLog::EventParameters {
+ public:
+  ItemFinishedParameters(int64 bytes_so_far, const std::string& final_hash);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const int64 bytes_so_far_;
+  const std::string final_hash_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemFinishedParameters);
+};
+
+// NetLog parameters when a DownloadItem is canceled.
+class ItemCanceledParameters : public net::NetLog::EventParameters {
+ public:
+  ItemCanceledParameters(int64 bytes_so_far, const std::string& hash_state);
+  virtual base::Value* ToValue() const OVERRIDE;
+
+ private:
+  const int64 bytes_so_far_;
+  const std::string hash_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ItemCanceledParameters);
+};
+
 // NetLog parameters when a DownloadFile is opened.
 class FileOpenedParameters : public net::NetLog::EventParameters {
  public: