Adding active_downloads_ map.

chrome/browser/download/download_item.cc

 // Copyright (c) 2010 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.
 
 #include "chrome/browser/download/download_item.h"
 
 #include "app/l10n_util.h"
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/format_macros.h"
@@ skipping 176 matching lines @@
 void DownloadItem::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 void DownloadItem::UpdateObservers() {
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadUpdated(this));
 }
 
 void DownloadItem::NotifyObserversDownloadFileCompleted() {
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadFileCompleted(this));
+
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      NewRunnableMethod(download_manager_,
+                        &DownloadManager::OnDownloadFileCompleted,
+                        id()));

[Randy Smith (Not in Mondays), 2011/01/03 22:02:22]

Oh, what a tangled web we're trying to unweave :-J.  Several comments on this
single statement, going from most detailed/low-level to most
abstract/high-level:
* The download item always runs on the UI thread (or if it doesn't we should
have big warning signs in neon around the routines where it doesn't) so you can
just call this method directly.

* I think calling this method from NotifyObserversDownloadFileComplete breaks an
implicit pattern--everywhere in the code that I've seen NotifyObservers<blah>,
the *only* thing the routine has done is notify the observers.  I'd rather not
break that implicit expectation (and violate the principle of least
astonishment) to save a single code duplication
(NotifyObserversDownloadFileComplete is only called from two places).

* If I poke around in the calling threads, it doesn't look to me as if
NotifyObserversDownloadFileComplete will *really* be called from two places. 
Specifically, I think that the comment at the end of DownloadItem::Finished() is
in error, and that if we have gotten to the end of DownloadItem::Finished(),
then the name has been finalized.  I could be wrong on this point (thus the
tangled web comment above) but if I am, we should figure that out.  And
presuming the name is finalized by the end of DownloadItem::Finished() I think
that's the right place for the call; that's the place at which the download
system really lets go of the DownloadItem.

[ahendrickson, 2011/01/04 16:51:38]

Hmm, looking at the |DownloadItem| code, I don't see any indication that it runs
only on the UI thread, except for some OS X specific ones in |OpenDownload()|
and |ShowDownloadInShell()|.

The only other reference to the UI thread that I see is for |Update()|, and that
one implies that it can be called from another thread (although I only see it
called from the UI thread).

I think we at least need a comment in the header indicating that it should only
be used on the UI thread.
OK, I've made it a straight call, and moved it to where
|NotifyObserversDownloadFileCompleted()| gets called.

I thought that comment was correct (the download happens in parallel with the
renaming).  I'm putting the call in there as well until we decide what we really
want to do.

 }
 
 bool DownloadItem::CanOpenDownload() {
   return !Extension::IsExtension(target_name_) &&
       download_util::IsFileSafe(target_name_);
 }
 
 bool DownloadItem::ShouldOpenFileBasedOnExtension() {
   return download_manager_->ShouldOpenFileBasedOnExtension(
       GetUserVerifiedFilePath());
@@ skipping 337 matching lines @@
         target_name_.value().c_str(),
         full_path().value().c_str());
   } else {
     description += base::StringPrintf(" url = \"%s\"", url().spec().c_str());
   }
 
   description += " }";
 
   return description;
 }