Implement chrome.downloads.onDeterminingFilename()

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/download/download_file_icon_extractor.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_test_file_chooser_observer.h"
 #include "chrome/browser/extensions/api/downloads/downloads_api.h"
 #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"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ skipping 65 matching lines @@
         args_(base::JSONReader::Read(json_args)),
         caught_(caught) {
     }
 
     const base::Time& caught() { return caught_; }
 
     bool Equals(const Event& other) {
       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)) ==

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

Huh?  (If you consider my confusion understandable, add a comment, otherwise,
just tell me what's going on.)

[benjhayden, 2013/01/25 16:21:36]

Sorry, this is 'starts with', which I now see is already a utility function.

[Randy Smith (Not in Mondays), 2013/01/25 18:22:45]

Confusion understanding fail :-}.  Why is the filename determination event
checked for by prefix, whereas the other events are straight string equals?

[benjhayden, 2013/02/04 19:34:52]

The actual name of the event includes the sub_event_id:
'downloads.onDeterminingFilename/0', 'downloads.onDeterminingFilename/1', etc.
This is a quirk of how the event works. The webRequest API does the same thing.
It's needed to allow downloads_api.cc to keep track of which handlers have
responded.
Note how downloads_custom_bindings.js makes chrome.Event objects named
'downloads.onDeterminingFilename/0' etc.

+            events::kOnDownloadDeterminingFilename) ||
+           (event_name_ == events::kOnDownloadChanged)) &&
           args_.get() &&
           other.args_.get()) {
         base::ListValue* left_list = NULL;
         base::DictionaryValue* left_dict = NULL;
         base::ListValue* right_list = NULL;
         base::DictionaryValue* right_dict = NULL;
         if (!args_->GetAsList(&left_list) ||
             !other.args_->GetAsList(&right_list) ||
             !left_list->GetDictionary(0, &left_dict) ||
             !right_list->GetDictionary(0, &right_dict))
@@ skipping 187 matching lines @@
                             "  \"state\": {"
                             "    \"previous\": \"in_progress\","
                             "    \"current\": \"interrupted\"}}]",
                             item->GetId(),
                             expected_error));
   }
 
   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 =
       downloads_directory_.path().AppendASCII(path).AsUTF8Unsafe();
 #if defined(OS_WIN)
     for (std::string::size_type next = result.find("\\");
          next != std::string::npos;
          next = result.find("\\", next)) {
       result.replace(next, 1, "\\\\");
       next += 2;
@@ skipping 1125 matching lines @@
 
 // Test that we can start a download and that the correct sequence of events is
 // fired for it.
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_Basic) {
   LoadExtension("downloads_split");
   CHECK(StartTestServer());
   std::string download_url = test_server()->GetURL("slow?0").spec();
   GoOnTheRecord();
 
+  // Start downloading a file.
   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());
@@ skipping 20 matching lines @@
 
 // Test that we can start a download from an incognito context, and that the
 // download knows that it's incognito.
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_Incognito) {
   LoadExtension("downloads_split");
   CHECK(StartTestServer());
   GoOffTheRecord();
   std::string download_url = test_server()->GetURL("slow?0").spec();
 
+  // Start downloading a file.
   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());
@@ skipping 168 matching lines @@
                           "    \"current\": \"complete\"}}]",
                           result_id,
                           GetFilename("slow.txt.crdownload").c_str(),
                           GetFilename("slow.txt").c_str())));
 }
 
 // Valid data URLs are valid URLs.
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_DataURL) {
   LoadExtension("downloads_split");
-  CHECK(StartTestServer());

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

What controls which tests this should be in and which not?  It seems a bit
arbitrary where it is in the file.

[benjhayden, 2013/01/25 16:21:36]

A test needs to start the test server if it plans on hitting it.
I had copy-pasta'd the line, but this test doesn't reference test_server(), so
it didn't need it.

   std::string download_url = "data:text/plain,hello";
   GoOnTheRecord();
 
   scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
         new DownloadsDownloadFunction(), base::StringPrintf(
       "[{\"url\": \"%s\","
       "  \"filename\": \"data.txt\"}]", download_url.c_str())));
   ASSERT_TRUE(result.get());
   int result_id = -1;
   ASSERT_TRUE(result->GetAsInteger(&result_id));
@@ skipping 19 matching lines @@
                           "    \"current\": \"complete\"}}]",
                           result_id,
                           GetFilename("data.txt.crdownload").c_str(),
                           GetFilename("data.txt").c_str())));
 }
 
 // Valid file URLs are valid URLs.
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
                        DownloadExtensionTest_Download_File) {
   GoOnTheRecord();
-  CHECK(StartTestServer());
   LoadExtension("downloads_split");
   std::string download_url = "file:///";
 #if defined(OS_WIN)
   download_url += "C:/";
 #endif
 
   scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
         new DownloadsDownloadFunction(), base::StringPrintf(
       "[{\"url\": \"%s\","
       "  \"filename\": \"file.txt\"}]", download_url.c_str())));
@@ skipping 386 matching lines @@
                           "  \"state\": {"
                           "    \"previous\": \"in_progress\","
                           "    \"current\": \"complete\"}}]",
                           result_id,
                           GetFilename("on_record.txt.crdownload").c_str(),
                           GetFilename("on_record.txt").c_str())));
   std::string disk_data;
   EXPECT_TRUE(file_util::ReadFileToString(item->GetFullPath(), &disk_data));
   EXPECT_STREQ(kPayloadData, disk_data.c_str());
 }
+
+IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
+                       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();
+
+  // Start downloading a file.
+  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());
+
+  // Wait for the onCreated and onDeterminingFilename events.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  ASSERT_TRUE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(),
+      false));
+
+  // 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,
+                          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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL("sneaky/../../sneaky.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("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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  ASSERT_FALSE(ExtensionDownloadsEventRouter::DetermineFilename(
+      browser()->profile(),
+      false,
+      GetExtensionId(),
+      "42",
+      result_id,
+      FilePath(FILE_PATH_LITERAL(".")),
+      false));
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,

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

I find myself wondering about the interface here.  It's certainly easiest for us
to drop the determine filename on the floor in this case, but should we try and
find a way to raise an error to the user when they do something about this? 
Probably it would complicate the interface too much to add another event.  But I
wanted to at least raise the question.

[benjhayden, 2013/01/25 16:21:36]

When an extension returns an invalid filename, a red error message is printed in
the dev tools console. Filename Determination continues because it isn't waiting
on any other determiners. I might not be understanding you correctly, but I
would think that dropping the download on the floor whenever an extension
returns an invalid filename would not be the least-astonishing reaction.

Ah, the usual way to return errors to extensions is that they pass a callback to
an API function call; if the API function call encounters an error, then
chrome.extension.lastError is non-null when the callback is called. In this
case, the API function (chrome.downloadsInternal.determineFilename) is not being
called by the extension, it's being called by the secret custom bindings, so the
extension can't pass a callback to it.

[Randy Smith (Not in Mondays), 2013/01/25 18:22:45]

Ok, that's fine.

+      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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename. Absolute paths should be rejected.
+  FilePath override;
+#if defined(OS_WIN)
+  override = FilePath(FILE_PATH_LITERAL("C:\\sneaky\\sneaky.dll"));

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

Unless you do a mkdir, I wouldn't expect these directories to exist and be
writable.  I was thinking more of using /tmp and the windows equivalent; does
that not make sense to you?

[benjhayden, 2013/01/25 16:21:36]

These paths are invalid because they're absolute. They're rejected long before
chrome checks whether the directory is writable.
Besides, the ReservationTracker does mkdirs.

[Randy Smith (Not in Mondays), 2013/01/25 18:22:45]

Yes, but that's the behavior we're *testing*, so we shouldn't count on it.  I'd
like to make sure that if the absolute paths aren't rejected, we don't get a
false test success because the directories aren't writable.

[benjhayden, 2013/02/04 19:34:52]

Done.

+#else
+  override = FilePath(FILE_PATH_LITERAL("/sneaky/sneaky"));
+#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();
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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)));
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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());
+  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();
+
+  // Start downloading a file.
+  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(WaitFor(
+      std::string(events::kOnDownloadDeterminingFilename) + "/43",
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"filename\":\"slow.txt\"}]",
+                         result_id)));
+  ASSERT_TRUE(item->GetTargetFilePath().empty());
+  ASSERT_TRUE(item->IsInProgress());
+
+  // Respond to the onDeterminingFilename.
+  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)));
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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(

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

Do you want to do this after filename determination has started?  I think that
would test a different set of code, or at least in a different way.

[benjhayden, 2013/01/25 16:21:36]

Done.

+      browser()->profile(),
+      GetExtensionId(),
+      "42"));
+
+  // Start downloading a file.
+  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());
+
+  // Respond to the onDeterminingFilename.
+  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
+// TODO test interruption while extensions determining filename, re-run through
+// fan-out and fan-in
+
+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();
+
+  // Start downloading a file.
+  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();
+
+  // The download should complete because it isn't waiting for any other
+  // determiners.
+  ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
+      base::StringPrintf("[{\"id\": %d,"
+                         "  \"state\": {"
+                         "    \"previous\": \"in_progress\","
+                         "    \"current\": \"complete\"}}]",
+                         result_id)));
+}