Implement chrome.downloads.onDeterminingFilename()

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 451bb6a58f8dd3636b642e461207934e28a08ed1..ba08621144f208de426c1fc3231e265e54d6b7e2 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/history/download_row.h"
 #include "chrome/browser/net/url_request_mock_util.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -103,8 +104,11 @@ class DownloadsEventsListener : public content::NotificationObserver {
       if ((profile_ != other.profile_) ||
           (event_name_ != other.event_name_))
         return false;
-      if ((event_name_ == events::kOnDownloadCreated ||
-           event_name_ == events::kOnDownloadChanged) &&
+      if (((event_name_ == events::kOnDownloadCreated) ||
+           (event_name_.substr(
+               0, strlen(events::kOnDownloadDeterminingFilename)) ==
+            events::kOnDownloadDeterminingFilename) ||
+           (event_name_ == events::kOnDownloadChanged)) &&
           args_.get() &&
           other.args_.get()) {
         base::ListValue* left_list = NULL;
@@ -169,6 +173,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();
           events_.push_back(new_event);
           if (waiting_ &&
               waiting_for_.get() &&
@@ -312,6 +317,17 @@ class DownloadExtensionTest : public ExtensionApiTest {
   std::string GetExtensionURL() {
     return extension_->url().spec();
   }
+  std::string GetExtensionId() {
+    return extension_->id();
+  }
+  void DisableExtension() {
+    browser()->profile()->GetExtensionService()->DisableExtension(
+        GetExtensionId(), extensions::Extension::DISABLE_USER_ACTION);
+  }
+  void EnableExtension() {
+    browser()->profile()->GetExtensionService()->EnableExtension(
+        GetExtensionId());
+  }
 
   std::string GetFilename(const char* path) {
     std::string result =
@@ -1709,7 +1725,6 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_DataURL) {
   LoadExtension("downloads_split");
-  CHECK(StartTestServer());
   std::string download_url = "data:text/plain,hello";
   GoOnTheRecord();
 
@@ -1749,7 +1764,6 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_File) {
   GoOnTheRecord();
-  CHECK(StartTestServer());
   LoadExtension("downloads_split");
   std::string download_url = "file:///";
 #if defined(OS_WIN)
@@ -2156,3 +2170,760 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
   EXPECT_TRUE(file_util::ReadFileToString(item->GetFullPath(), &disk_data));
   EXPECT_STREQ(kPayloadData, disk_data.c_str());
 }
+
+IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,

[Randy Smith (Not in Mondays), 2013/01/22 19:43:08]

Could you put in some line breaks in the following tests so that it's easy for
the reader to group related lines into functional chunks?  Occasional comments
indicating the purposes of the chunks would be good too.

[benjhayden, 2013/01/23 21:30:10]

Done.

+                       DownloadExtensionTest_OnDeterminingFilename_NoChange) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_DangerousOverride) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("overridden.swf")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"danger\": {"
+                         "    \"previous\":\"safe\","
+                         "    \"current\":\"file\"},"
+                         "  \"dangerAccepted\": {"
+                         "    \"current\":false}}]",
+                         result_id)));
+  item->DangerousDownloadValidated();
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"dangerAccepted\": {"
+                         "    \"previous\":false,"
+                         "    \"current\":true}}]",
+                         result_id)));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("overridden.swf.crdownload").c_str(),
+                          GetFilename("overridden.swf").c_str())));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_ReferencesParentInvalid) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("../sneaky.txt")),

[Randy Smith (Not in Mondays), 2013/01/22 19:43:08]

Just because I'm a paranoid control free, could you make this test not start
with ".."?  (I.e. "subdir/../../sneaky.txt"?)

[benjhayden, 2013/01/23 21:30:10]

Done.

+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_CurDirInvalid) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL(".")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_ParentDirInvalid) {
+  CHECK(StartTestServer());
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("..")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_AbsPathInvalid) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  FilePath override;
+#if defined(OS_WIN)
+  override = FilePath(FILE_PATH_LITERAL("C:\\windows\\system32\\mwahaha.dll"));

[Randy Smith (Not in Mondays), 2013/01/22 19:43:08]

Suggestion: Make these locations you could write to (publicly writable, no file
already there?)

[benjhayden, 2013/01/23 21:30:10]

Done.

+#else
+  override = FilePath(FILE_PATH_LITERAL("/bin/cat"));
+#endif
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      override,
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_Override) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+
+  result.reset(RunFunctionAndReturnResult(
+      new DownloadsDownloadFunction(), base::StringPrintf(
+          "[{\"url\": \"%s\"}]", download_url.c_str())));
+  ASSERT_TRUE(result.get());
+  result_id = -1;
+  ASSERT_TRUE(result->GetAsInteger(&result_id));
+  item = GetCurrentManager()->GetDownload(result_id);
+  ASSERT_TRUE(item);
+  ScopedCancellingItem canceller2(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("slow.txt")),
+      true));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("slow.txt.crdownload").c_str(),
+                          GetFilename("slow.txt").c_str())));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_LaterDeterminerPrecedence) {
+  CHECK(StartTestServer());
+  LOG(INFO) << "occam";
+  GoOnTheRecord();
+  LOG(INFO) << "occam";
+  LoadExtension("downloads_split");
+  LOG(INFO) << "occam";
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  LOG(INFO) << "occam";
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "43"));
+  LOG(INFO) << "occam";
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  LOG(INFO) << "occam";
+  scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
+      new DownloadsDownloadFunction(), base::StringPrintf(
+          "[{\"url\": \"%s\"}]", download_url.c_str())));
+  LOG(INFO) << "occam";
+  ASSERT_TRUE(result.get());
+  LOG(INFO) << "occam";
+  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,"
+                          "  \"id\": %d,"
+                          "  \"mime\": \"text/plain\","
+                          "  \"paused\": false,"
+                          "  \"url\": \"%s\"}]",
+                          result_id,
+                          download_url.c_str())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/43",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("42.txt")),
+      false));
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "43",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("43.txt")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("43.txt.crdownload").c_str(),
+                          GetFilename("43.txt").c_str())));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+
+  result.reset(RunFunctionAndReturnResult(
+      new DownloadsDownloadFunction(), base::StringPrintf(
+          "[{\"url\": \"%s\"}]", download_url.c_str())));
+  ASSERT_TRUE(result.get());
+  result_id = -1;
+  ASSERT_TRUE(result->GetAsInteger(&result_id));
+  item = GetCurrentManager()->GetDownload(result_id);
+  ASSERT_TRUE(item);
+  ScopedCancellingItem canceller2(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/43",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "43",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("43b.txt")),
+      false));
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("42b.txt")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                          "  \"filename\": {"
+                          "    \"previous\": \"%s\","
+                          "    \"current\": \"%s\"},"
+                          "  \"state\": {"
+                          "    \"previous\": \"in_progress\","
+                          "    \"current\": \"complete\"}}]",
+                          result_id,
+                          GetFilename("43b.txt.crdownload").c_str(),
+                          GetFilename("43b.txt").c_str())));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}
+
+// TODO test precedence rules: install_time
+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_RemoveFilenameDeterminer) {
+  CHECK(StartTestServer());
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "43"));
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::RemoveFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/43",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "43",
+      result_id,
+      FilePath(),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}
+
+// TODO test incognito
+// TODO test incognito=spanning

[Randy Smith (Not in Mondays), 2013/01/22 19:43:08]

Would it be possible to add a test that runs through the logic twice for a
single download item?  I ask because that logic is a little tricky, and is going
to be probed in the field very rarely, so if there is a bug there it'd be easy
for it to hide.

[benjhayden, 2013/01/23 21:30:10]

Done.

[Randy Smith (Not in Mondays), 2013/01/24 19:12:46]

I'm not sure you added the test I was looking for, which is ok; I'm not sure it
*can* be added in the current situation :-}.  I wanted to test out the
reentrancy for a single EDERD, which requires running through file name
determination twice for the same download, which requires interaction with
resumption, which may not be ready for such interaction.  

Having said that, I'm not sure that the "two downloads in a row" cases that you
did add do much--if you think they're useful, feel free to keep them, but if you
only put them in for me, feel free to remove them.

+
+IN_PROC_BROWSER_TEST_F(
+    DownloadExtensionTest,
+    DownloadExtensionTest_OnDeterminingFilename_DisableExtension) {
+  GoOnTheRecord();
+  LoadExtension("downloads_split");
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+  CHECK(StartTestServer());
+  std::string download_url = test_server()->GetURL("slow?0").spec();
+  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);
+  ScopedCancellingItem canceller(item);
+  ASSERT_EQ(download_url, item->GetOriginalUrl().spec());
+  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())));
+  ASSERT_TRUE(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/42",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+  DisableExtension();
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}