A few minor changes to the chrome.downloads extension API

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>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/download/download_danger_prompt.h"
 #include "chrome/browser/download/download_file_icon_extractor.h"
+#include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_query.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
+#include "chrome/browser/download/download_shelf.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/icon_loader.h"
 #include "chrome/browser/icon_manager.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/cancelable_task_tracker.h"
 #include "chrome/common/extensions/api/downloads.h"
 #include "chrome/common/extensions/permissions/permissions_data.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_save_info.h"
 #include "content/public/browser/download_url_parameters.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ skipping 11 matching lines @@
 namespace events = extensions::event_names;
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::DownloadItem;
 using content::DownloadManager;
 
 namespace download_extension_errors {
 
 // Error messages
-const char kGenericError[] = "I'm afraid I can't do that";
-const char kIconNotFoundError[] = "Icon not found";
-const char kInvalidDangerTypeError[] = "Invalid danger type";
-const char kInvalidFilenameError[] = "Invalid filename";
-const char kInvalidFilterError[] = "Invalid query filter";
-const char kInvalidOperationError[] = "Invalid operation";
-const char kInvalidOrderByError[] = "Invalid orderBy field";
+const char kEmptyFile[] = "Filename not yet determined";
+const char kFileAlreadyDeleted[] = "Download file already deleted";
+const char kFileIsDirectory[] = "Download file is a directory";
+const char kIconNotFound[] = "Icon not found";
+const char kInvalidDangerType[] = "Invalid danger type";
+const char kInvalidFilename[] = "Invalid filename";
+const char kInvalidFilter[] = "Invalid query filter";
+const char kInvalidHeader[] = "Invalid request header";
+const char kInvalidId[] = "Invalid downloadId";
+const char kInvalidOrderBy[] = "Invalid orderBy field";
 const char kInvalidQueryLimit[] = "Invalid query limit";
-const char kInvalidStateError[] = "Invalid state";
-const char kInvalidURLError[] = "Invalid URL";
-const char kNotImplementedError[] = "NotImplemented";
-const char kTooManyListenersError[] = "Each extension may have at most one "
+const char kInvalidState[] = "Invalid state";
+const char kInvalidURL[] = "Invalid URL";
+const char kInvisibleContext[] = "Javascript execution context is not visible "
+  "(tab, window, popup bubble)";
+const char kNotComplete[] = "Download must be complete";
+const char kNotDangerous[] = "Download must be dangerous";
+const char kNotImplemented[] = "NotImplemented";
+const char kNotInProgress[] = "Download must be in progress";
+const char kNotPermittedURL[] = "In order to access that URL, this extension "
+  "must add the host to \"permissions\" in manifest.json";
+const char kTooManyListeners[] = "Each extension may have at most one "
   "onDeterminingFilename listener between all of its renderer execution "
   "contexts.";
+const char kUnexpectedDeterminer[] = "Unexpected determineFilename call";
 
 }  // namespace download_extension_errors
 
+namespace errors = download_extension_errors;
+
 namespace {
 
 // Default icon size for getFileIcon() in pixels.
 const int  kDefaultIconSize = 32;
 
 // Parameter keys
 const char kBytesReceivedKey[] = "bytesReceived";
-const char kDangerAcceptedKey[] = "dangerAccepted";
+const char kCanResumeKey[] = "canResume";
+const char kDangerAccepted[] = "accepted";
 const char kDangerContent[] = "content";
 const char kDangerFile[] = "file";
+const char kDangerHost[] = "host";
 const char kDangerKey[] = "danger";
 const char kDangerSafe[] = "safe";
 const char kDangerUncommon[] = "uncommon";
-const char kDangerAccepted[] = "accepted";
-const char kDangerHost[] = "host";
 const char kDangerUrl[] = "url";
 const char kEndTimeKey[] = "endTime";
 const char kEndedAfterKey[] = "endedAfter";
 const char kEndedBeforeKey[] = "endedBefore";
 const char kErrorKey[] = "error";
+const char kEstimatedEndTimeKey[] = "estimatedEndTime";
 const char kExistsKey[] = "exists";
 const char kFileSizeKey[] = "fileSize";
 const char kFilenameKey[] = "filename";
 const char kFilenameRegexKey[] = "filenameRegex";
 const char kIdKey[] = "id";
-const char kIncognito[] = "incognito";
+const char kIncognitoKey[] = "incognito";
 const char kMimeKey[] = "mime";
 const char kPausedKey[] = "paused";
 const char kQueryKey[] = "query";
+const char kReferrerUrlKey[] = "referrer";
 const char kStartTimeKey[] = "startTime";
 const char kStartedAfterKey[] = "startedAfter";
 const char kStartedBeforeKey[] = "startedBefore";
 const char kStateComplete[] = "complete";
 const char kStateInProgress[] = "in_progress";
 const char kStateInterrupted[] = "interrupted";
 const char kStateKey[] = "state";
 const char kTotalBytesGreaterKey[] = "totalBytesGreater";
 const char kTotalBytesKey[] = "totalBytes";
 const char kTotalBytesLessKey[] = "totalBytesLess";
@@ skipping 72 matching lines @@
 }
 
 scoped_ptr<base::DictionaryValue> DownloadItemToJSON(
     DownloadItem* download_item,
     bool incognito) {
   base::DictionaryValue* json = new base::DictionaryValue();
   json->SetBoolean(kExistsKey, !download_item->GetFileExternallyRemoved());
   json->SetInteger(kIdKey, download_item->GetId());
   const GURL& url = download_item->GetOriginalUrl();
   json->SetString(kUrlKey, (url.is_valid() ? url.spec() : std::string()));
+  const GURL& referrer = download_item->GetReferrerUrl();
+  json->SetString(kReferrerUrlKey, (referrer.is_valid() ? referrer.spec()
+                                                        : std::string()));
   json->SetString(kFilenameKey,
                   download_item->GetTargetFilePath().LossyDisplayName());
   json->SetString(kDangerKey, DangerString(download_item->GetDangerType()));
-  if (download_item->GetDangerType() !=
-      content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS)
-    json->SetBoolean(kDangerAcceptedKey,
-                     download_item->GetDangerType() ==
-                     content::DOWNLOAD_DANGER_TYPE_USER_VALIDATED);
   json->SetString(kStateKey, StateString(download_item->GetState()));
+  json->SetBoolean(kCanResumeKey, download_item->CanResume());
   json->SetBoolean(kPausedKey, download_item->IsPaused());
   json->SetString(kMimeKey, download_item->GetMimeType());
   json->SetString(kStartTimeKey, TimeToISO8601(download_item->GetStartTime()));
   json->SetInteger(kBytesReceivedKey, download_item->GetReceivedBytes());
   json->SetInteger(kTotalBytesKey, download_item->GetTotalBytes());
-  json->SetBoolean(kIncognito, incognito);
+  json->SetBoolean(kIncognitoKey, incognito);
   if (download_item->GetState() == DownloadItem::INTERRUPTED) {
-    json->SetInteger(kErrorKey, static_cast<int>(
+    json->SetString(kErrorKey, content::InterruptReasonDebugString(
         download_item->GetLastReason()));
   } else if (download_item->GetState() == DownloadItem::CANCELLED) {
-    json->SetInteger(kErrorKey, static_cast<int>(
+    json->SetString(kErrorKey, content::InterruptReasonDebugString(
         content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED));
   }
   if (!download_item->GetEndTime().is_null())
     json->SetString(kEndTimeKey, TimeToISO8601(download_item->GetEndTime()));
+  base::TimeDelta time_remaining;
+  if (download_item->TimeRemaining(&time_remaining)) {
+    base::Time now = base::Time::Now();
+    json->SetString(kEstimatedEndTimeKey, TimeToISO8601(now + time_remaining));
+  }
   // TODO(benjhayden): Implement fileSize.
   json->SetInteger(kFileSizeKey, download_item->GetTotalBytes());
   return scoped_ptr<base::DictionaryValue>(json);
 }
 
 class DownloadFileIconExtractorImpl : public DownloadFileIconExtractor {
  public:
   DownloadFileIconExtractorImpl() {}
 
   virtual ~DownloadFileIconExtractorImpl() {}
@@ skipping 40 matching lines @@
     default:
       NOTREACHED();
       return IconLoader::NORMAL;
   }
 }
 
 typedef base::hash_map<std::string, DownloadQuery::FilterType> FilterTypeMap;
 
 void InitFilterTypeMap(FilterTypeMap& filter_types) {
   filter_types[kBytesReceivedKey] = DownloadQuery::FILTER_BYTES_RECEIVED;
-  filter_types[kDangerAcceptedKey] = DownloadQuery::FILTER_DANGER_ACCEPTED;
   filter_types[kExistsKey] = DownloadQuery::FILTER_EXISTS;
   filter_types[kFilenameKey] = DownloadQuery::FILTER_FILENAME;
   filter_types[kFilenameRegexKey] = DownloadQuery::FILTER_FILENAME_REGEX;
   filter_types[kMimeKey] = DownloadQuery::FILTER_MIME;
   filter_types[kPausedKey] = DownloadQuery::FILTER_PAUSED;
   filter_types[kQueryKey] = DownloadQuery::FILTER_QUERY;
   filter_types[kEndedAfterKey] = DownloadQuery::FILTER_ENDED_AFTER;
   filter_types[kEndedBeforeKey] = DownloadQuery::FILTER_ENDED_BEFORE;
   filter_types[kEndTimeKey] = DownloadQuery::FILTER_END_TIME;
   filter_types[kStartedAfterKey] = DownloadQuery::FILTER_STARTED_AFTER;
   filter_types[kStartedBeforeKey] = DownloadQuery::FILTER_STARTED_BEFORE;
   filter_types[kStartTimeKey] = DownloadQuery::FILTER_START_TIME;
   filter_types[kTotalBytesKey] = DownloadQuery::FILTER_TOTAL_BYTES;
   filter_types[kTotalBytesGreaterKey] =
     DownloadQuery::FILTER_TOTAL_BYTES_GREATER;
   filter_types[kTotalBytesLessKey] = DownloadQuery::FILTER_TOTAL_BYTES_LESS;
   filter_types[kUrlKey] = DownloadQuery::FILTER_URL;
   filter_types[kUrlRegexKey] = DownloadQuery::FILTER_URL_REGEX;
 }
 
 typedef base::hash_map<std::string, DownloadQuery::SortType> SortTypeMap;
 
 void InitSortTypeMap(SortTypeMap& sorter_types) {
   sorter_types[kBytesReceivedKey] = DownloadQuery::SORT_BYTES_RECEIVED;
   sorter_types[kDangerKey] = DownloadQuery::SORT_DANGER;
-  sorter_types[kDangerAcceptedKey] = DownloadQuery::SORT_DANGER_ACCEPTED;
   sorter_types[kEndTimeKey] = DownloadQuery::SORT_END_TIME;
   sorter_types[kExistsKey] = DownloadQuery::SORT_EXISTS;
   sorter_types[kFilenameKey] = DownloadQuery::SORT_FILENAME;
   sorter_types[kMimeKey] = DownloadQuery::SORT_MIME;
   sorter_types[kPausedKey] = DownloadQuery::SORT_PAUSED;
   sorter_types[kStartTimeKey] = DownloadQuery::SORT_START_TIME;
   sorter_types[kStateKey] = DownloadQuery::SORT_STATE;
   sorter_types[kTotalBytesKey] = DownloadQuery::SORT_TOTAL_BYTES;
   sorter_types[kUrlKey] = DownloadQuery::SORT_URL;
 }
@@ skipping 25 matching lines @@
 DownloadItem* GetDownload(Profile* profile, bool include_incognito, int id) {
   DownloadManager* manager = NULL;
   DownloadManager* incognito_manager = NULL;
   GetManagers(profile, include_incognito, &manager, &incognito_manager);
   DownloadItem* download_item = manager->GetDownload(id);
   if (!download_item && incognito_manager)
     download_item = incognito_manager->GetDownload(id);
   return download_item;
 }
 
-DownloadItem* GetDownloadIfInProgress(
-    Profile* profile,
-    bool include_incognito,
-    int id) {
-  DownloadItem* download_item = GetDownload(profile, include_incognito, id);
-  if (download_item && (download_item->GetState() == DownloadItem::IN_PROGRESS))
-    return download_item;
-  return NULL;
-}
-
 enum DownloadsFunctionName {
   DOWNLOADS_FUNCTION_DOWNLOAD = 0,
   DOWNLOADS_FUNCTION_SEARCH = 1,
   DOWNLOADS_FUNCTION_PAUSE = 2,
   DOWNLOADS_FUNCTION_RESUME = 3,
   DOWNLOADS_FUNCTION_CANCEL = 4,
   DOWNLOADS_FUNCTION_ERASE = 5,
   DOWNLOADS_FUNCTION_SET_DESTINATION = 6,
   DOWNLOADS_FUNCTION_ACCEPT_DANGER = 7,
   DOWNLOADS_FUNCTION_SHOW = 8,
   DOWNLOADS_FUNCTION_DRAG = 9,
   DOWNLOADS_FUNCTION_GET_FILE_ICON = 10,
   DOWNLOADS_FUNCTION_OPEN = 11,
+  DOWNLOADS_FUNCTION_DELETE_FILE = 12,
+  DOWNLOADS_FUNCTION_SET_SHELF_VISIBLE = 13,
   // Insert new values here, not at the beginning.
   DOWNLOADS_FUNCTION_LAST
 };
 
 void RecordApiFunctions(DownloadsFunctionName function) {
   UMA_HISTOGRAM_ENUMERATION("Download.ApiFunctions",
                             function,
                             DOWNLOADS_FUNCTION_LAST);
 }
 
 void CompileDownloadQueryOrderBy(
-    const std::string& order_by_str, std::string* error, DownloadQuery* query) {
+    const std::vector<std::string>& order_by_strs,
+    std::string* error,
+    DownloadQuery* query) {
   // TODO(benjhayden): Consider switching from LazyInstance to explicit string
   // comparisons.
   static base::LazyInstance<SortTypeMap> sorter_types =
     LAZY_INSTANCE_INITIALIZER;
   if (sorter_types.Get().size() == 0)
     InitSortTypeMap(sorter_types.Get());
 
-  std::vector<std::string> order_by_strs;
-  base::SplitString(order_by_str, ' ', &order_by_strs);
   for (std::vector<std::string>::const_iterator iter = order_by_strs.begin();
        iter != order_by_strs.end(); ++iter) {
     std::string term_str = *iter;
     if (term_str.empty())
       continue;
     DownloadQuery::SortDirection direction = DownloadQuery::ASCENDING;
     if (term_str[0] == '-') {
       direction = DownloadQuery::DESCENDING;
       term_str = term_str.substr(1);
     }
     SortTypeMap::const_iterator sorter_type =
         sorter_types.Get().find(term_str);
     if (sorter_type == sorter_types.Get().end()) {
-      *error = download_extension_errors::kInvalidOrderByError;
+      *error = errors::kInvalidOrderBy;
       return;
     }
     query->AddSorter(sorter_type->second, direction);
   }
 }
 
 void RunDownloadQuery(
     const extensions::api::downloads::DownloadQuery& query_in,
     DownloadManager* manager,
     DownloadManager* incognito_manager,
     std::string* error,
     DownloadQuery::DownloadVector* results) {
   // TODO(benjhayden): Consider switching from LazyInstance to explicit string
   // comparisons.
   static base::LazyInstance<FilterTypeMap> filter_types =
     LAZY_INSTANCE_INITIALIZER;
   if (filter_types.Get().size() == 0)
     InitFilterTypeMap(filter_types.Get());
 
   DownloadQuery query_out;
 
+  size_t limit = 1000;
   if (query_in.limit.get()) {
     if (*query_in.limit.get() < 0) {
-      *error = download_extension_errors::kInvalidQueryLimit;
+      *error = errors::kInvalidQueryLimit;
       return;
     }
-    query_out.Limit(*query_in.limit.get());
+    limit = *query_in.limit.get();
   }
+  if (limit > 0) {
+    query_out.Limit(limit);
+  }
+
   std::string state_string =
       extensions::api::downloads::ToString(query_in.state);
   if (!state_string.empty()) {
     DownloadItem::DownloadState state = StateEnumFromString(state_string);
     if (state == DownloadItem::MAX_DOWNLOAD_STATE) {
-      *error = download_extension_errors::kInvalidStateError;
+      *error = errors::kInvalidState;
       return;
     }
     query_out.AddFilter(state);
   }
   std::string danger_string =
       extensions::api::downloads::ToString(query_in.danger);
   if (!danger_string.empty()) {
     content::DownloadDangerType danger_type = DangerEnumFromString(
         danger_string);
     if (danger_type == content::DOWNLOAD_DANGER_TYPE_MAX) {
-      *error = download_extension_errors::kInvalidDangerTypeError;
+      *error = errors::kInvalidDangerType;
       return;
     }
     query_out.AddFilter(danger_type);
   }
   if (query_in.order_by.get()) {
     CompileDownloadQueryOrderBy(*query_in.order_by.get(), error, &query_out);
     if (!error->empty())
       return;
   }
 
   scoped_ptr<base::DictionaryValue> query_in_value(query_in.ToValue().Pass());
   for (base::DictionaryValue::Iterator query_json_field(*query_in_value.get());
        !query_json_field.IsAtEnd(); query_json_field.Advance()) {
     FilterTypeMap::const_iterator filter_type =
         filter_types.Get().find(query_json_field.key());
     if (filter_type != filter_types.Get().end()) {
       if (!query_out.AddFilter(filter_type->second, query_json_field.value())) {
-        *error = download_extension_errors::kInvalidFilterError;
+        *error = errors::kInvalidFilter;
         return;
       }
     }
   }
 
   DownloadQuery::DownloadVector all_items;
   if (query_in.id.get()) {
     DownloadItem* download_item = manager->GetDownload(*query_in.id.get());
     if (!download_item && incognito_manager)
       download_item = incognito_manager->GetDownload(*query_in.id.get());
     if (download_item)
       all_items.push_back(download_item);
   } else {
     manager->GetAllDownloads(&all_items);
     if (incognito_manager)
       incognito_manager->GetAllDownloads(&all_items);
   }
   query_out.AddFilter(base::Bind(&IsNotTemporaryDownloadFilter));
   query_out.Search(all_items.begin(), all_items.end(), results);
 }
 
+DownloadPathReservationTracker::FilenameConflictAction ConvertConflictAction(
+    extensions::api::downloads::FilenameConflictAction action) {
+  switch (action) {
+    case extensions::api::downloads::FILENAME_CONFLICT_ACTION_NONE:
+    case extensions::api::downloads::FILENAME_CONFLICT_ACTION_UNIQUIFY:
+      return DownloadPathReservationTracker::UNIQUIFY;
+    case extensions::api::downloads::FILENAME_CONFLICT_ACTION_OVERWRITE:
+      return DownloadPathReservationTracker::OVERWRITE;
+    case extensions::api::downloads::FILENAME_CONFLICT_ACTION_PROMPT:
+      return DownloadPathReservationTracker::PROMPT;
+  }
+  NOTREACHED();
+  return DownloadPathReservationTracker::UNIQUIFY;
+}
+
 class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {
  public:
   static ExtensionDownloadsEventRouterData* Get(DownloadItem* download_item) {
     base::SupportsUserData::Data* data = download_item->GetUserData(kKey);
     return (data == NULL) ? NULL :
         static_cast<ExtensionDownloadsEventRouterData*>(data);
   }
 
   static void Remove(DownloadItem* download_item) {
     download_item->RemoveUserData(kKey);
   }
 
   explicit ExtensionDownloadsEventRouterData(
       DownloadItem* download_item,
       scoped_ptr<base::DictionaryValue> json_item)
       : updated_(0),
         changed_fired_(0),
         json_(json_item.Pass()),
+        creator_conflict_action_(
+            extensions::api::downloads::FILENAME_CONFLICT_ACTION_UNIQUIFY),
         determined_conflict_action_(
             extensions::api::downloads::FILENAME_CONFLICT_ACTION_UNIQUIFY) {
     download_item->SetUserData(kKey, this);
   }
 
   virtual ~ExtensionDownloadsEventRouterData() {
     if (updated_ > 0) {
       UMA_HISTOGRAM_PERCENTAGE("Download.OnChanged",
                                (changed_fired_ * 100 / updated_));
     }
   }
 
   const base::DictionaryValue& json() const { return *json_.get(); }
   void set_json(scoped_ptr<base::DictionaryValue> json_item) {
     json_ = json_item.Pass();
   }
 
   void OnItemUpdated() { ++updated_; }
   void OnChangedFired() { ++changed_fired_; }
 
   void set_filename_change_callbacks(
       const base::Closure& no_change,
       const ExtensionDownloadsEventRouter::FilenameChangedCallback& change) {
     filename_no_change_ = no_change;
     filename_change_ = change;
+    determined_filename_ = creator_suggested_filename_;
+    determined_conflict_action_ = creator_conflict_action_;
+    // determiner_.install_time should default to 0 so that creator suggestions
+    // should be lower priority than any actual onDeterminingFilename listeners.
   }
 
   void ClearPendingDeterminers() {
     determined_filename_.clear();
     determined_conflict_action_ =
       extensions::api::downloads::FILENAME_CONFLICT_ACTION_UNIQUIFY;
     determiner_ = DeterminerInfo();
     filename_no_change_ = base::Closure();
     filename_change_ = ExtensionDownloadsEventRouter::FilenameChangedCallback();
     weak_ptr_factory_.reset();
@@ skipping 27 matching lines @@
 
   bool DeterminerAlreadyReported(const std::string& extension_id) {
     for (size_t index = 0; index < determiners_.size(); ++index) {
       if (determiners_[index].extension_id == extension_id) {
         return determiners_[index].reported;
       }
     }
     return false;
   }
 
+  void CreatorSuggestedFilename(
+      const base::FilePath& filename,
+      extensions::api::downloads::FilenameConflictAction conflict_action) {
+    creator_suggested_filename_ = filename;
+    creator_conflict_action_ = conflict_action;
+  }
+
+  base::FilePath creator_suggested_filename() const {
+    return creator_suggested_filename_;
+  }
+
+  extensions::api::downloads::FilenameConflictAction
+  creator_conflict_action() const {
+    return creator_conflict_action_;
+  }
+
+  void ResetCreatorSuggestion() {
+    creator_suggested_filename_.clear();
+    creator_conflict_action_ =
+      extensions::api::downloads::FILENAME_CONFLICT_ACTION_UNIQUIFY;
+  }
+
   // Returns false if this |extension_id| was not expected or if this
   // |extension_id| has already reported. The caller is responsible for
   // validating |filename|.
   bool DeterminerCallback(
       const std::string& extension_id,
       const base::FilePath& filename,
       extensions::api::downloads::FilenameConflictAction conflict_action) {
     bool found_info = false;
     for (size_t index = 0; index < determiners_.size(); ++index) {
       if (determiners_[index].extension_id == extension_id) {
@@ skipping 41 matching lines @@
     for (DeterminerInfoVector::iterator iter = determiners_.begin();
          iter != determiners_.end(); ++iter) {
       if (!iter->reported)
         return;
     }
     if (determined_filename_.empty()) {
       if (!filename_no_change_.is_null())
         filename_no_change_.Run();
     } else {
       if (!filename_change_.is_null()) {
-        DownloadPathReservationTracker::FilenameConflictAction conflict_action =
-          DownloadPathReservationTracker::UNIQUIFY;
-        if (determined_conflict_action_ ==
-            extensions::api::downloads::FILENAME_CONFLICT_ACTION_OVERWRITE)
-          conflict_action = DownloadPathReservationTracker::OVERWRITE;
-        if (determined_conflict_action_ ==
-            extensions::api::downloads::FILENAME_CONFLICT_ACTION_PROMPT)
-          conflict_action = DownloadPathReservationTracker::PROMPT;
-        filename_change_.Run(determined_filename_, conflict_action);
+        filename_change_.Run(determined_filename_, ConvertConflictAction(
+            determined_conflict_action_));
       }
     }
     // Don't clear determiners_ immediately in case there's a second listener
     // for one of the extensions, so that DetermineFilename can return
-    // kTooManyListenersError. After a few seconds, DetermineFilename will
-    // return kInvalidOperationError instead of kTooManyListenersError so that
-    // determiners_ doesn't keep hogging memory.
+    // kTooManyListeners. After a few seconds, DetermineFilename will return
+    // kUnexpectedDeterminer instead of kTooManyListeners so that determiners_
+    // doesn't keep hogging memory.
     weak_ptr_factory_.reset(
         new base::WeakPtrFactory<ExtensionDownloadsEventRouterData>(this));
     base::MessageLoopForUI::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&ExtensionDownloadsEventRouterData::ClearPendingDeterminers,
                    weak_ptr_factory_->GetWeakPtr()),
         base::TimeDelta::FromSeconds(30));
   }
 
   int updated_;
   int changed_fired_;
   scoped_ptr<base::DictionaryValue> json_;
 
   base::Closure filename_no_change_;
   ExtensionDownloadsEventRouter::FilenameChangedCallback filename_change_;
 
   DeterminerInfoVector determiners_;
 
+  base::FilePath creator_suggested_filename_;
+  extensions::api::downloads::FilenameConflictAction
+    creator_conflict_action_;
   base::FilePath determined_filename_;
   extensions::api::downloads::FilenameConflictAction
     determined_conflict_action_;
   DeterminerInfo determiner_;
 
   scoped_ptr<base::WeakPtrFactory<ExtensionDownloadsEventRouterData> >
     weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionDownloadsEventRouterData);
 };
@@ skipping 77 matching lines @@
     Profile* profile,
     const extensions::Extension* extension,
     base::ListValue* event_args) {
   *any_determiners = true;
   base::Time installed = extensions::ExtensionSystem::Get(
       profile)->extension_service()->extension_prefs()->
     GetInstallTime(extension->id());
   data->AddPendingDeterminer(extension->id(), installed);
 }
 
+bool Fault(
+    std::string* message_out,

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

style nit: no newline after opening (

[benjhayden, 2013/07/19 15:53:55]

Done.

+    bool error,
+    const std::string& message_in) {

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

Would it make sense to have this take const char* instead, and only do the
autoconversion to std::string a little below in the body if |error| is true?
That would avoid unneeded std::string object creations right? (This is called so
infrequently that it probably doesn't matter one way or the other)

One other minor point - it might read slightly better at the call sites if
|error| were the first argument instead of the second, since that's the
condition you care about as you're reading the line.

[benjhayden, 2013/07/19 15:53:55]

Done.

+  if (!error)
+    return false;
+  *message_out = message_in;
+  return true;
+}
+
+bool InvalidId(std::string* message_out, bool valid_item) {
+  return Fault(message_out, !valid_item, errors::kInvalidId);
+}
+
+bool IsDownloadDeltaField(const std::string& field) {
+  return ((field == kUrlKey) ||
+          (field == kFilenameKey) ||
+          (field == kDangerKey) ||
+          (field == kMimeKey) ||
+          (field == kStartTimeKey) ||
+          (field == kEndTimeKey) ||
+          (field == kStateKey) ||
+          (field == kCanResumeKey) ||
+          (field == kPausedKey) ||
+          (field == kErrorKey) ||
+          (field == kTotalBytesKey) ||
+          (field == kFileSizeKey) ||
+          (field == kExistsKey));
+}
+
 }  // namespace
 
 DownloadsDownloadFunction::DownloadsDownloadFunction() {}
 
 DownloadsDownloadFunction::~DownloadsDownloadFunction() {}
 
 bool DownloadsDownloadFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Download::Params> params(
       extensions::api::downloads::Download::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   const extensions::api::downloads::DownloadOptions& options = params->options;
   GURL download_url(options.url);
-  if (!download_url.is_valid() ||
-      (!download_url.SchemeIs("data") &&
-       download_url.GetOrigin() != GetExtension()->url().GetOrigin() &&
-       !extensions::PermissionsData::HasHostPermission(GetExtension(),
-                                                       download_url))) {
-    error_ = download_extension_errors::kInvalidURLError;
+  if (Fault(&error_, !download_url.is_valid(), errors::kInvalidURL) ||
+      Fault(&error_, (
+          !download_url.SchemeIs("data") &&
+          (download_url.GetOrigin() != GetExtension()->url().GetOrigin()) &&
+          !extensions::PermissionsData::HasHostPermission(
+              GetExtension(), download_url)), errors::kNotPermittedURL))
     return false;
-  }
 
   Profile* current_profile = profile();
   if (include_incognito() && profile()->HasOffTheRecordProfile())
     current_profile = profile()->GetOffTheRecordProfile();
 
   scoped_ptr<content::DownloadUrlParameters> download_params(
       new content::DownloadUrlParameters(
           download_url,
           render_view_host()->GetProcess()->GetID(),
           render_view_host()->GetRoutingID(),
           current_profile->GetResourceContext()));
 
+  base::FilePath creator_suggested_filename;
   if (options.filename.get()) {
-    // TODO(benjhayden): Make json_schema_compiler generate string16s instead of
-    // std::strings. Can't get filename16 from options.ToValue() because that
-    // converts it from std::string.
+#if defined(OS_WIN)
+    // Can't get filename16 from options.ToValue() because that converts it from
+    // std::string.
     base::DictionaryValue* options_value = NULL;
     EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &options_value));
-    string16 filename16;
+    base::string16 filename16;
     EXTENSION_FUNCTION_VALIDATE(options_value->GetString(
         kFilenameKey, &filename16));
-#if defined(OS_WIN)
-    base::FilePath file_path(filename16);
+    creator_suggested_filename = base::FilePath(filename16);
 #elif defined(OS_POSIX)
-    base::FilePath file_path(*options.filename.get());
+    creator_suggested_filename = base::FilePath(*options.filename.get());
 #endif
-    if (!net::IsSafePortableBasename(file_path) ||
-        (file_path.DirName().value() != base::FilePath::kCurrentDirectory)) {
-      error_ = download_extension_errors::kInvalidFilenameError;
+    if (!net::IsSafePortableRelativePath(creator_suggested_filename)) {
+      error_ = errors::kInvalidFilename;
       return false;
     }
-    // TODO(benjhayden) Ensure that this filename is interpreted as a path
-    // relative to the default downloads directory without allowing '..'.
-    download_params->set_suggested_name(filename16);
   }
 
   if (options.save_as.get())
     download_params->set_prompt(*options.save_as.get());
 
   if (options.headers.get()) {
     typedef extensions::api::downloads::HeaderNameValuePair HeaderNameValuePair;
     for (std::vector<linked_ptr<HeaderNameValuePair> >::const_iterator iter =
          options.headers->begin();
          iter != options.headers->end();
          ++iter) {
       const HeaderNameValuePair& name_value = **iter;
       if (!net::HttpUtil::IsSafeHeader(name_value.name)) {
-        error_ = download_extension_errors::kGenericError;
+        error_ = errors::kInvalidHeader;
         return false;
       }
       download_params->add_request_header(name_value.name, name_value.value);
     }
   }
 
   std::string method_string =
       extensions::api::downloads::ToString(options.method);
   if (!method_string.empty())
     download_params->set_method(method_string);
   if (options.body.get())
     download_params->set_post_body(*options.body.get());
   download_params->set_callback(base::Bind(
-      &DownloadsDownloadFunction::OnStarted, this));
+      &DownloadsDownloadFunction::OnStarted, this,
+      creator_suggested_filename, options.conflict_action));
   // Prevent login prompts for 401/407 responses.
   download_params->set_load_flags(net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
 
   DownloadManager* manager = BrowserContext::GetDownloadManager(
       current_profile);
   manager->DownloadUrl(download_params.Pass());
   RecordApiFunctions(DOWNLOADS_FUNCTION_DOWNLOAD);
   return true;
 }
 
 void DownloadsDownloadFunction::OnStarted(
-    DownloadItem* item, net::Error error) {
+    const base::FilePath& creator_suggested_filename,
+    extensions::api::downloads::FilenameConflictAction creator_conflict_action,
+    DownloadItem* item,
+    net::Error error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   VLOG(1) << __FUNCTION__ << " " << item << " " << error;
   if (item) {
     DCHECK_EQ(net::OK, error);
     SetResult(base::Value::CreateIntegerValue(item->GetId()));
+    if (!creator_suggested_filename.empty()) {
+      ExtensionDownloadsEventRouterData* data =
+          ExtensionDownloadsEventRouterData::Get(item);
+      if (!data) {
+        data = new ExtensionDownloadsEventRouterData(
+            item,
+            scoped_ptr<base::DictionaryValue>(new base::DictionaryValue()));
+      }
+      data->CreatorSuggestedFilename(
+          creator_suggested_filename, creator_conflict_action);
+    }
   } else {
     DCHECK_NE(net::OK, error);
     error_ = net::ErrorToString(error);
   }
   SendResponse(error_.empty());
 }
 
 DownloadsSearchFunction::DownloadsSearchFunction() {}
 
 DownloadsSearchFunction::~DownloadsSearchFunction() {}
@@ skipping 33 matching lines @@
 }
 
 DownloadsPauseFunction::DownloadsPauseFunction() {}
 
 DownloadsPauseFunction::~DownloadsPauseFunction() {}
 
 bool DownloadsPauseFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Pause::Params> params(
       extensions::api::downloads::Pause::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  DownloadItem* download_item = GetDownloadIfInProgress(
+  DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
-  if (download_item == NULL) {
-    // This could be due to an invalid download ID, or it could be due to the
-    // download not being currently active.
-    error_ = download_extension_errors::kInvalidOperationError;
-  } else {
-    // If the item is already paused, this is a no-op and the
-    // operation will silently succeed.
-    download_item->Pause();
-  }
-  if (error_.empty())
-    RecordApiFunctions(DOWNLOADS_FUNCTION_PAUSE);
-  return error_.empty();
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetState() != DownloadItem::IN_PROGRESS,
+            errors::kNotInProgress))
+    return false;
+  // If the item is already paused, this is a no-op and the operation will
+  // silently succeed.
+  download_item->Pause();
+  RecordApiFunctions(DOWNLOADS_FUNCTION_PAUSE);
+  return true;
 }
 
 DownloadsResumeFunction::DownloadsResumeFunction() {}
 
 DownloadsResumeFunction::~DownloadsResumeFunction() {}
 
 bool DownloadsResumeFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Resume::Params> params(
       extensions::api::downloads::Resume::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  DownloadItem* download_item = GetDownloadIfInProgress(
+  DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
-  if (download_item == NULL) {
-    // This could be due to an invalid download ID, or it could be due to the
-    // download not being currently active.
-    error_ = download_extension_errors::kInvalidOperationError;
-  } else {
-    // Note that if the item isn't paused, this will be a no-op, and
-    // the extension call will seem successful.
-    download_item->Resume();
-  }
-  if (error_.empty())
-    RecordApiFunctions(DOWNLOADS_FUNCTION_RESUME);
-  return error_.empty();
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetState() != DownloadItem::IN_PROGRESS,
+            errors::kNotInProgress))
+    return false;
+  // Note that if the item isn't paused, this will be a no-op, and the extension
+  // call will seem successful.
+  download_item->Resume();
+  RecordApiFunctions(DOWNLOADS_FUNCTION_RESUME);
+  return true;
 }
 
 DownloadsCancelFunction::DownloadsCancelFunction() {}
 
 DownloadsCancelFunction::~DownloadsCancelFunction() {}
 
 bool DownloadsCancelFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Resume::Params> params(
       extensions::api::downloads::Resume::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  DownloadItem* download_item = GetDownloadIfInProgress(
+  DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
-  if (download_item != NULL)
+  if (download_item &&
+      (download_item->GetState() == DownloadItem::IN_PROGRESS))
     download_item->Cancel(true);
   // |download_item| can be NULL if the download ID was invalid or if the
   // download is not currently active.  Either way, it's not a failure.
   RecordApiFunctions(DOWNLOADS_FUNCTION_CANCEL);
   return true;
 }
 
 DownloadsEraseFunction::DownloadsEraseFunction() {}
 
 DownloadsEraseFunction::~DownloadsEraseFunction() {}
@@ skipping 17 matching lines @@
   for (DownloadManager::DownloadVector::const_iterator it = results.begin();
        it != results.end(); ++it) {
     json_results->Append(base::Value::CreateIntegerValue((*it)->GetId()));
     (*it)->Remove();
   }
   SetResult(json_results);
   RecordApiFunctions(DOWNLOADS_FUNCTION_ERASE);
   return true;
 }
 
+DownloadsDeleteFileFunction::DownloadsDeleteFileFunction() {}
+
+DownloadsDeleteFileFunction::~DownloadsDeleteFileFunction() {}
+
+bool DownloadsDeleteFileFunction::RunImpl() {
+  scoped_ptr<extensions::api::downloads::DeleteFile::Params> params(
+      extensions::api::downloads::DeleteFile::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  DownloadItem* download_item = GetDownload(
+      profile(), include_incognito(), params->download_id);
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, (download_item->GetState() != DownloadItem::COMPLETE),
+            errors::kNotComplete) ||
+      Fault(&error_, download_item->GetFileExternallyRemoved(),
+            errors::kFileAlreadyDeleted))
+    return false;
+  RecordApiFunctions(DOWNLOADS_FUNCTION_DELETE_FILE);
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadsDeleteFileFunction::DeleteOnFileThread, this,
+                 download_item->GetFullPath()),
+      base::Bind(&DownloadsDeleteFileFunction::RespondOnUIThread, this));
+  return true;
+}
+
+void DownloadsDeleteFileFunction::DeleteOnFileThread(
+    const base::FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  if (Fault(&error_, file_util::DirectoryExists(path),
+            errors::kFileIsDirectory))
+    return;
+  base::Delete(path, false);
+}
+
+void DownloadsDeleteFileFunction::RespondOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadManager* manager = NULL;
+  DownloadManager* incognito_manager = NULL;
+  GetManagers(profile(), include_incognito(), &manager, &incognito_manager);
+  ManagerDestructionObserver::CheckForHistoryFilesRemoval(manager);
+  ManagerDestructionObserver::CheckForHistoryFilesRemoval(incognito_manager);
+  SendResponse(error_.empty());
+}
+
 DownloadsAcceptDangerFunction::DownloadsAcceptDangerFunction() {}
 
 DownloadsAcceptDangerFunction::~DownloadsAcceptDangerFunction() {}
 
 bool DownloadsAcceptDangerFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::AcceptDanger::Params> params(
       extensions::api::downloads::AcceptDanger::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  DownloadItem* download_item = GetDownloadIfInProgress(
+  DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
   content::WebContents* web_contents =
       dispatcher()->delegate()->GetVisibleWebContents();
-  if (!download_item ||
-      !download_item->IsDangerous() ||
-      !web_contents) {
-    error_ = download_extension_errors::kInvalidOperationError;
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetState() != DownloadItem::IN_PROGRESS,
+            errors::kNotInProgress) ||
+      Fault(&error_, !download_item->IsDangerous(), errors::kNotDangerous) ||
+      Fault(&error_, !web_contents, errors::kInvisibleContext))
     return false;
-  }
   RecordApiFunctions(DOWNLOADS_FUNCTION_ACCEPT_DANGER);
   // DownloadDangerPrompt displays a modal dialog using native widgets that the
   // user must either accept or cancel. It cannot be scripted.
   DownloadDangerPrompt::Create(
       download_item,
       web_contents,
       true,
       base::Bind(&DownloadsAcceptDangerFunction::DangerPromptCallback,
-                 this, true, params->download_id),
-      base::Bind(&DownloadsAcceptDangerFunction::DangerPromptCallback,
-                 this, false, params->download_id));
+                 this, params->download_id));
   // DownloadDangerPrompt deletes itself
   return true;
 }
 
 void DownloadsAcceptDangerFunction::DangerPromptCallback(
-    bool accept, int download_id) {
-  if (accept) {
-    DownloadItem* download_item = GetDownloadIfInProgress(
-        profile(), include_incognito(), download_id);
-    if (download_item)
+    int download_id, DownloadDangerPrompt::Action action) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadItem* download_item = GetDownload(
+      profile(), include_incognito(), download_id);
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetState() != DownloadItem::IN_PROGRESS,
+            errors::kNotInProgress))
+    return;
+  switch (action) {
+    case DownloadDangerPrompt::ACCEPT:
       download_item->ValidateDangerousDownload();
+      break;
+    case DownloadDangerPrompt::CANCEL:
+      download_item->Remove();
+      break;
+    case DownloadDangerPrompt::DISMISS:
+      break;
   }
   SendResponse(error_.empty());
 }
 
 DownloadsShowFunction::DownloadsShowFunction() {}
 
 DownloadsShowFunction::~DownloadsShowFunction() {}
 
 bool DownloadsShowFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Show::Params> params(
       extensions::api::downloads::Show::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  DownloadItem* download_item = GetDownload(
-      profile(), include_incognito(), params->download_id);
-  if (!download_item) {
-    error_ = download_extension_errors::kInvalidOperationError;
-    return false;
+  if (params->download_id) {
+    DownloadItem* download_item = GetDownload(
+        profile(), include_incognito(), *params->download_id);
+    if (InvalidId(&error_, download_item))
+      return false;
+    download_item->ShowDownloadInShell();
+  } else {
+    DownloadManager* manager = NULL;
+    DownloadManager* incognito_manager = NULL;
+    GetManagers(profile(), include_incognito(), &manager, &incognito_manager);
+    platform_util::OpenItem(DownloadPrefs::FromDownloadManager(
+        manager)->DownloadPath());
   }
-  download_item->ShowDownloadInShell();
   RecordApiFunctions(DOWNLOADS_FUNCTION_SHOW);
   return true;
 }
 
 DownloadsOpenFunction::DownloadsOpenFunction() {}
 
 DownloadsOpenFunction::~DownloadsOpenFunction() {}
 
 bool DownloadsOpenFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Open::Params> params(
       extensions::api::downloads::Open::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
-  if (!download_item || download_item->GetState() != DownloadItem::COMPLETE) {
-    error_ = download_extension_errors::kInvalidOperationError;
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetState() != DownloadItem::COMPLETE,
+            errors::kNotComplete))
     return false;
-  }
   download_item->OpenDownload();
   RecordApiFunctions(DOWNLOADS_FUNCTION_OPEN);
   return true;
 }
 
 DownloadsDragFunction::DownloadsDragFunction() {}
 
 DownloadsDragFunction::~DownloadsDragFunction() {}
 
 bool DownloadsDragFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Drag::Params> params(
       extensions::api::downloads::Drag::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
   content::WebContents* web_contents =
       dispatcher()->delegate()->GetVisibleWebContents();
-  if (!download_item || !web_contents) {
-    error_ = download_extension_errors::kInvalidOperationError;
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, !web_contents, errors::kInvisibleContext))
     return false;
-  }
   RecordApiFunctions(DOWNLOADS_FUNCTION_DRAG);
   gfx::Image* icon = g_browser_process->icon_manager()->LookupIconFromFilepath(
       download_item->GetTargetFilePath(), IconLoader::NORMAL);
   gfx::NativeView view = web_contents->GetView()->GetNativeView();
   {
     // Enable nested tasks during DnD, while |DragDownload()| blocks.
     base::MessageLoop::ScopedNestableTaskAllower allow(
         base::MessageLoop::current());
     download_util::DragDownload(download_item, icon, view);
   }
   return true;
 }
 
+DownloadsSetShelfVisibleFunction::DownloadsSetShelfVisibleFunction() {}
+
+DownloadsSetShelfVisibleFunction::~DownloadsSetShelfVisibleFunction() {}
+
+bool DownloadsSetShelfVisibleFunction::RunImpl() {
+  scoped_ptr<extensions::api::downloads::SetShelfVisible::Params> params(
+      extensions::api::downloads::SetShelfVisible::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  DownloadShelf* shelf = GetCurrentBrowser()->window()->GetDownloadShelf();
+  if (params->visible) {
+    shelf->Unhide();
+  } else {
+    shelf->Hide();
+  }

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

style nit: single line if/else bodies can optionally omit braces (looks like you
do so elsewhere in the file, so you should probably do so here too)

[benjhayden, 2013/07/19 15:53:55]

Done.

+  RecordApiFunctions(DOWNLOADS_FUNCTION_SET_SHELF_VISIBLE);
+  return true;
+}
+
 DownloadsGetFileIconFunction::DownloadsGetFileIconFunction()
     : icon_extractor_(new DownloadFileIconExtractorImpl()) {
 }
 
 DownloadsGetFileIconFunction::~DownloadsGetFileIconFunction() {}
 
 void DownloadsGetFileIconFunction::SetIconExtractorForTesting(
     DownloadFileIconExtractor* extractor) {
   DCHECK(extractor);
   icon_extractor_.reset(extractor);
 }
 
 bool DownloadsGetFileIconFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::GetFileIcon::Params> params(
       extensions::api::downloads::GetFileIcon::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   const extensions::api::downloads::GetFileIconOptions* options =
       params->options.get();
   int icon_size = kDefaultIconSize;
   if (options && options->size.get())
     icon_size = *options->size.get();
   DownloadItem* download_item = GetDownload(
       profile(), include_incognito(), params->download_id);
-  if (!download_item || download_item->GetTargetFilePath().empty()) {
-    error_ = download_extension_errors::kInvalidOperationError;
+  if (InvalidId(&error_, download_item) ||
+      Fault(&error_, download_item->GetTargetFilePath().empty(),
+            errors::kEmptyFile))
     return false;
-  }
   // In-progress downloads return the intermediate filename for GetFullPath()
   // which doesn't have the final extension. Therefore a good file icon can't be
   // found, so use GetTargetFilePath() instead.
   DCHECK(icon_extractor_.get());
   DCHECK(icon_size == 16 || icon_size == 32);
   EXTENSION_FUNCTION_VALIDATE(icon_extractor_->ExtractIconURLForPath(
       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);
-    SetResult(base::Value::CreateStringValue(url));
+  if (Fault(&error_, url.empty(), errors::kIconNotFound)) {
+    SendResponse(false);
+    return;
   }
-  SendResponse(error_.empty());
+  RecordApiFunctions(DOWNLOADS_FUNCTION_GET_FILE_ICON);
+  SetResult(base::Value::CreateStringValue(url));
+  SendResponse(true);
 }
 
 ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
     Profile* profile,
     DownloadManager* manager)
     : profile_(profile),
       notifier_(manager, this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(profile_);
   extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)->
@@ skipping 54 matching lines @@
       item, profile_->IsOffTheRecord()).release();
   json->SetString(kFilenameKey, suggested_path.LossyDisplayName());
   DispatchEvent(events::kOnDownloadDeterminingFilename,
                 false,
                 base::Bind(&OnDeterminingFilenameWillDispatchCallback,
                            &any_determiners,
                            data),
                 json);
   if (!any_determiners) {
     data->ClearPendingDeterminers();
-    no_change.Run();
+    if (!data->creator_suggested_filename().empty()) {
+      change.Run(data->creator_suggested_filename(),
+                 ConvertConflictAction(data->creator_conflict_action()));
+      // If all listeners are removed, don't keep |data| around.
+      data->ResetCreatorSuggestion();
+    } else {
+      no_change.Run();
+    }
   }
 }
 
 bool ExtensionDownloadsEventRouter::DetermineFilename(
     Profile* profile,
     bool include_incognito,
     const std::string& ext_id,
     int download_id,
     const base::FilePath& const_filename,
     extensions::api::downloads::FilenameConflictAction conflict_action,
     std::string* error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* item = GetDownload(profile, include_incognito, download_id);
-  if (!item) {
-    *error = download_extension_errors::kInvalidOperationError;
-    return false;
-  }
   ExtensionDownloadsEventRouterData* data =
-      ExtensionDownloadsEventRouterData::Get(item);
-  if (!data) {
-    *error = download_extension_errors::kInvalidOperationError;
-    return false;
-  }
+      item ? ExtensionDownloadsEventRouterData::Get(item) : NULL;
   // maxListeners=1 in downloads.idl and suggestCallback in
   // downloads_custom_bindings.js should prevent duplicate DeterminerCallback
   // calls from the same renderer, but an extension may have more than one
   // renderer, so don't DCHECK(!reported).
-  if (data->DeterminerAlreadyReported(ext_id)) {
-    *error = download_extension_errors::kTooManyListenersError;
+  if (InvalidId(error, item) ||
+      Fault(error, item->GetState() != DownloadItem::IN_PROGRESS,
+            errors::kNotInProgress) ||
+      Fault(error, !data, errors::kUnexpectedDeterminer) ||
+      Fault(error, data->DeterminerAlreadyReported(ext_id),
+            errors::kTooManyListeners))
     return false;
-  }
-  if (item->GetState() != DownloadItem::IN_PROGRESS) {
-    *error = download_extension_errors::kInvalidOperationError;
-    return false;
-  }
   base::FilePath::StringType filename_str(const_filename.value());
   // Allow windows-style directory separators on all platforms.
   std::replace(filename_str.begin(), filename_str.end(),
                FILE_PATH_LITERAL('\\'), FILE_PATH_LITERAL('/'));
   base::FilePath filename(filename_str);
   bool valid_filename = net::IsSafePortableRelativePath(filename);
   filename = (valid_filename ? filename.NormalizePathSeparators() :
               base::FilePath());
-  if (!data->DeterminerCallback(ext_id, filename, conflict_action)) {
-    // Nobody expects this ext_id!
-    *error = download_extension_errors::kInvalidOperationError;
+  // If the invalid filename check is moved to before DeterminerCallback(), then
+  // it will block forever waiting for this ext_id to report.
+  if (Fault(error, !data->DeterminerCallback(
+          ext_id, filename, conflict_action), errors::kUnexpectedDeterminer) ||
+      Fault(error, (!const_filename.empty() && !valid_filename),
+            errors::kInvalidFilename))
     return false;
-  }
-  if (!const_filename.empty() && !valid_filename) {
-    // If this is moved to before DeterminerCallback(), then it will block
-    // forever waiting for this ext_id to report.
-    *error = download_extension_errors::kInvalidFilenameError;
-    return false;
-  }
   return true;
 }
 
 void ExtensionDownloadsEventRouter::OnListenerRemoved(
     const extensions::EventListenerInfo& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadManager* manager = notifier_.GetManager();
   if (!manager)
     return;
   bool determiner_removed = (
@@ skipping 15 matching lines @@
     if (!data)
       continue;
     if (determiner_removed) {
       // Notify any items that may be waiting for callbacks from this
       // extension/determiner.  This will almost always be a no-op, however, it
       // is possible for an extension renderer to be unloaded while a download
       // item is waiting for a determiner. In that case, the download item
       // should proceed.
       data->DeterminerRemoved(details.extension_id);
     }
-    if (!any_listeners) {
+    if (!any_listeners &&
+        data->creator_suggested_filename().empty()) {
       ExtensionDownloadsEventRouterData::Remove(*iter);
     }
   }
 }
 
 // That's all the methods that have to do with filename determination. The rest
 // have to do with the other, less special events.
 
 void ExtensionDownloadsEventRouter::OnDownloadCreated(
     DownloadManager* manager, DownloadItem* download_item) {
@@ skipping 49 matching lines @@
   std::set<std::string> new_fields;
   bool changed = false;
 
   // For each field in the new json representation of the download_item except
   // the bytesReceived field, if the field has changed from the previous old
   // json, set the differences in the |delta| object and remember that something
   // significant changed.
   for (base::DictionaryValue::Iterator iter(*new_json.get());
        !iter.IsAtEnd(); iter.Advance()) {
     new_fields.insert(iter.key());
-    if (iter.key() != kBytesReceivedKey) {
+    if (IsDownloadDeltaField(iter.key())) {
       const base::Value* old_value = NULL;
       if (!data->json().HasKey(iter.key()) ||
           (data->json().Get(iter.key(), &old_value) &&
            !iter.value().Equals(old_value))) {
         delta->Set(iter.key() + ".current", iter.value().DeepCopy());
         if (old_value)
           delta->Set(iter.key() + ".previous", old_value->DeepCopy());
         changed = true;
       }
     }
   }
 
   // If a field was in the previous json but is not in the new json, set the
   // difference in |delta|.
   for (base::DictionaryValue::Iterator iter(data->json());
        !iter.IsAtEnd(); iter.Advance()) {
-    if (new_fields.find(iter.key()) == new_fields.end()) {
+    if ((new_fields.find(iter.key()) == new_fields.end()) &&
+        IsDownloadDeltaField(iter.key())) {
+      // estimatedEndTime disappears after completion, but bytesReceived stays.
       delta->Set(iter.key() + ".previous", iter.value().DeepCopy());
       changed = true;
     }
   }
 
   // Update the OnChangedStat and dispatch the event if something significant
   // changed. Replace the stored json with the new json.
   data->OnItemUpdated();
   if (changed) {
     DispatchEvent(events::kOnDownloadChanged,
@@ skipping 43 matching lines @@
   DownloadsNotificationSource notification_source;
   notification_source.event_name = event_name;
   notification_source.profile = profile_;
   content::Source<DownloadsNotificationSource> content_source(
       &notification_source);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT,
       content_source,
       content::Details<std::string>(&json_args));
 }