Skip to content

GitLab

T
tangled
Commit 41b33504 authored by xhwang@chromium.org's avatar xhwang@chromium.org
Browse files
Options

Revert 192563 "Fix DownloadExtensionTest_OnDeterminingFilename_I..." 

This test failed on Vista Tests bot:

http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20(2)/builds/33887/steps/browser_tests/logs/DownloadExtensionTest_OnDeterminingFilename_InterruptedResume

DownloadExtensionTest.DownloadExtensionTest_OnDeterminingFilename_InterruptedResume: 
[0405/091818:ERROR:gl_surface_egl.cc(98)] eglInitialize failed with error
UNKNOWN
[0405/091818:ERROR:gl_surface_win.cc(54)] GLSurfaceEGL::InitializeOneOff failed.
HTTP server started on 127.0.0.1:51868...
sending server_data: {"host": "127.0.0.1", "port": 51868} (36 bytes)
browser\extensions\api\downloads\downloads_api_unittest.cc(3222): error: Value
of: item->GetState()
Actual: 0
Expected: DownloadItem::INTERRUPTED
Which is: 3
browser\extensions\api\downloads\downloads_api_unittest.cc(3223): error: Value
of: item->IsInterrupted()
Actual: false
Expected: true

> Fix DownloadExtensionTest_OnDeterminingFilename_InterruptedResume
> 
> Review URL: https://codereview.chromium.org/12422012

TBR=benjhayden@chromium.org
Review URL: https://codereview.chromium.org/13598016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192579 0039d316-1c4b-4281-b951-d872f2087c98
parent ced9eeff
Hide whitespace changes
Inline Side-by-side
Showing with 66 additions and 121 deletions
chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
View file @ 41b33504
...@@ -36,9 +36,9 @@ ...@@ -36,9 +36,9 @@
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/page_transition_types.h" #include "content/public/common/page_transition_types.h"
#include "content/public/test/download_test_observer.h" #include "content/public/test/download_test_observer.h"
#include "content/public/test/test_file_error_injector.h"
#include "content/test/net/url_request_slow_download_job.h" #include "content/test/net/url_request_slow_download_job.h"
#include "net/base/data_url.h" #include "net/base/data_url.h"
#include "net/base/net_util.h" #include "net/base/net_util.h"
...@@ -86,11 +86,6 @@ class DownloadsEventsListener : public content::NotificationObserver { ...@@ -86,11 +86,6 @@ class DownloadsEventsListener : public content::NotificationObserver {
STLDeleteElements(&events_); STLDeleteElements(&events_);
} }
void ClearEvents() {
STLDeleteElements(&events_);
events_.clear();
}
class Event { class Event {
public: public:
Event(Profile* profile, Event(Profile* profile,
...@@ -340,10 +335,6 @@ class DownloadExtensionTest : public ExtensionApiTest { ...@@ -340,10 +335,6 @@ class DownloadExtensionTest : public ExtensionApiTest {
expected_error)); expected_error));
} }
void ClearEvents() {
events_listener_->ClearEvents();
}
std::string GetExtensionURL() { std::string GetExtensionURL() {
return extension_->url().spec(); return extension_->url().spec();
} }
...@@ -3105,154 +3096,108 @@ IN_PROC_BROWSER_TEST_F( ...@@ -3105,154 +3096,108 @@ IN_PROC_BROWSER_TEST_F(
result_id))); result_id)));
} }
// TODO(benjhayden) Merge this with the other TestObservers. // Test download interruption while extensions determining filename, re-run
class JustInProgressDownloadObserver // through fan-out and fan-in.
: public content::DownloadTestObserverInProgress { // TODO(rdsmith): FILE_OPERATION_INITIALIZE is not right for this test.
public:
JustInProgressDownloadObserver(
DownloadManager* download_manager, size_t wait_count)
: content::DownloadTestObserverInProgress(download_manager, wait_count) {
}
virtual ~JustInProgressDownloadObserver() {}
private:
virtual bool IsDownloadInFinalState(DownloadItem* item) OVERRIDE {
return item->GetState() == DownloadItem::IN_PROGRESS;
}
DISALLOW_COPY_AND_ASSIGN(JustInProgressDownloadObserver);
};
// Test download interruption while extensions determining filename. Should not
// re-dispatch onDeterminingFilename.
#if defined(OS_CHROMEOS)
#define MAYBE_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume \
DISABLED_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume
#else
#define MAYBE_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume \
DownloadExtensionTest_OnDeterminingFilename_InterruptedResume
#endif
IN_PROC_BROWSER_TEST_F( IN_PROC_BROWSER_TEST_F(
DownloadExtensionTest, DownloadExtensionTest,
MAYBE_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume) { DISABLED_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume) {
CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableDownloadResumption);
LoadExtension("downloads_split"); LoadExtension("downloads_split");
CHECK(StartTestServer()); CHECK(StartTestServer());
std::string download_url = test_server()->GetURL("slow?0").spec();
GoOnTheRecord(); GoOnTheRecord();
AddFilenameDeterminer(); AddFilenameDeterminer();
// TODO Interrupt the download instead of responding to onDeterminingFilename.
scoped_refptr<content::TestFileErrorInjector> injector(
content::TestFileErrorInjector::Create(
GetCurrentManager()));
content::TestFileErrorInjector::FileErrorInfo error_info = {
download_url,
content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
0,
content::DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE};
injector->AddError(error_info);
injector->InjectErrors();
// Start a download. // Start a download.
DownloadItem* item = NULL; scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
{ new DownloadsDownloadFunction(), base::StringPrintf(
DownloadManager* manager = GetCurrentManager(); "[{\"url\": \"%s\"}]", download_url.c_str())));
scoped_ptr<content::DownloadTestObserver> observer( ASSERT_TRUE(result.get());
new JustInProgressDownloadObserver(manager, 1)); int result_id = -1;
ASSERT_EQ(0, manager->InProgressCount()); ASSERT_TRUE(result->GetAsInteger(&result_id));
// Tabs created just for a download are automatically closed, invalidating DownloadItem* item = GetCurrentManager()->GetDownload(result_id);
// the download's WebContents. Downloads without WebContents cannot be ASSERT_TRUE(item);
// resumed. http://crbug.com/225901
ui_test_utils::NavigateToURLWithDisposition(
current_browser(),
GURL(URLRequestSlowDownloadJob::kUnknownSizeUrl),
CURRENT_TAB,
ui_test_utils::BROWSER_TEST_NONE);
observer->WaitForFinished();
EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::IN_PROGRESS));
DownloadManager::DownloadVector items;
manager->GetAllDownloads(&items);
for (DownloadManager::DownloadVector::iterator iter = items.begin();
iter != items.end(); ++iter) {
if ((*iter)->GetState() == DownloadItem::IN_PROGRESS) {
// There should be only one IN_PROGRESS item.
EXPECT_EQ(NULL, item);
item = *iter;
}
}
ASSERT_TRUE(item);
}
ScopedCancellingItem canceller(item); ScopedCancellingItem canceller(item);
ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
// Wait for the onCreated and onDeterminingFilename event. // Wait for the onCreated and onDeterminingFilename event.
ASSERT_TRUE(WaitFor(events::kOnDownloadCreated, ASSERT_TRUE(WaitFor(events::kOnDownloadCreated,
base::StringPrintf("[{\"danger\": \"safe\"," base::StringPrintf("[{\"danger\": \"safe\","
" \"incognito\": false," " \"incognito\": false,"
" \"id\": %d," " \"id\": %d,"
" \"mime\": \"application/octet-stream\"," " \"mime\": \"text/plain\","
" \"paused\": false}]", " \"paused\": false,"
item->GetId()))); " \"url\": \"%s\"}]",
result_id,
download_url.c_str())));
ASSERT_TRUE(WaitFor( ASSERT_TRUE(WaitFor(
events::kOnDownloadDeterminingFilename, events::kOnDownloadDeterminingFilename,
base::StringPrintf("[{\"id\": %d," base::StringPrintf("[{\"id\": %d,"
" \"incognito\": false," " \"incognito\": false,"
" \"filename\":\"download-unknown-size\"}]", " \"filename\":\"slow.txt\"}]",
item->GetId()))); result_id)));
ASSERT_TRUE(item->GetTargetFilePath().empty()); ASSERT_TRUE(item->GetTargetFilePath().empty());
ASSERT_TRUE(item->IsInProgress()); ASSERT_TRUE(item->IsInProgress());
ClearEvents(); ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
ui_test_utils::NavigateToURLWithDisposition( base::StringPrintf("[{\"id\": %d,"
current_browser(), " \"state\": {"
GURL(URLRequestSlowDownloadJob::kErrorDownloadUrl), " \"previous\": \"in_progress\","
NEW_BACKGROUND_TAB, " \"current\": \"interrupted\"}}]",
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); result_id)));
ASSERT_TRUE(item->IsInterrupted());
item->ResumeInterruptedDownload();
// Errors caught before filename determination are delayed until after // Wait for and respond to the onDeterminingFilename event.
// filename determination. ASSERT_TRUE(WaitFor(
events::kOnDownloadDeterminingFilename,
base::StringPrintf("[{\"id\": %d,"
" \"incognito\": false,"
" \"filename\":\"slow.txt\"}]",
result_id)));
ASSERT_TRUE(item->GetTargetFilePath().empty());
ASSERT_TRUE(item->IsInProgress());
std::string error; std::string error;
ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename( ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
current_browser()->profile(), current_browser()->profile(),
false, false,
GetExtensionId(), GetExtensionId(),
item->GetId(), result_id,
base::FilePath(FILE_PATH_LITERAL("42.txt")), base::FilePath(FILE_PATH_LITERAL("42.txt")),
false, false,
&error)) << error; &error));
EXPECT_EQ("", error); EXPECT_EQ("", error);
// The download should complete successfully.
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d," base::StringPrintf("[{\"id\": %d,"
" \"error\":{\"current\":20}," " \"filename\": {"
" \"state\":{" " \"previous\": \"%s\","
" \"previous\":\"in_progress\"," " \"current\": \"%s\"},"
" \"current\":\"interrupted\"}}]", " \"state\": {"
item->GetId()))); " \"previous\": \"in_progress\","
// TODO(benjhayden) These next two lines fail on chromeos. " \"current\": \"complete\"}}]",
EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState()); result_id,
ASSERT_TRUE(item->IsInterrupted()); GetFilename("42.txt.crdownload").c_str(),
GetFilename("42.txt").c_str())));
ClearEvents();
item->ResumeInterruptedDownload();
// Errors caught before filename determination is complete are delayed until
// after filename determination so that, on resumption, filename determination
// does not need to be re-done. So, there will not be a second
// onDeterminingFilename event.
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
" \"error\":{\"previous\":20},"
" \"state\":{"
" \"previous\":\"interrupted\","
" \"current\":\"in_progress\"}}]",
item->GetId())));
ClearEvents();
FinishPendingSlowDownloads();
// The download should complete successfully.
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged, ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d," base::StringPrintf("[{\"id\": %d,"
" \"filename\": {"
" \"previous\": \"%s\","
" \"current\": \"%s\"},"
" \"state\": {" " \"state\": {"
" \"previous\": \"in_progress\"," " \"previous\": \"in_progress\","
" \"current\": \"complete\"}}]", " \"current\": \"complete\"}}]",
item->GetId(), result_id)));
GetFilename("42.txt.crdownload").c_str(),
GetFilename("42.txt").c_str())));
} }
// TODO(benjhayden) Figure out why DisableExtension() does not fire // TODO(benjhayden) Figure out why DisableExtension() does not fire
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment