chrome/browser/extensions/api/downloads/downloads_api.cc - Issue 16924017: A few minor changes to the chrome.downloads extension API

Unified Diff: chrome/browser/extensions/api/downloads/downloads_api.cc

Issue 16924017: A few minor changes to the chrome.downloads extension API (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: @r212092 Created 7 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « chrome/browser/extensions/api/downloads/downloads_api.h ('k') | chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc » ('j') | chrome/common/extensions/api/downloads.idl » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/extensions/api/downloads/downloads_api.cc

diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc

index 0ebe8c59d6784b3ae00b31fd908ea2e2d89a0ff4..c52667cf721b0db6b55af8e8c6721486cc6e742e 100644

--- a/chrome/browser/extensions/api/downloads/downloads_api.cc

+++ b/chrome/browser/extensions/api/downloads/downloads_api.cc

@@ -14,6 +14,7 @@

#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"

@@ -30,9 +31,11 @@

#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"

@@ -43,8 +46,10 @@

#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"

@@ -76,23 +81,36 @@ 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.

@@ -100,28 +118,30 @@ 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";

@@ -214,30 +234,34 @@ scoped_ptr<base::DictionaryValue> DownloadItemToJSON(

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);

@@ -298,7 +322,6 @@ 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;

@@ -324,7 +347,6 @@ 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;

@@ -370,16 +392,6 @@ DownloadItem* GetDownload(Profile* profile, bool include_incognito, int 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,

@@ -393,6 +405,8 @@ enum DownloadsFunctionName {

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

};

@@ -404,7 +418,9 @@ void RecordApiFunctions(DownloadsFunctionName function) {

}

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 =

@@ -412,8 +428,6 @@ void CompileDownloadQueryOrderBy(

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;

@@ -427,7 +441,7 @@ void CompileDownloadQueryOrderBy(

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);

@@ -449,19 +463,24 @@ void RunDownloadQuery(

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);

@@ -472,7 +491,7 @@ void RunDownloadQuery(

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);

@@ -490,7 +509,7 @@ void RunDownloadQuery(

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;

}

@@ -512,6 +531,21 @@ void RunDownloadQuery(

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) {

@@ -530,6 +564,8 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {

: 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);

@@ -555,6 +591,10 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {

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() {

@@ -602,6 +642,28 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {

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|.

@@ -663,22 +725,15 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {

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(

@@ -697,6 +752,9 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {

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_;

@@ -794,6 +852,36 @@ void OnDeterminingFilenameWillDispatchCallback(

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*

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() {}

@@ -806,14 +894,13 @@ bool DownloadsDownloadFunction::RunImpl() {

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())

@@ -826,28 +913,24 @@ bool DownloadsDownloadFunction::RunImpl() {

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())

@@ -861,7 +944,7 @@ bool DownloadsDownloadFunction::RunImpl() {

++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);

@@ -875,7 +958,8 @@ bool DownloadsDownloadFunction::RunImpl() {

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);

@@ -887,12 +971,26 @@ bool DownloadsDownloadFunction::RunImpl() {

}

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);

@@ -946,20 +1044,17 @@ 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() {}

@@ -970,20 +1065,17 @@ 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() {}

@@ -994,9 +1086,10 @@ 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.

@@ -1034,6 +1127,50 @@ bool DownloadsEraseFunction::RunImpl() {

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() {}

@@ -1042,16 +1179,16 @@ 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.

@@ -1060,20 +1197,29 @@ bool DownloadsAcceptDangerFunction::RunImpl() {

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());

}

@@ -1086,13 +1232,19 @@ 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;

}

@@ -1107,10 +1259,10 @@ bool DownloadsOpenFunction::RunImpl() {

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;

@@ -1128,10 +1280,9 @@ bool DownloadsDragFunction::RunImpl() {

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);

@@ -1145,6 +1296,24 @@ bool DownloadsDragFunction::RunImpl() {

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 optional

benjhayden 2013/07/19 15:53:55 Done.

+ RecordApiFunctions(DOWNLOADS_FUNCTION_SET_SHELF_VISIBLE);

+ return true;

DownloadsGetFileIconFunction::DownloadsGetFileIconFunction()

: icon_extractor_(new DownloadFileIconExtractorImpl()) {

}

@@ -1168,10 +1337,10 @@ bool DownloadsGetFileIconFunction::RunImpl() {

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.

@@ -1186,13 +1355,13 @@ bool DownloadsGetFileIconFunction::RunImpl() {

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(

@@ -1267,7 +1436,14 @@ void ExtensionDownloadsEventRouter::OnDeterminingFilename(

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();

+ }

}

@@ -1281,28 +1457,19 @@ bool ExtensionDownloadsEventRouter::DetermineFilename(

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;

- return false;

- }

- if (item->GetState() != DownloadItem::IN_PROGRESS) {

- *error = download_extension_errors::kInvalidOperationError;

+ 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;

- }

base::FilePath::StringType filename_str(const_filename.value());

// Allow windows-style directory separators on all platforms.

std::replace(filename_str.begin(), filename_str.end(),

@@ -1311,17 +1478,13 @@ bool ExtensionDownloadsEventRouter::DetermineFilename(

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;

}

@@ -1357,7 +1520,8 @@ void ExtensionDownloadsEventRouter::OnListenerRemoved(

// should proceed.

data->DeterminerRemoved(details.extension_id);

}

- if (!any_listeners) {

+ if (!any_listeners &&

+ data->creator_suggested_filename().empty()) {

ExtensionDownloadsEventRouterData::Remove(*iter);

}

@@ -1427,7 +1591,7 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated(

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) &&

@@ -1444,7 +1608,9 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated(

// 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;

}