Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1385)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/extensions/api/downloads/downloads_api_unittest.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: @r211135 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
« chrome/browser/extensions/api/downloads/downloads_api.cc ('K') | « chrome/browser/extensions/api/downloads/downloads_api.cc ('k') | chrome/browser/extensions/extension_function_histogram_value.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
diff --git a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
index b3f08d058aa635c184589c5f7f0a0492bf98be7a..3d06d135476d04e738ef87de11a9d769811d2af1 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
@@ -182,6 +182,7 @@ class DownloadsEventsListener : public content::NotificationObserver {
dns->profile,
dns->event_name,
*content::Details<std::string>(details).ptr(), base::Time::Now());
+ LOG(INFO) << "occam caught " << new_event->Debug();
asanka 2013/07/16 19:57:52 to be removed?
benjhayden 2013/07/19 15:53:55 Done.
events_.push_back(new_event);
if (waiting_ &&
waiting_for_.get() &&
@@ -206,6 +207,7 @@ class DownloadsEventsListener : public content::NotificationObserver {
return true;
}
}
+ LOG(INFO) << "occam waiting " << waiting_for_->Debug();
waiting_ = true;
content::RunMessageLoop();
bool success = !waiting_;
@@ -349,19 +351,22 @@ class DownloadExtensionTest : public ExtensionApiTest {
current_browser()->profile(), event_name, json_args);
}
- bool WaitForInterruption(DownloadItem* item, int expected_error,
- const std::string& on_created_event) {
+ bool WaitForInterruption(
+ DownloadItem* item,
+ content::DownloadInterruptReason expected_error,
+ const std::string& on_created_event) {
if (!WaitFor(events::kOnDownloadCreated, on_created_event))
return false;
// Now, onCreated is always fired before interruption.
return WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
- " \"error\": {\"current\": %d},"
- " \"state\": {"
- " \"previous\": \"in_progress\","
- " \"current\": \"interrupted\"}}]",
- item->GetId(),
- expected_error));
+ " \"error\": {\"current\": \"%s\"},"
+ " \"state\": {"
+ " \"previous\": \"in_progress\","
+ " \"current\": \"interrupted\"}}]",
+ item->GetId(),
+ content::InterruptReasonDebugString(
+ expected_error).c_str()));
}
void ClearEvents() {
@@ -1242,7 +1247,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
&items));
scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
- new DownloadsSearchFunction(), "[{\"orderBy\": \"filename\"}]"));
+ new DownloadsSearchFunction(), "[{\"orderBy\": [\"filename\"]}]"));
ASSERT_TRUE(result.get());
base::ListValue* result_list = NULL;
ASSERT_TRUE(result->GetAsList(&result_list));
@@ -1275,7 +1280,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
&items));
scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
- new DownloadsSearchFunction(), "[{\"orderBy\": \"\"}]"));
+ new DownloadsSearchFunction(), "[{\"orderBy\": []}]"));
ASSERT_TRUE(result.get());
base::ListValue* result_list = NULL;
ASSERT_TRUE(result->GetAsList(&result_list));
@@ -1355,7 +1360,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
EXPECT_STREQ(download_extension_errors::kInvalidFilterError,
error.c_str());
error = RunFunctionAndReturnError(
- new DownloadsSearchFunction(), "[{\"orderBy\": \"goat\"}]");
+ new DownloadsSearchFunction(), "[{\"orderBy\": [\"goat\"]}]");
EXPECT_STREQ(download_extension_errors::kInvalidOrderByError,
error.c_str());
error = RunFunctionAndReturnError(
@@ -1386,7 +1391,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
new DownloadsSearchFunction(), "[{"
"\"state\": \"complete\", "
"\"danger\": \"content\", "
- "\"orderBy\": \"filename\", "
+ "\"orderBy\": [\"filename\"], "
"\"limit\": 1}]"));
ASSERT_TRUE(result.get());
base::ListValue* result_list = NULL;
@@ -1671,7 +1676,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
for (size_t index = 0; index < arraysize(kUnsafeHeaders); ++index) {
std::string download_url = test_server()->GetURL("slow?0").spec();
- EXPECT_STREQ(download_extension_errors::kGenericError,
+ EXPECT_STREQ(download_extension_errors::kInvalidHeaderError,
RunFunctionAndReturnError(new DownloadsDownloadFunction(),
base::StringPrintf(
"[{\"url\": \"%s\","
@@ -1685,8 +1690,6 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
}
}
-// Test that subdirectories (slashes) are disallowed in filenames.
-// TODO(benjhayden) Update this when subdirectories are supported.
IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
DownloadExtensionTest_Download_Subdirectory) {
LoadExtension("downloads_split");
@@ -1695,12 +1698,39 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
std::string download_url = test_server()->GetURL("slow?0").spec();
GoOnTheRecord();
- EXPECT_STREQ(download_extension_errors::kInvalidFilenameError,
- RunFunctionAndReturnError(new DownloadsDownloadFunction(),
- base::StringPrintf(
- "[{\"url\": \"%s\","
- " \"filename\": \"sub/dir/ect/ory.txt\"}]",
- download_url.c_str())).c_str());
+ scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
+ new DownloadsDownloadFunction(), base::StringPrintf(
+ "[{\"url\": \"%s\","
+ " \"filename\": \"sub/dir/ect/ory.txt\"}]",
+ download_url.c_str())));
+ ASSERT_TRUE(result.get());
+ int result_id = -1;
+ ASSERT_TRUE(result->GetAsInteger(&result_id));
+ DownloadItem* item = GetCurrentManager()->GetDownload(result_id);
+ ASSERT_TRUE(item);
+ ScopedCancellingItem canceller(item);
+ ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+
+ ASSERT_TRUE(WaitFor(events::kOnDownloadCreated,
+ base::StringPrintf("[{\"danger\": \"safe\","
+ " \"incognito\": false,"
+ " \"mime\": \"text/plain\","
+ " \"paused\": false,"
+ " \"url\": \"%s\"}]",
+ download_url.c_str())));
+ ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+ base::StringPrintf("[{\"id\": %d,"
+ " \"filename\": {"
+ " \"previous\": \"\","
+ " \"current\": \"%s\"}}]",
+ result_id,
+ GetFilename("sub/dir/ect/ory.txt").c_str())));
+ ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+ base::StringPrintf("[{\"id\": %d,"
+ " \"state\": {"
+ " \"previous\": \"in_progress\","
+ " \"current\": \"complete\"}}]",
+ result_id)));
}
// Test that invalid filenames are disallowed.
@@ -1730,20 +1760,32 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
"foo bar",
"../hello",
"/hello",
- "google.com/",
"http://",
"#frag",
"foo/bar.html#frag",
+ "google.com/",
+ };
+
+ for (size_t index = 0; index < arraysize(kInvalidURLs); ++index) {
+ EXPECT_STREQ(download_extension_errors::kInvalidURLError,
+ RunFunctionAndReturnError(new DownloadsDownloadFunction(),
+ base::StringPrintf(
+ "[{\"url\": \"%s\"}]", kInvalidURLs[index])).c_str())
+ << kInvalidURLs[index];
+ }
+
+ static const char* kNotPermittedURLs[] = {
"javascript:document.write(\\\"hello\\\");",
"javascript:return false;",
"ftp://example.com/example.txt",
};
- for (size_t index = 0; index < arraysize(kInvalidURLs); ++index) {
- EXPECT_STREQ(download_extension_errors::kInvalidURLError,
+ for (size_t index = 0; index < arraysize(kNotPermittedURLs); ++index) {
+ EXPECT_STREQ(download_extension_errors::kNotPermittedURLError,
RunFunctionAndReturnError(new DownloadsDownloadFunction(),
base::StringPrintf(
- "[{\"url\": \"%s\"}]", kInvalidURLs[index])).c_str());
+ "[{\"url\": \"%s\"}]", kNotPermittedURLs[index])).c_str())
+ << kNotPermittedURLs[index];
}
}
@@ -1910,13 +1952,15 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
ScopedCancellingItem canceller(item);
ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
- ASSERT_TRUE(WaitForInterruption(item, 30, base::StringPrintf(
- "[{\"danger\": \"safe\","
- " \"incognito\": false,"
- " \"mime\": \"text/html\","
- " \"paused\": false,"
- " \"url\": \"%s\"}]",
- download_url.c_str())));
+ ASSERT_TRUE(WaitForInterruption(
+ item,
+ content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED,
+ base::StringPrintf("[{\"danger\": \"safe\","
+ " \"incognito\": false,"
+ " \"mime\": \"text/html\","
+ " \"paused\": false,"
+ " \"url\": \"%s\"}]",
+ download_url.c_str())));
}
// Test that DownloadsDownloadFunction propagates |headers| to the URLRequest.
@@ -1993,14 +2037,16 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
ScopedCancellingItem canceller(item);
ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
- ASSERT_TRUE(WaitForInterruption(item, 33, base::StringPrintf(
- "[{\"danger\": \"safe\","
- " \"incognito\": false,"
- " \"bytesReceived\": 0,"
- " \"mime\": \"\","
- " \"paused\": false,"
- " \"url\": \"%s\"}]",
- download_url.c_str())));
+ ASSERT_TRUE(WaitForInterruption(
+ item,
+ content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+ base::StringPrintf("[{\"danger\": \"safe\","
+ " \"incognito\": false,"
+ " \"bytesReceived\": 0,"
+ " \"mime\": \"\","
+ " \"paused\": false,"
+ " \"url\": \"%s\"}]",
+ download_url.c_str())));
}
// Test that DownloadsDownloadFunction propagates the Authorization header
@@ -2119,15 +2165,17 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
ScopedCancellingItem canceller(item);
ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
- ASSERT_TRUE(WaitForInterruption(item, 33, base::StringPrintf(
- "[{\"danger\": \"safe\","
- " \"incognito\": false,"
- " \"mime\": \"\","
- " \"paused\": false,"
- " \"id\": %d,"
- " \"url\": \"%s\"}]",
- result_id,
- download_url.c_str())));
+ ASSERT_TRUE(WaitForInterruption(
+ item,
+ content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+ base::StringPrintf("[{\"danger\": \"safe\","
+ " \"incognito\": false,"
+ " \"mime\": \"\","
+ " \"paused\": false,"
+ " \"id\": %d,"
+ " \"url\": \"%s\"}]",
+ result_id,
+ download_url.c_str())));
}
// Test that downloadPostSuccess would fail if the resource requires the POST
@@ -2158,15 +2206,17 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
ScopedCancellingItem canceller(item);
ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
- ASSERT_TRUE(WaitForInterruption(item, 33, base::StringPrintf(
- "[{\"danger\": \"safe\","
- " \"incognito\": false,"
- " \"mime\": \"\","
- " \"paused\": false,"
- " \"id\": %d,"
- " \"url\": \"%s\"}]",
- result_id,
- download_url.c_str())));
+ ASSERT_TRUE(WaitForInterruption(
+ item,
+ content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+ base::StringPrintf("[{\"danger\": \"safe\","
+ " \"incognito\": false,"
+ " \"mime\": \"\","
+ " \"paused\": false,"
+ " \"id\": %d,"
+ " \"url\": \"%s\"}]",
+ result_id,
+ download_url.c_str())));
}
// Test that cancel()ing an in-progress download causes its state to transition
@@ -2205,7 +2255,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
item->Cancel(true);
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
- " \"error\": {\"current\": 40},"
+ " \"error\": {\"current\":\"USER_CANCELED\"},"
" \"state\": {"
" \"previous\": \"in_progress\","
" \"current\": \"interrupted\"}}]",
@@ -2365,11 +2415,13 @@ IN_PROC_BROWSER_TEST_F(
" \"url\": \"%s\"}]",
result_id,
download_url.c_str())));
+ LOG(INFO) << "occam";
ASSERT_TRUE(WaitFor(
events::kOnDownloadDeterminingFilename,
base::StringPrintf("[{\"id\": %d,"
" \"filename\":\"slow.txt\"}]",
result_id)));
+ LOG(INFO) << "occam";
ASSERT_TRUE(item->GetTargetFilePath().empty());
ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
@@ -2389,24 +2441,25 @@ IN_PROC_BROWSER_TEST_F(
base::StringPrintf("[{\"id\": %d,"
" \"danger\": {"
" \"previous\":\"safe\","
- " \"current\":\"file\"},"
- " \"dangerAccepted\": {"
- " \"current\":false}}]",
+ " \"current\":\"file\"}}]",
result_id)));
+ LOG(INFO) << "occam";
item->ValidateDangerousDownload();
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
- " \"dangerAccepted\": {"
- " \"previous\":false,"
- " \"current\":true}}]",
+ " \"danger\": {"
+ " \"previous\":\"file\","
+ " \"current\":\"accepted\"}}]",
result_id)));
+ LOG(INFO) << "occam";
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
" \"state\": {"
" \"previous\": \"in_progress\","
" \"current\": \"complete\"}}]",
result_id)));
+ LOG(INFO) << "occam";
EXPECT_EQ(downloads_directory().AppendASCII("overridden.swf"),
item->GetTargetFilePath());
}
@@ -3477,7 +3530,7 @@ IN_PROC_BROWSER_TEST_F(
ASSERT_TRUE(interrupted.WaitForEvent());
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
- " \"error\":{\"current\":20},"
+ " \"error\":{\"current\":\"NETWORK_FAILED\"},"
" \"state\":{"
" \"previous\":\"in_progress\","
" \"current\":\"interrupted\"}}]",
@@ -3496,7 +3549,7 @@ IN_PROC_BROWSER_TEST_F(
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
- " \"error\":{\"previous\":20},"
+ " \"error\":{\"previous\":\"NETWORK_FAILED\"},"
" \"state\":{"
" \"previous\":\"interrupted\","
" \"current\":\"in_progress\"}}]",
« chrome/browser/extensions/api/downloads/downloads_api.cc ('K') | « chrome/browser/extensions/api/downloads/downloads_api.cc ('k') | chrome/browser/extensions/extension_function_histogram_value.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698