Fix DownloadExtensionTest_OnDeterminingFilename_InterruptedResume

chrome/browser/extensions/api/downloads/downloads_api_unittest.cc

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 3ec086412b6585302b21eebd3150869a20e3badf..02459f534d030cfb745e50bf6d9c7eae674c8902 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
@@ -36,9 +36,9 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/storage_partition.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/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 "net/base/data_url.h"
 #include "net/base/net_util.h"
@@ -86,6 +86,11 @@ class DownloadsEventsListener : public content::NotificationObserver {
     STLDeleteElements(&events_);
   }
 
+  void ClearEvents() {
+    STLDeleteElements(&events_);
+    events_.clear();
+  }
+
   class Event {
    public:
     Event(Profile* profile,
@@ -335,6 +340,10 @@ class DownloadExtensionTest : public ExtensionApiTest {
                             expected_error));
   }
 
+  void ClearEvents() {
+    events_listener_->ClearEvents();
+  }
+
   std::string GetExtensionURL() {
     return extension_->url().spec();
   }
@@ -847,6 +856,29 @@ const char HTML5FileWriter::kHTML5FileWritten[] = "html5_file_written";
 const char HTML5FileWriter::kURLRequestContextToreDown[] =
   "url_request_context_tore_down";
 
+// TODO(benjhayden) Merge this with the other TestObservers.
+class JustInProgressDownloadObserver
+    : public content::DownloadTestObserverInProgress {
+ 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);
+};
+
+bool ItemIsInterrupted(DownloadItem* item) {
+  return item->IsInterrupted();
+}
+
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
@@ -3096,108 +3128,128 @@ IN_PROC_BROWSER_TEST_F(
                          result_id)));
 }
 
-// Test download interruption while extensions determining filename, re-run
-// through fan-out and fan-in.
-// TODO(rdsmith): FILE_OPERATION_INITIALIZE is not right for this test.
+// Test download interruption while extensions determining filename. Should not
+// re-dispatch onDeterminingFilename.
 IN_PROC_BROWSER_TEST_F(
     DownloadExtensionTest,
-    DISABLED_DownloadExtensionTest_OnDeterminingFilename_InterruptedResume) {
+    DownloadExtensionTest_OnDeterminingFilename_InterruptedResume) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableDownloadResumption);
   LoadExtension("downloads_split");
   CHECK(StartTestServer());
-  std::string download_url = test_server()->GetURL("slow?0").spec();
   GoOnTheRecord();
   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.
-  scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
-      new DownloadsDownloadFunction(), base::StringPrintf(
-          "[{\"url\": \"%s\"}]", 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);
+  DownloadItem* item = NULL;
+  {
+    DownloadManager* manager = GetCurrentManager();
+    scoped_ptr<content::DownloadTestObserver> observer(
+        new JustInProgressDownloadObserver(manager, 1));
+    ASSERT_EQ(0, manager->InProgressCount());
+    // Tabs created just for a download are automatically closed, invalidating
+    // the download's WebContents. Downloads without WebContents cannot be
+    // 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);
-  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
 
   // Wait for the onCreated and onDeterminingFilename event.
   ASSERT_TRUE(WaitFor(events::kOnDownloadCreated,
       base::StringPrintf("[{\"danger\": \"safe\","
-                          "  \"incognito\": false,"
-                          "  \"id\": %d,"
-                          "  \"mime\": \"text/plain\","
-                          "  \"paused\": false,"
-                          "  \"url\": \"%s\"}]",
-                          result_id,
-                          download_url.c_str())));
+                         "  \"incognito\": false,"
+                         "  \"id\": %d,"
+                         "  \"mime\": \"application/octet-stream\","
+                         "  \"paused\": false}]",
+                         item->GetId())));
   ASSERT_TRUE(WaitFor(
       events::kOnDownloadDeterminingFilename,
       base::StringPrintf("[{\"id\": %d,"
                          "  \"incognito\": false,"
-                         "  \"filename\":\"slow.txt\"}]",
-                         result_id)));
+                         "  \"filename\":\"download-unknown-size\"}]",
+                         item->GetId())));
   ASSERT_TRUE(item->GetTargetFilePath().empty());
   ASSERT_TRUE(item->IsInProgress());
 
-  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
-      base::StringPrintf("[{\"id\": %d,"
-                          "  \"state\": {"
-                          "    \"previous\": \"in_progress\","
-                          "    \"current\": \"interrupted\"}}]",
-                          result_id)));
-  ASSERT_TRUE(item->IsInterrupted());
-  item->ResumeInterruptedDownload();
+  ClearEvents();
+  ui_test_utils::NavigateToURLWithDisposition(
+      current_browser(),
+      GURL(URLRequestSlowDownloadJob::kErrorDownloadUrl),
+      NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
 
-  // Wait for and respond to the onDeterminingFilename event.
-  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());
+  // Errors caught before filename determination are delayed until after
+  // filename determination.
   std::string error;
   ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
       current_browser()->profile(),
       false,
       GetExtensionId(),
-      result_id,
+      item->GetId(),
       base::FilePath(FILE_PATH_LITERAL("42.txt")),
       false,
-      &error));
+      &error)) << error;
   EXPECT_EQ("", error);
 
-  // The download should complete successfully.
+  content::DownloadUpdatedObserver interrupted(item, base::Bind(
+        ItemIsInterrupted));
+  ASSERT_TRUE(interrupted.WaitForEvent());
   ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
       base::StringPrintf("[{\"id\": %d,"
-                          "  \"filename\": {"
-                          "    \"previous\": \"%s\","
-                          "    \"current\": \"%s\"},"
-                          "  \"state\": {"
-                          "    \"previous\": \"in_progress\","
-                          "    \"current\": \"complete\"}}]",
-                          result_id,
-                          GetFilename("42.txt.crdownload").c_str(),
-                          GetFilename("42.txt").c_str())));
+                         "  \"error\":{\"current\":20},"
+                         "  \"state\":{"
+                         "    \"previous\":\"in_progress\","
+                         "    \"current\":\"interrupted\"}}]",
+                         item->GetId())));
+
+  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,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\": {"
+                         "    \"previous\": \"%s\","
+                         "    \"current\": \"%s\"},"
                          "  \"state\": {"
                          "    \"previous\": \"in_progress\","
                          "    \"current\": \"complete\"}}]",
-                         result_id)));
+                         item->GetId(),
+                         GetFilename("42.txt.crdownload").c_str(),
+                         GetFilename("42.txt").c_str())));
 }
 
 // TODO(benjhayden) Figure out why DisableExtension() does not fire