Make DownloadHistory observe manager, items

content/browser/download/download_item_impl.cc

 // Copyright (c) 2012 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.
 
 // File method ordering: Methods in this file are in the same order
 // as in download_item_impl.h, with the following exception: The public
 // interfaces DelayedDownloadOpened, OnDownloadTargetDetermined, and
 // OnDownloadCompleting are placed in chronological order with the other
 // (private) routines that together define a DownloadItem's state transitions
 // as the download progresses.  See "Download progression cascade" later in
@@ skipping 27 matching lines @@
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_interrupt_reasons_impl.h"
 #include "content/browser/download/download_item_impl_delegate.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/download_persistent_store_info.h"
 #include "net/base/net_util.h"
 
 using content::BrowserThread;
 using content::DownloadFile;
 using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
-using content::DownloadPersistentStoreInfo;
 using content::WebContents;
 
 namespace {
 
 static void DeleteDownloadedFile(const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // Make sure we only delete files.
   if (!file_util::DirectoryExists(path))
     file_util::Delete(path, false);
@@ skipping 34 matching lines @@
   virtual void CancelRequest() const OVERRIDE {}
   virtual std::string DebugString() const OVERRIDE {
     return "Null DownloadRequestHandle";
   }
 };
 
 }  // namespace
 
 namespace content {
 
-// Our download table ID starts at 1, so we use 0 to represent a download that
-// has started, but has not yet had its data persisted in the table. We use fake
-// database handles in incognito mode starting at -1 and progressively getting
-// more negative.
-// static
-const int DownloadItem::kUninitializedHandle = 0;
-
 const char DownloadItem::kEmptyFileHash[] = "";
 
 }
 
-// Our download table ID starts at 1, so we use 0 to represent a download that
-// has started, but has not yet had its data persisted in the table. We use fake
-// database handles in incognito mode starting at -1 and progressively getting
-// more negative.
-
 // Constructor for reading from the history service.
 DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
                                    DownloadId download_id,
-                                   const DownloadPersistentStoreInfo& info,
+                                   const FilePath& path,
+                                   const GURL& url,
+                                   const GURL& referrer_url,
+                                   const base::Time& start_time,
+                                   const base::Time& end_time,
+                                   int64 received_bytes,
+                                   int64 total_bytes,
+                                   DownloadItem::DownloadState state,
+                                   bool opened,
                                    const net::BoundNetLog& bound_net_log)
     : download_id_(download_id),
-      current_path_(info.path),
-      target_path_(info.path),
+      current_path_(path),
+      target_path_(path),
       target_disposition_(TARGET_DISPOSITION_OVERWRITE),
-      url_chain_(1, info.url),
-      referrer_url_(info.referrer_url),
+      url_chain_(1, url),
+      referrer_url_(referrer_url),
       transition_type_(content::PAGE_TRANSITION_LINK),
       has_user_gesture_(false),
-      total_bytes_(info.total_bytes),
-      received_bytes_(info.received_bytes),
+      total_bytes_(total_bytes),
+      received_bytes_(received_bytes),
       bytes_per_sec_(0),
       last_reason_(content::DOWNLOAD_INTERRUPT_REASON_NONE),
       start_tick_(base::TimeTicks()),
-      state_(ExternalToInternalState(info.state)),
+      state_(ExternalToInternalState(state)),
       danger_type_(content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS),
-      start_time_(info.start_time),
-      end_time_(info.end_time),
-      db_handle_(info.db_handle),
+      start_time_(start_time),
+      end_time_(end_time),
       delegate_(delegate),
       is_paused_(false),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
       auto_opened_(false),
-      is_persisted_(true),
       is_temporary_(false),
       all_data_saved_(false),
-      opened_(info.opened),
+      opened_(opened),
       open_enabled_(true),
       delegate_delayed_complete_(false),
       bound_net_log_(bound_net_log),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   delegate_->Attach();
   if (state_ == IN_PROGRESS_INTERNAL)
     state_ = CANCELLED_INTERNAL;
   if (state_ == COMPLETE_INTERNAL)
     all_data_saved_ = true;
   Init(false /* not actively downloading */,
@@ skipping 23 matching lines @@
       referrer_charset_(info.referrer_charset),
       remote_address_(info.remote_address),
       total_bytes_(info.total_bytes),
       received_bytes_(0),
       bytes_per_sec_(0),
       last_reason_(content::DOWNLOAD_INTERRUPT_REASON_NONE),
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS_INTERNAL),
       danger_type_(content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS),
       start_time_(info.start_time),
-      db_handle_(DownloadItem::kUninitializedHandle),
       delegate_(delegate),
       is_paused_(false),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
       auto_opened_(false),
-      is_persisted_(false),
       is_temporary_(!info.save_info.file_path.empty()),
       all_data_saved_(false),
       opened_(false),
       open_enabled_(true),
       delegate_delayed_complete_(false),
       bound_net_log_(bound_net_log),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   delegate_->Attach();
   Init(true /* actively downloading */,
        download_net_logs::SRC_NEW_DOWNLOAD);
@@ skipping 27 matching lines @@
       mime_type_(mime_type),
       original_mime_type_(mime_type),
       total_bytes_(0),
       received_bytes_(0),
       bytes_per_sec_(0),
       last_reason_(content::DOWNLOAD_INTERRUPT_REASON_NONE),
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS_INTERNAL),
       danger_type_(content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS),
       start_time_(base::Time::Now()),
-      db_handle_(DownloadItem::kUninitializedHandle),
       delegate_(delegate),
       is_paused_(false),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
       auto_opened_(false),
-      is_persisted_(false),
       is_temporary_(false),
       all_data_saved_(false),
       opened_(false),
       open_enabled_(true),
       delegate_delayed_complete_(false),
       bound_net_log_(bound_net_log),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   delegate_->Attach();
   Init(true /* actively downloading */,
        download_net_logs::SRC_SAVE_PAGE_AS);
@@ skipping 158 matching lines @@
 }
 
 int32 DownloadItemImpl::GetId() const {
   return download_id_.local();
 }
 
 DownloadId DownloadItemImpl::GetGlobalId() const {
   return download_id_;
 }
 
-int64 DownloadItemImpl::GetDbHandle() const {
-  return db_handle_;
-}
-
 DownloadItem::DownloadState DownloadItemImpl::GetState() const {
   return InternalToExternalState(state_);
 }
 
 content::DownloadInterruptReason DownloadItemImpl::GetLastReason() const {
   return last_reason_;
 }
 
 bool DownloadItemImpl::IsPaused() const {
   return is_paused_;
 }
 
 bool DownloadItemImpl::IsTemporary() const {
   return is_temporary_;
 }
 
-bool DownloadItemImpl::IsPersisted() const {
-  return is_persisted_;
-}
-
 // TODO(ahendrickson) -- Move |INTERRUPTED| from |IsCancelled()| to
 // |IsPartialDownload()|, when resuming interrupted downloads is implemented.
 bool DownloadItemImpl::IsPartialDownload() const {
   return InternalToExternalState(state_) == IN_PROGRESS;
 }
 
 bool DownloadItemImpl::IsInProgress() const {
   return InternalToExternalState(state_) == IN_PROGRESS;
 }
 
@@ skipping 196 matching lines @@
 }
 
 bool DownloadItemImpl::GetAutoOpened() {
   return auto_opened_;
 }
 
 bool DownloadItemImpl::GetOpened() const {
   return opened_;
 }
 
-DownloadPersistentStoreInfo DownloadItemImpl::GetPersistentStoreInfo() const {
-  // TODO(asanka): Persist GetTargetFilePath() as well.
-  return DownloadPersistentStoreInfo(GetFullPath(),
-                                     GetURL(),
-                                     GetReferrerUrl(),
-                                     GetStartTime(),
-                                     GetEndTime(),
-                                     GetReceivedBytes(),
-                                     GetTotalBytes(),
-                                     GetState(),
-                                     GetDbHandle(),
-                                     GetOpened());
-}
-
 content::BrowserContext* DownloadItemImpl::GetBrowserContext() const {
   return delegate_->GetBrowserContext();
 }
 
 WebContents* DownloadItemImpl::GetWebContents() const {
   // TODO(rdsmith): Remove null check after removing GetWebContents() from
   // paths that might be used by DownloadItems created from history import.
   // Currently such items have null request_handle_s, where other items
   // (regular and SavePackage downloads) have actual objects off the pointer.
   if (request_handle_.get())
@@ skipping 41 matching lines @@
     ++iter;
     for ( ; verbose && (iter != last); ++iter) {
       url_list += " ->\n\t";
       const GURL& next_url = *iter;
       url_list += next_url.spec();
     }
   }
 
   if (verbose) {
     description += base::StringPrintf(
-        " db_handle = %" PRId64
         " total = %" PRId64
         " received = %" PRId64
         " reason = %s"
         " paused = %c"
         " safety = %s"
         " last_modified = '%s'"
         " etag = '%s'"
         " url_chain = \n\t\"%s\"\n\t"
         " full_path = \"%" PRFilePath "\""
         " target_path = \"%" PRFilePath "\"",
-        GetDbHandle(),
         GetTotalBytes(),
         GetReceivedBytes(),
         InterruptReasonDebugString(last_reason_).c_str(),
         IsPaused() ? 'T' : 'F',
         DebugSafetyStateString(GetSafetyState()),
         GetLastModifiedTime().c_str(),
         GetETag().c_str(),
         url_list.c_str(),
         GetFullPath().value().c_str(),
         GetTargetFilePath().value().c_str());
@@ skipping 104 matching lines @@
 }
 
 void DownloadItemImpl::MarkAsComplete() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DCHECK(all_data_saved_);
   end_time_ = base::Time::Now();
   TransitionTo(COMPLETE_INTERNAL);
 }
 
-void DownloadItemImpl::SetIsPersisted() {
-  is_persisted_ = true;
-  UpdateObservers();
-}
-
-void DownloadItemImpl::SetDbHandle(int64 handle) {
-  db_handle_ = handle;
-
-  bound_net_log_.AddEvent(
-      net::NetLog::TYPE_DOWNLOAD_ITEM_IN_HISTORY,
-      net::NetLog::Int64Callback("db_handle", db_handle_));
-}
-
 // **** Download progression cascade
 
 void DownloadItemImpl::Init(bool active,
                             download_net_logs::DownloadType download_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (active)
     download_stats::RecordDownloadCount(download_stats::START_COUNT);
 
   if (target_path_.empty())
     target_path_ = current_path_;
   std::string file_name;
   if (download_type == download_net_logs::SRC_HISTORY_IMPORT) {
     // target_path_ works for History and Save As versions.
     file_name = target_path_.AsUTF8Unsafe();
   } else {
     // See if it's set programmatically.
     file_name = forced_file_path_.AsUTF8Unsafe();
     // Possibly has a 'download' attribute for the anchor.
     if (file_name.empty())
       file_name = suggested_filename_;
     // From the URL file name.
     if (file_name.empty())
       file_name = GetURL().ExtractFileName();
   }
 
-  bound_net_log_.BeginEvent(
-      net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE,
-      base::Bind(&download_net_logs::ItemActivatedCallback,
-                 this, download_type, &file_name));
-
-  // If this is not an active download, end the ACTIVE event now.
-  if (!active) {
+  base::Callback<base::Value*(net::NetLog::LogLevel)> active_data = base::Bind(
+      &download_net_logs::ItemActivatedCallback, this,
+      download_type, &file_name);
+  if (active) {
+    bound_net_log_.BeginEvent(
+        net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE, active_data);
+  } else {
     bound_net_log_.AddEvent(

[asanka, 2012/09/28 19:02:30]

EndEvent()

[benjhayden, 2012/10/01 14:10:03]

Can you elaborate why?
This is Init(), which is only called when downloads are created, so there was no
corresponding BeginEvent. I'd expect it to be incorrect to call EndEvent with no
corresponding BeginEvent.

[asanka, 2012/10/01 15:59:36]

Ignore my comment. I misread.

-        net::NetLog::TYPE_DOWNLOAD_ITEM_IN_HISTORY,
-        net::NetLog::Int64Callback("db_handle", db_handle_));
-
-    bound_net_log_.EndEvent(net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE);
+        net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE, active_data);
   }
 
   VLOG(20) << __FUNCTION__ << "() " << DebugString(true);
 }
 
 // Called by DownloadManagerImpl when the download target path has been
 // determined.
 void DownloadItemImpl::OnDownloadTargetDetermined(
     const FilePath& target_path,
     TargetDisposition disposition,
@@ skipping 116 matching lines @@
 
   if (content::DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
     Interrupt(reason);
     return;
   }
 
   // full_path is now the current and target file path.
   DCHECK(!full_path.empty());
   target_path_ = full_path;
   SetFullPath(full_path);
-  delegate_->DownloadRenamedToFinalName(this);
+  UpdateObservers();

[asanka, 2012/09/28 19:02:30]

Given that with this change we always call UpdateObservers() after
SetFullPath(), we might as well fold the call into SetFullPath().

[benjhayden, 2012/10/01 14:10:03]

Done.

 
   // Complete the download and release the DownloadFile.
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFileManager::CompleteDownload,
                  delegate_->GetDownloadFileManager(), GetGlobalId(),
                  base::Bind(&DownloadItemImpl::OnDownloadFileReleased,
                             weak_ptr_factory_.GetWeakPtr())));
   TransitionTo(COMPLETING_INTERNAL);
 }
@@ skipping 188 matching lines @@
       return "COMPLETE";
     case CANCELLED_INTERNAL:
       return "CANCELLED";
     case INTERRUPTED_INTERNAL:
       return "INTERRUPTED";
     default:
       NOTREACHED() << "Unknown download state " << state;
       return "unknown";
   };
 }