Make DownloadHistory observe manager, items

chrome/browser/extensions/api/downloads/downloads_api.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.
 
 #include "chrome/browser/extensions/api/downloads/downloads_api.h"
 
 #include <algorithm>
 #include <cctype>
 #include <iterator>
 #include <set>
@@ skipping 837 matching lines @@
       params->options.get();
   int icon_size = kDefaultIconSize;
   if (options && options->size.get())
     icon_size = *options->size.get();
   DownloadManager* manager = NULL;
   DownloadManager* incognito_manager = NULL;
   GetManagers(profile(), include_incognito(), &manager, &incognito_manager);
   DownloadItem* download_item = manager->GetDownload(params->download_id);
   if (!download_item && incognito_manager)
     download_item = incognito_manager->GetDownload(params->download_id);
-  if (!download_item) {
+  if (!download_item || download_item->GetTargetFilePath().empty()) {
     // The DownloadItem is is added to history when the path is determined. If

[Randy Smith (Not in Mondays), 2012/09/24 18:03:25]

Is this comment still accurate?

[benjhayden, 2012/09/24 20:12:11]

Nope. Deleted.
chrome.downloads.onCreated is fired in OnDownloadCreated, before the filename is
determined.
chrome.downloads.onChanged is fired when the filename is determined.
Extensions can wait for onChanged with a non-empty filename before calling
getFileIcon().
That should probably be documented somewhere. OTOH, it's a transient non-fatal
error that good hackers might be able to figure out on their own. If they can't,
then they'll probably file a bug, at which point I'll look into documenting it
somewhere.

     // the download is not in history, then we don't have a path / final
     // filename and no icon.
     error_ = download_extension_errors::kInvalidOperationError;
     return false;
   }
   // In-progress downloads return the intermediate filename for GetFullPath()
   // which doesn't have the final extension. Therefore we won't be able to
   // derive a good file icon for it. So we use GetTargetFilePath() instead.
-  FilePath path = download_item->GetTargetFilePath();
-  DCHECK(!path.empty());
   DCHECK(icon_extractor_.get());
   DCHECK(icon_size == 16 || icon_size == 32);
   EXTENSION_FUNCTION_VALIDATE(icon_extractor_->ExtractIconURLForPath(
-      path, IconLoaderSizeFromPixelSize(icon_size),
+      download_item->GetTargetFilePath(),
+      IconLoaderSizeFromPixelSize(icon_size),
       base::Bind(&DownloadsGetFileIconFunction::OnIconURLExtracted, this)));
   return true;
 }
 
 void DownloadsGetFileIconFunction::OnIconURLExtracted(const std::string& url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (url.empty()) {
     error_ = download_extension_errors::kIconNotFoundError;
   } else {
     RecordApiFunctions(DOWNLOADS_FUNCTION_GET_FILE_ICON);
@@ skipping 118 matching lines @@
   if (profile_->HasOffTheRecordProfile() &&
       !profile_->IsOffTheRecord()) {
     DispatchEventInternal(
         profile_->GetOffTheRecordProfile(),
         event_name,
         json_args,
         scoped_ptr<base::ListValue>(args->DeepCopy()));
   }
   DispatchEventInternal(profile_, event_name, json_args, args.Pass());
 }