Implement chrome.downloads.onDeterminingFilename()

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

Index: chrome/browser/extensions/api/downloads/downloads_api.cc
diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc
index 931ee6bd489d52a03df1acd887a1590f52c374cf..9402de5d4c01f0e00aa1fe7bed3cc5e48a30a221 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api.cc
@@ -14,6 +14,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/file_path.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ -33,6 +34,7 @@
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
+#include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/icon_loader.h"
 #include "chrome/browser/icon_manager.h"
@@ -179,14 +181,11 @@ DownloadItem::DownloadState StateEnumFromString(const std::string& state) {
   return DownloadItem::MAX_DOWNLOAD_STATE;
 }
 
-bool ValidateFilename(const string16& filename) {
-  // TODO(benjhayden): More robust validation of filename.
-  if ((filename.find('/') != string16::npos) ||
-      (filename.find('\\') != string16::npos))
-    return false;
-  if (filename.size() >= 2u && filename[0] == L'.' && filename[1] == L'.')
-    return false;
-  return true;
+bool ValidateFilename(const FilePath& filename) {
+  return !filename.empty() &&
+         (filename.BaseName().value() != FilePath::kCurrentDirectory) &&
+         !filename.ReferencesParent() &&
+         !filename.IsAbsolute();
 }
 
 std::string TimeToISO8601(const base::Time& t) {
@@ -258,8 +257,7 @@ bool DownloadFileIconExtractorImpl::ExtractIconURLForPath(
   IconManager* im = g_browser_process->icon_manager();
   // The contents of the file at |path| may have changed since a previous
   // request, in which case the associated icon may also have changed.
-  // Therefore, for the moment we always call LoadIcon instead of attempting
-  // a LookupIcon.
+  // Therefore, always call LoadIcon instead of attempting a LookupIcon.
   im->LoadIcon(path,
                icon_size,
                base::Bind(&DownloadFileIconExtractorImpl::OnIconLoadComplete,
@@ -405,7 +403,7 @@ void CompileDownloadQueryOrderBy(
   std::vector<std::string> order_by_strs;
   base::SplitString(order_by_str, ' ', &order_by_strs);
   for (std::vector<std::string>::const_iterator iter = order_by_strs.begin();
-      iter != order_by_strs.end(); ++iter) {
+       iter != order_by_strs.end(); ++iter) {
     std::string term_str = *iter;
     if (term_str.empty())
       continue;
@@ -540,7 +538,9 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {
       scoped_ptr<base::DictionaryValue> json_item)
       : updated_(0),
         changed_fired_(0),
-        json_(json_item.Pass()) {
+        json_(json_item.Pass()),
+        determiner_index_(-1),
+        determined_overwrite_(false) {
     download_item->SetUserData(kKey, this);
   }
 
@@ -559,16 +559,136 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {
   void OnItemUpdated() { ++updated_; }
   void OnChangedFired() { ++changed_fired_; }
 
+  void set_filename_change_callbacks(
+      const base::Closure& no_change,
+      const ExtensionDownloadsEventRouter::FilenameChangedCallback& change) {
+    filename_no_change_ = no_change;
+    filename_change_ = change;
+  }
+
+  void ClearPendingDeterminers() {
+    determiners_.clear();
+  }
+
+  void ExtensionUnloaded(const std::string& extension_id) {
+    if (determiners_.empty())

[Randy Smith (Not in Mondays), 2013/01/17 19:15:42]

When I first read through this, I spent a couple of minutes being confused about
why this statement was here, since a loop traversal should do nothing if the
list was empty.  I eventually realized that it was a guard against EDERDs that
weren't currently involved in a call out to extensions getting their callbacks
calls.  But I could easily imagine future code writers thinking it was an
optimization.  Comment?

[benjhayden, 2013/01/18 20:59:50]

Done.

+      return;
+    // Remove this extension from |determiners_|.
+    for (DeterminerInfoVector::iterator iter = determiners_.begin();
+         iter != determiners_.end();) {
+      if (iter->extension_id == extension_id) {
+        iter = determiners_.erase(iter);
+      } else {
+        ++iter;
+      }
+    }
+    CheckAllDeterminersCalled();
+  }
+
+  void AddPendingDeterminer(const std::string& extension_id,
+                            const std::string& sub_event_id,
+                            const base::Time& installed) {
+    determiners_.push_back(DeterminerInfo(
+        extension_id, sub_event_id, installed));
+  }
+
+  bool DeterminerCallback(
+      const std::string& extension_id,
+      const std::string& sub_event_id,
+      const FilePath& filename,
+      bool overwrite) {
+    bool filename_valid = ValidateFilename(filename);
+    bool found_info = false;
+    for (int index = 0; index < static_cast<int>(determiners_.size());
+         ++index) {
+      if ((determiners_[index].extension_id == extension_id) &&
+          (determiners_[index].sub_event_id == sub_event_id)) {
+        found_info = true;
+        DCHECK(!determiners_[index].reported);
+        determiners_[index].reported = true;
+        DCHECK_NE(index, determiner_index_);
+        // Do not use filename if another determiner has already overridden the
+        // filename and they take precedence. Extensions that were installed
+        // later take precedence over previous extensions, and listeners that
+        // were added later take precedence over previous listeners.
+        if (filename_valid &&
+            ((determiner_index_ < 0) ||
+             ((determiners_[index].extension_id ==
+               determiners_[determiner_index_].extension_id) ?
+              (determiners_[index].sub_event_id >
+               determiners_[determiner_index_].sub_event_id) :
+              (determiners_[index].install_time >
+               determiners_[determiner_index_].install_time)))) {
+          determined_filename_ = filename;
+          determined_overwrite_ = overwrite;
+          determiner_index_ = index;
+        }
+      }
+    }
+    if (!found_info)
+      return false;
+    CheckAllDeterminersCalled();
+    return filename.empty() || filename_valid;
+  }
+
  private:
+  struct DeterminerInfo {
+    DeterminerInfo(const std::string& e_id,
+                   const std::string& sub_id,
+                   const base::Time& installed);
+    ~DeterminerInfo();
+
+    std::string extension_id;
+    std::string sub_event_id;
+    base::Time install_time;
+    bool reported;
+  };
+  typedef std::vector<DeterminerInfo> DeterminerInfoVector;
+
   static const char kKey[];
 
+  void CheckAllDeterminersCalled() {
+    for (DeterminerInfoVector::iterator iter = determiners_.begin();
+         iter != determiners_.end(); ++iter) {
+      if (!iter->reported)
+        return;
+    }
+    ClearPendingDeterminers();
+    if (determined_filename_.empty()) {
+      filename_no_change_.Run();
+    } else {
+      filename_change_.Run(determined_filename_, determined_overwrite_);
+    }
+  }
+
   int updated_;
   int changed_fired_;
   scoped_ptr<base::DictionaryValue> json_;
 
+  base::Closure filename_no_change_;
+  ExtensionDownloadsEventRouter::FilenameChangedCallback filename_change_;
+
+  DeterminerInfoVector determiners_;
+
+  int determiner_index_;
+  FilePath determined_filename_;

[Randy Smith (Not in Mondays), 2013/01/17 19:15:42]

I may have missed it, but where do you reset these values?  I'm specifically
thinking of the interrupt->resume case where we call back into this code; if
determiner_index_ is somewhat high, won't that result in results from some
extensions being dropped on the floor?  And if the second time through no-one
overrides, won't we re-use determined_filename_?

[benjhayden, 2013/01/18 20:59:50]

Good catch. Added that to right after where filename_change_ is run.

+  bool determined_overwrite_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionDownloadsEventRouterData);
 };
 
+ExtensionDownloadsEventRouterData::DeterminerInfo::DeterminerInfo(
+    const std::string& e_id,
+    const std::string& sub_id,
+    const base::Time& installed)
+    : extension_id(e_id),
+      sub_event_id(sub_id),
+      install_time(installed),
+      reported(false) {
+}
+
+ExtensionDownloadsEventRouterData::DeterminerInfo::~DeterminerInfo() {}
+
 const char ExtensionDownloadsEventRouterData::kKey[] =
   "DownloadItem ExtensionDownloadsEventRouterData";
 
@@ -631,6 +751,7 @@ std::map<DownloadManager*, ManagerDestructionObserver*>*
 }  // namespace
 
 DownloadsDownloadFunction::DownloadsDownloadFunction() {}
+
 DownloadsDownloadFunction::~DownloadsDownloadFunction() {}
 
 bool DownloadsDownloadFunction::RunImpl() {
@@ -667,7 +788,13 @@ bool DownloadsDownloadFunction::RunImpl() {
     string16 filename16;
     EXTENSION_FUNCTION_VALIDATE(options_value->GetString(
         kFilenameKey, &filename16));
-    if (!ValidateFilename(filename16)) {
+#if defined(OS_WIN)
+    FilePath file_path(filename16);

[Randy Smith (Not in Mondays), 2013/01/17 19:15:42]

nit: Move everything that's only relevant to creating filename16 inside the if,
to make the conditionalization clearer.

[benjhayden, 2013/01/18 20:59:50]

No, filename16 is used on L803 of this PS.

+#elif defined(OS_POSIX)
+    FilePath file_path(*options.filename.get());
+#endif
+    if (!ValidateFilename(file_path) ||
+        (file_path.DirName().value() != FilePath::kCurrentDirectory)) {
       error_ = download_extension_errors::kGenericError;
       return false;
     }
@@ -727,6 +854,7 @@ void DownloadsDownloadFunction::OnStarted(
 }
 
 DownloadsSearchFunction::DownloadsSearchFunction() {}
+
 DownloadsSearchFunction::~DownloadsSearchFunction() {}
 
 bool DownloadsSearchFunction::RunImpl() {
@@ -764,6 +892,7 @@ bool DownloadsSearchFunction::RunImpl() {
 }
 
 DownloadsPauseFunction::DownloadsPauseFunction() {}
+
 DownloadsPauseFunction::~DownloadsPauseFunction() {}
 
 bool DownloadsPauseFunction::RunImpl() {
@@ -787,6 +916,7 @@ bool DownloadsPauseFunction::RunImpl() {
 }
 
 DownloadsResumeFunction::DownloadsResumeFunction() {}
+
 DownloadsResumeFunction::~DownloadsResumeFunction() {}
 
 bool DownloadsResumeFunction::RunImpl() {
@@ -801,7 +931,7 @@ bool DownloadsResumeFunction::RunImpl() {
     error_ = download_extension_errors::kInvalidOperationError;
   } else {
     // Note that if the item isn't paused, this will be a no-op, and
-    // we will silently treat the extension call as a success.
+    // the extension call will seem successful.
     download_item->Resume();
   }
   if (error_.empty())
@@ -810,6 +940,7 @@ bool DownloadsResumeFunction::RunImpl() {
 }
 
 DownloadsCancelFunction::DownloadsCancelFunction() {}
+
 DownloadsCancelFunction::~DownloadsCancelFunction() {}
 
 bool DownloadsCancelFunction::RunImpl() {
@@ -821,13 +952,13 @@ bool DownloadsCancelFunction::RunImpl() {
   if (download_item != NULL)
     download_item->Cancel(true);
   // |download_item| can be NULL if the download ID was invalid or if the
-  // download is not currently active.  Either way, we don't consider it a
-  // failure.
+  // download is not currently active.  Either way, it's not a failure.
   RecordApiFunctions(DOWNLOADS_FUNCTION_CANCEL);
   return true;
 }
 
 DownloadsEraseFunction::DownloadsEraseFunction() {}
+
 DownloadsEraseFunction::~DownloadsEraseFunction() {}
 
 bool DownloadsEraseFunction::RunImpl() {
@@ -856,20 +987,8 @@ bool DownloadsEraseFunction::RunImpl() {
   return true;
 }
 
-DownloadsSetDestinationFunction::DownloadsSetDestinationFunction() {}
-DownloadsSetDestinationFunction::~DownloadsSetDestinationFunction() {}
-
-bool DownloadsSetDestinationFunction::RunImpl() {
-  scoped_ptr<extensions::api::downloads::SetDestination::Params> params(
-      extensions::api::downloads::SetDestination::Params::Create(*args_));
-  EXTENSION_FUNCTION_VALIDATE(params.get());
-  error_ = download_extension_errors::kNotImplementedError;
-  if (error_.empty())
-    RecordApiFunctions(DOWNLOADS_FUNCTION_SET_DESTINATION);
-  return error_.empty();
-}
-
 DownloadsAcceptDangerFunction::DownloadsAcceptDangerFunction() {}
+
 DownloadsAcceptDangerFunction::~DownloadsAcceptDangerFunction() {}
 
 bool DownloadsAcceptDangerFunction::RunImpl() {
@@ -883,6 +1002,7 @@ bool DownloadsAcceptDangerFunction::RunImpl() {
 }
 
 DownloadsShowFunction::DownloadsShowFunction() {}
+
 DownloadsShowFunction::~DownloadsShowFunction() {}
 
 bool DownloadsShowFunction::RunImpl() {
@@ -901,6 +1021,7 @@ bool DownloadsShowFunction::RunImpl() {
 }
 
 DownloadsOpenFunction::DownloadsOpenFunction() {}
+
 DownloadsOpenFunction::~DownloadsOpenFunction() {}
 
 bool DownloadsOpenFunction::RunImpl() {
@@ -919,6 +1040,7 @@ bool DownloadsOpenFunction::RunImpl() {
 }
 
 DownloadsDragFunction::DownloadsDragFunction() {}
+
 DownloadsDragFunction::~DownloadsDragFunction() {}
 
 bool DownloadsDragFunction::RunImpl() {
@@ -973,8 +1095,8 @@ bool DownloadsGetFileIconFunction::RunImpl() {
     return false;
   }
   // In-progress downloads return the intermediate filename for GetFullPath()
-  // which doesn't have the final extension. Therefore we won't be able to
-  // derive a good file icon for it. So we use GetTargetFilePath() instead.
+  // which doesn't have the final extension. Therefore a good file icon can't be
+  // found, so use GetTargetFilePath() instead.
   DCHECK(icon_extractor_.get());
   DCHECK(icon_size == 16 || icon_size == 32);
   EXTENSION_FUNCTION_VALIDATE(icon_extractor_->ExtractIconURLForPath(
@@ -995,6 +1117,123 @@ void DownloadsGetFileIconFunction::OnIconURLExtracted(const std::string& url) {
   SendResponse(error_.empty());
 }
 
+// The method by which extensions hook into the filename determination process
+// is based on the method by which the webRequest API allows extensions to hook
+// into the resource loading process.  Extensions that wish to play a part in
+// the filename determination process call
+// chrome.downloads.onDeterminingFilename.addListener, which secretly (see
+// chrome/renderer/resources/extensions/downloads_custom_bindings.js) calls
+// DownloadsInternalAddFilenameDeterminerFunction (see ../downloads_internal/),
+// which adds the extension's ID to the profile's ExtensionDownloadsEventRouter
+// (EDER).
+//
+// When a download's filename is being determined,
+// ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone (CVRBD) passes
+// 2 callbacks to EDER::OnDownloadFilenameDetermined (ODFD), which stores the
+// callbacks in the item's ExtensionDownloadsEventRouterData (EDERD) along with
+// all of the extension IDs that are listening for onDeterminingFilename events.
+// ODFD dispatches chrome.downloads.onDeterminingFilename.
+//
+// When the extension's event handler returns, downloads_custom_bindings.js
+// calls DownloadsInternalDetermineFilenameFunction::RunImpl, which notifies the
+// item's EDERD.
+//
+// When the last extension's event handler returns, EDERD calls one of the two
+// callbacks that CVRBD passed to ODFD, allowing CDMD to complete the filename
+// determination process. If multiple extensions wish to override the filename,
+// then the extension that was last installed wins; if multiple determiners
+// within an extension compete, then the determiner that was last added wins.
+
+bool ExtensionDownloadsEventRouter::AddFilenameDeterminer(
+    Profile* profile,
+    const std::string& ext_id,
+    const std::string& sub_event_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // Only incognito/regular renderer processes receive onDeterminingFilename
+  // events for incognito/regular downloads.  Do not automatically add this

[Randy Smith (Not in Mondays), 2013/01/17 19:15:42]

So this seems weird to me.  My understanding (please correct if wrong) is that
everywhere else we give incognito extension full and symmetric visibility to
both incognito and regular downloads (within that profile).  But it sounds like
here we're breaking that symmetry and only allowing interposition for filename
determination for non-incognito profiles by non-incognito extensions.  Why?

[benjhayden, 2013/01/18 20:59:50]

Nope, when incognito extensions' renderers call
onDeterminingFilename.addListener, the |profile| that is passed here is the
incognito profile, so it'll use the incognito EDER.

I agree, it does feel a little weird how onDeterminingFIlename works with
incognito differently from how the other events work with incognito, but
onDeterminingFilename does everything else differently, too.

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

Sounds good.  As discussed offline, could you:
* Convince yourself that this pattern (probes from incognito extensions seeing
both regular and incognito data, but inserted listeners just getting events from
their profile) is common and well understood among extensions.
* Failing that, check in with someone Wise In the Ways of Extensions to confirm
it makes sense to do it this way, and make sure to document it in the extensions
API documentation?

+  // determiner to the other (incognito/regular) EDER; if the extension wants to
+  // listen for onDeterminingFilename in those extension renderer processes,
+  // then it will call addListener there, and |profile| will be
+  // incognito/regular for those addListener calls, so they will use the
+  // incognito/regular EDER.
+  ExtensionDownloadsEventRouter* router =
+      DownloadServiceFactory::GetForProfile(profile)->GetExtensionEventRouter();
+  if (!router)
+    return false;
+  for (DeterminerVector::const_iterator iter = router->determiners_.begin();
+       iter != router->determiners_.end(); ++iter) {
+    if (iter->extension_id == ext_id) {
+      DCHECK_NE(iter->sub_event_id, sub_event_id);
+    }
+    if ((iter->extension_id == ext_id) &&
+        (iter->sub_event_id == sub_event_id))
+      return false;
+  }
+  router->determiners_.push_back(DeterminerId(ext_id, sub_event_id));
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
+      &ExtensionDownloadsEventRouter::GetInstallTime,
+      ext_id,
+      scoped_refptr<ExtensionInfoMap>(
+          extensions::ExtensionSystem::Get(profile)->info_map()),
+      &router->determiners_[router->determiners_.size() - 1].installed));
+  return true;
+}
+
+void ExtensionDownloadsEventRouter::GetInstallTime(
+    const std::string& ext_id,
+    ExtensionInfoMap* info_map,
+    base::Time* install_time) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  *install_time = info_map->GetInstallTime(ext_id);
+}
+
+ExtensionDownloadsEventRouter::DeterminerId::DeterminerId(

[Randy Smith (Not in Mondays), 2013/01/17 19:15:42]

So there's an officially sanctioned order for methods, which is the same in the
.cc file as it is in the .h file.  I'm not neurotic about that; if there's some
other order that makes sense, go for it.  But I don't see why interrupting the
EDER methods in order to put in some trivial methods for EDER::DeterminerId
would count?

[benjhayden, 2013/01/18 20:59:50]

Done.

+    const std::string& ext_id,
+    const std::string& sub_id)
+    : extension_id(ext_id),
+      sub_event_id(sub_id) {
+}
+
+ExtensionDownloadsEventRouter::DeterminerId::~DeterminerId() {}
+
+bool ExtensionDownloadsEventRouter::RemoveFilenameDeterminer(
+    Profile* profile,
+    const std::string& ext_id,
+    const std::string& sub_event_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  ExtensionDownloadsEventRouter* router =
+      DownloadServiceFactory::GetForProfile(profile)->GetExtensionEventRouter();
+  for (DeterminerVector::iterator iter = router->determiners_.begin();
+       iter != router->determiners_.end(); ++iter) {
+    if (iter->extension_id == ext_id && iter->sub_event_id == sub_event_id) {
+      router->determiners_.erase(iter);
+      return true;
+    }
+  }
+  DCHECK(false) << ext_id << " " << sub_event_id;
+  return false;
+}
+
+bool ExtensionDownloadsEventRouter::DetermineFilename(
+    Profile* profile,
+    bool include_incognito,
+    const std::string& ext_id,
+    const std::string& sub_event_id,
+    int download_id,
+    const FilePath& filename,
+    bool overwrite) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadItem* item = GetDownloadIfInProgress(
+      profile, include_incognito, download_id);
+  if (!item)
+    return false;
+  ExtensionDownloadsEventRouterData* data =
+      ExtensionDownloadsEventRouterData::Get(item);
+  if (!data)
+    return false;
+  return data->DeterminerCallback(ext_id, sub_event_id, filename, overwrite);
+}
+
 ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
     Profile* profile,
     DownloadManager* manager)
@@ -1002,6 +1241,47 @@ ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
       ALLOW_THIS_IN_INITIALIZER_LIST(notifier_(manager, this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(profile_);
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile_));
+}
+
+void ExtensionDownloadsEventRouter::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+      const extensions::Extension* extension = content::Details<
+          extensions::UnloadedExtensionInfo>(details)->extension;
+      // Remove this extension from |determiners_|.
+      for (DeterminerVector::iterator iter = determiners_.begin();
+           iter != determiners_.end();) {
+        if (iter->extension_id == extension->id()) {
+          iter = determiners_.erase(iter);
+        } else {
+          ++iter;
+        }
+      }
+      // Notify any items that may be waiting for callbacks from this extension.
+      DownloadManager* manager = notifier_.GetManager();
+      if (manager) {
+        DownloadManager::DownloadVector items;
+        manager->GetAllDownloads(&items);
+        for (DownloadManager::DownloadVector::const_iterator iter =
+             items.begin();
+             iter != items.end(); ++iter) {
+          ExtensionDownloadsEventRouterData* data =
+              ExtensionDownloadsEventRouterData::Get(*iter);
+          if (data)
+            data->ExtensionUnloaded(extension->id());
+        }
+      }
+      break;
+    }
+    default:
+      NOTREACHED();
+      break;
+  }
 }
 
 ExtensionDownloadsEventRouter::~ExtensionDownloadsEventRouter() {
@@ -1017,11 +1297,50 @@ void ExtensionDownloadsEventRouter::OnDownloadCreated(
   scoped_ptr<base::DictionaryValue> json_item(
       DownloadItemToJSON(download_item, profile_->IsOffTheRecord()));
   DispatchEvent(extensions::event_names::kOnDownloadCreated,
+                true,
                 json_item->DeepCopy());
   if (!ExtensionDownloadsEventRouterData::Get(download_item))
     new ExtensionDownloadsEventRouterData(download_item, json_item.Pass());
 }
 
+void ExtensionDownloadsEventRouter::OnDownloadFilenameDetermined(
+    DownloadItem* item,
+    const FilePath& suggested_path,
+    const base::Closure& no_change,
+    const FilenameChangedCallback& change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (determiners_.empty()) {
+    no_change.Run();
+    return;
+  }
+
+  ExtensionDownloadsEventRouterData* data =
+      ExtensionDownloadsEventRouterData::Get(item);
+  data->set_filename_change_callbacks(no_change, change);
+  data->ClearPendingDeterminers();
+  std::set<std::string> sub_events;
+  for (DeterminerVector::const_iterator iter = determiners_.begin();
+       iter != determiners_.end(); ++iter) {
+    data->AddPendingDeterminer(iter->extension_id,
+                               iter->sub_event_id,
+                               iter->installed);
+    sub_events.insert(iter->sub_event_id);
+  }
+  std::string event_name(
+      extensions::event_names::kOnDownloadDeterminingFilename);
+  event_name += "/";
+  for (std::set<std::string>::const_iterator iter = sub_events.begin();
+       iter != sub_events.end(); ++iter) {
+    base::DictionaryValue* json = DownloadItemToJSON(
+        item, profile_->IsOffTheRecord()).release();
+    json->SetString(kFilenameKey, suggested_path.LossyDisplayName());
+    // If there's a listener in an incognito extension renderer, then it will
+    // have registered its own determiner in the incognito EDER, which will be
+    // called for incognito downloads.
+    DispatchEvent((event_name + (*iter)).c_str(), false, json);
+  }
+}
+
 void ExtensionDownloadsEventRouter::OnDownloadUpdated(
     DownloadManager* manager, DownloadItem* download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -1036,7 +1355,7 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated(
     return;
   }
   scoped_ptr<base::DictionaryValue> new_json(DownloadItemToJSON(
-        download_item, profile_->IsOffTheRecord()));
+      download_item, profile_->IsOffTheRecord()));
   scoped_ptr<base::DictionaryValue> delta(new base::DictionaryValue());
   delta->SetInteger(kIdKey, download_item->GetId());
   std::set<std::string> new_fields;
@@ -1076,7 +1395,9 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated(
   // changed. Replace the stored json with the new json.
   data->OnItemUpdated();
   if (changed) {
-    DispatchEvent(extensions::event_names::kOnDownloadChanged, delta.release());
+    DispatchEvent(extensions::event_names::kOnDownloadChanged,
+                  true,
+                  delta.release());
     data->OnChangedFired();
   }
   data->set_json(new_json.Pass());
@@ -1088,11 +1409,12 @@ void ExtensionDownloadsEventRouter::OnDownloadRemoved(
   if (download_item->IsTemporary())
     return;
   DispatchEvent(extensions::event_names::kOnDownloadErased,
+                true,
                 base::Value::CreateIntegerValue(download_item->GetId()));
 }
 
 void ExtensionDownloadsEventRouter::DispatchEvent(
-    const char* event_name, base::Value* arg) {
+    const char* event_name, bool include_incognito, base::Value* arg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   scoped_ptr<base::ListValue> args(new base::ListValue());
   args->Append(arg);
@@ -1101,14 +1423,15 @@ void ExtensionDownloadsEventRouter::DispatchEvent(
   // There is a one EDER for each on-record Profile, and a separate EDER for
   // each off-record Profile, so there is exactly one EDER for each
   // DownloadManager.  EDER only watches its own DM, so all the items that an
-  // EDER sees are either all on-record or all off-record. However, we want
-  // extensions in off-record contexts to see on-record items. So, if this EDER
-  // is watching an on-record DM, and there is a corresponding off-record
-  // Profile, then dispatch this event to both the on-record Profile and the
-  // off-record Profile.  There may or may not be an off-record Profile, so send
-  // a *copy* of |args| to the off-record Profile, and Pass() |args|
-  // to the Profile that we know is there.
-  if (profile_->HasOffTheRecordProfile() &&
+  // EDER sees are either all on-record or all off-record. However, extensions
+  // in off-record contexts should see on-record items. So, if this EDER is
+  // watching an on-record DM, and there is a corresponding off-record Profile,
+  // then dispatch this event to both the on-record Profile and the off-record
+  // Profile.  There may or may not be an off-record Profile, so send a *copy*
+  // of |args| to the off-record Profile, and Pass() |args| to the Profile that
+  // is certainly there.
+  if (include_incognito &&
+      profile_->HasOffTheRecordProfile() &&
       !profile_->IsOffTheRecord()) {
     DispatchEventInternal(
         profile_->GetOffTheRecordProfile(),