Test file errors in downloads.

chrome/browser/download/download_browsertest.cc

Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index d3e0f19b564e8f9199c44fae8e5114aab17c18bb..a8c9e60a94f87912d5b6612c83d8aa4f24ca3029 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -55,6 +55,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/context_menu_params.h"
 #include "content/public/common/page_transition_types.h"
+#include "content/test/test_file_error_injector.h"
 #include "content/test/test_navigation_observer.h"
 #include "net/base/net_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -306,6 +307,11 @@ class DownloadTest : public InProcessBrowserTest {
     bool show_download_item;  // True if the download item appears on the shelf.
   };
 
+  struct FileErrorInjectInfo {
+    DownloadInfo download_info;
+    content::TestFileErrorInjector::FileErrorInfo error_info;
+  };
+
   DownloadTest() {
     EnableDOMAutomation();
   }
@@ -736,6 +742,26 @@ class DownloadTest : public InProcessBrowserTest {
     }
   }
 
+  void DownloadInsertFileErrorCheckErrors(const FileErrorInjectInfo& info) {
+    // Do initial setup.
+    ASSERT_TRUE(InitialSetup(false));
+
+    // Set up file failures.
+    // |injector| will be owned by the DownloadFileManager (indirectly).

[cbentzel, 2012/02/28 14:51:41]

This comment about ownership is no longer true - it's shared ownership.

[ahendrickson, 2012/03/01 09:17:32]

Done.

+    scoped_refptr<content::TestFileErrorInjector> injector(
+        content::TestFileErrorInjector::Create());
+    injector->AddError(info.error_info);
+    injector->InjectErrors();
+
+    DownloadFileCheckErrors(info.download_info);
+
+    ui_test_utils::RunAllPendingInMessageLoop();

[cbentzel, 2012/02/28 14:51:41]

Are you guaranteed that all of the work on the file thread has been completed,
and that all of the completed messages that get posted back to the UI thread are
complete?

I didn't do a full analysis, but I am worried about potential for flake here if
the ordering isn't completely right. Also worried about reported leaks of the
TestFileErrorInjector here.

[ahendrickson, 2012/03/01 09:17:32]

Added code to run all pending messages on the FILE thread.  Our notifications
happen on the UI thread.

+
+    EXPECT_EQ(1u, injector->FileCreationCount());
+    EXPECT_EQ(false, injector->HasFile(0));

[cbentzel, 2012/02/28 14:51:41]

Do you need this HasFile call? Doesn't the CurrentFileCount indicate it?

[ahendrickson, 2012/03/01 09:17:32]

Hmm, I guess it's not needed.

Removed.

+    EXPECT_EQ(0u, injector->CurrentFileCount());
+  }
+
  private:
   // Location of the test data.
   FilePath test_dir_;
@@ -2081,3 +2107,221 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError400Navigate) {
   ASSERT_TRUE(InitialSetup(false));
   DownloadFileCheckErrors(download_info);
 }
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNoSpaceOpenNavigate) {

[Randy Smith (Not in Mondays), 2012/02/28 22:06:13]

There's a fair amount of overhead in starting up and shutting down a browser
test instance.  Any chance that these tests could be combined into one test that
cycled through all the different errors?  This would require allowing
InjectErrors() to be called more than once (and defining clean semantics for
that).

[ahendrickson, 2012/03/01 09:17:32]

Done.

+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FILE_NO_SPACE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNoSpaceOpenDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FILE_NO_SPACE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNoSpaceWriteNavigate) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_FILE_NO_SPACE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNoSpaceWriteDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_FILE_NO_SPACE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileFailedOpenNavigate) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FAILED
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileFailedOpenDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FAILED
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileFailedWriteNavigate) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_FAILED
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileFailedWriteDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_FAILED
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNameTooLongOpenNavigate) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NAME_TOO_LONG,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FILE_PATH_TOO_LONG
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorFileNameTooLongOpenDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_NAME_TOO_LONG,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
+      0,
+      net::ERR_FILE_PATH_TOO_LONG
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest,
+                       DownloadErrorFileInvalidHandleWriteNavigate) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_NAVIGATE,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_INVALID_HANDLE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadTest,
+                       DownloadErrorFileInvalidHandleWriteDirect) {
+  FileErrorInjectInfo error_info = {
+    { "a_zip_file.zip",
+      DOWNLOAD_DIRECT,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+      1
+    },
+    {
+      0,
+      content::TestFileErrorInjector::FILE_OPERATION_WRITE,
+      0,
+      net::ERR_INVALID_HANDLE
+    }
+  };
+
+  DownloadInsertFileErrorCheckErrors(error_info);
+}