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 df50565ed39712137fdac97d7c83f43b2a48b1f7..1ce0b24830abaf0d3e0fb967fc840e76e83ad978 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"
@@ -23,6 +24,7 @@
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_danger_prompt.h"
@@ -34,6 +36,9 @@
 #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_prefs.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/icon_loader.h"
 #include "chrome/browser/icon_manager.h"
@@ -184,14 +189,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 base::FilePath& filename) {
+  return !filename.empty() &&

[asanka, 2013/02/22 18:55:48]

"foo/" shouldn't be considered a valid filename either.

[benjhayden, 2013/02/22 20:43:23]

I believe that the next line handles that case.

[asanka, 2013/02/22 21:22:29]

For "foo/" BaseName() returns "foo".

+         (filename.BaseName().value() != base::FilePath::kCurrentDirectory) &&
+         !filename.ReferencesParent() &&
+         !filename.IsAbsolute();
 }
 
 std::string TimeToISO8601(const base::Time& t) {
@@ -263,8 +265,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,
@@ -410,7 +411,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;
@@ -507,31 +508,6 @@ void RunDownloadQuery(
   query_out.Search(all_items.begin(), all_items.end(), results);
 }
 
-void DispatchEventInternal(
-    Profile* target_profile,
-    const char* event_name,
-    const std::string& json_args,
-    scoped_ptr<base::ListValue> event_args) {
-  if (!extensions::ExtensionSystem::Get(target_profile)->event_router())
-    return;
-  scoped_ptr<extensions::Event> event(new extensions::Event(
-      event_name, event_args.Pass()));
-  event->restrict_to_profile = target_profile;
-  extensions::ExtensionSystem::Get(target_profile)->event_router()->
-      BroadcastEvent(event.Pass());
-  ExtensionDownloadsEventRouter::DownloadsNotificationSource
-    notification_source;
-  notification_source.event_name = event_name;
-  notification_source.profile = target_profile;
-  content::Source<ExtensionDownloadsEventRouter::DownloadsNotificationSource>
-    content_source(&notification_source);
-  std::string args_copy(json_args);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT,
-      content_source,
-      content::Details<std::string>(&args_copy));
-}
-
 class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {
  public:
   static ExtensionDownloadsEventRouterData* Get(DownloadItem* download_item) {
@@ -545,7 +521,8 @@ 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()),
+        determined_overwrite_(false) {
     download_item->SetUserData(kKey, this);
   }
 
@@ -564,16 +541,166 @@ 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();
+    determined_filename_.clear();
+    determined_overwrite_ = false;
+    determiner_ = DeterminerInfo();
+    filename_no_change_ = base::Closure();
+    filename_change_ = ExtensionDownloadsEventRouter::FilenameChangedCallback();
+  }
+
+  void ExtensionUnloaded(const std::string& extension_id) {
+    // 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;
+      }
+    }
+    // If we just removed the last unreported determiner, then we need to call a
+    // callback.
+    CheckAllDeterminersCalled();
+  }
+
+  void DeterminerRemoved(const std::string& extension_id,
+                         int sub_event_id) {
+    for (DeterminerInfoVector::iterator iter = determiners_.begin();
+         iter != determiners_.end();) {
+      if ((iter->extension_id == extension_id) &&
+          (iter->sub_event_id == sub_event_id)) {
+        iter = determiners_.erase(iter);
+      } else {
+        ++iter;
+      }
+    }
+    // If we just removed the last unreported determiner, then we need to call a
+    // callback.
+    CheckAllDeterminersCalled();
+  }
+
+  void AddPendingDeterminer(const std::string& extension_id,
+                            int sub_event_id,
+                            const base::Time& installed) {
+    determiners_.push_back(DeterminerInfo(
+        extension_id, sub_event_id, installed));
+  }
+
+  bool DeterminerCallback(
+      const std::string& extension_id,
+      int sub_event_id,
+      const base::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;

[asanka, 2013/02/22 18:55:48]

Curious: Is it possible for there to be more than one match?

[benjhayden, 2013/02/22 20:43:23]

I hope not. :-)
See AddPendingDeterminer and search for "already_added" in the newest patchset.

+        DCHECK(!determiners_[index].reported);
+        determiners_[index].reported = true;
+        if (!determiner_.extension_id.empty() &&
+            (determiners_[index].extension_id == determiner_.extension_id)) {
+          DCHECK_NE(determiners_[index].sub_event_id, determiner_.sub_event_id);
+        }
+        // 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_.extension_id.empty() ||
+             ((determiners_[index].extension_id == determiner_.extension_id) ?
+              (determiners_[index].sub_event_id > determiner_.sub_event_id) :
+              (determiners_[index].install_time > determiner_.install_time)))) {
+          determined_filename_ = filename;
+          determined_overwrite_ = overwrite;
+          determiner_ = determiners_[index];
+        }
+      }
+    }
+    if (!found_info)
+      return false;
+    CheckAllDeterminersCalled();
+    return filename.empty() || filename_valid;
+  }
+
  private:
+  struct DeterminerInfo {
+    DeterminerInfo();
+    DeterminerInfo(const std::string& e_id,
+                   int sub_id,
+                   const base::Time& installed);
+    ~DeterminerInfo();
+
+    std::string extension_id;
+    int sub_event_id;
+    base::Time install_time;
+    bool reported;
+  };
+  typedef std::vector<DeterminerInfo> DeterminerInfoVector;
+
   static const char kKey[];
 
+  // This is safe to call even while not waiting for determiners to call back;
+  // in that case, the callbacks will be null so they won't be Run.
+  void CheckAllDeterminersCalled() {
+    for (DeterminerInfoVector::iterator iter = determiners_.begin();
+         iter != determiners_.end(); ++iter) {
+      if (!iter->reported)
+        return;
+    }
+    if (determined_filename_.empty()) {
+      if (!filename_no_change_.is_null())
+        filename_no_change_.Run();
+    } else {
+      if (!filename_change_.is_null())
+        filename_change_.Run(determined_filename_, determined_overwrite_);
+    }
+    ClearPendingDeterminers();
+  }
+
   int updated_;
   int changed_fired_;
   scoped_ptr<base::DictionaryValue> json_;
 
+  base::Closure filename_no_change_;
+  ExtensionDownloadsEventRouter::FilenameChangedCallback filename_change_;
+
+  DeterminerInfoVector determiners_;
+
+  base::FilePath determined_filename_;
+  bool determined_overwrite_;
+  DeterminerInfo determiner_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionDownloadsEventRouterData);
 };
 
+ExtensionDownloadsEventRouterData::DeterminerInfo::DeterminerInfo(
+    const std::string& e_id,
+    int sub_id,
+    const base::Time& installed)
+    : extension_id(e_id),
+      sub_event_id(sub_id),
+      install_time(installed),
+      reported(false) {
+}
+
+ExtensionDownloadsEventRouterData::DeterminerInfo::DeterminerInfo()
+    : reported(false) {
+}
+
+ExtensionDownloadsEventRouterData::DeterminerInfo::~DeterminerInfo() {}
+
 const char ExtensionDownloadsEventRouterData::kKey[] =
   "DownloadItem ExtensionDownloadsEventRouterData";
 
@@ -636,6 +763,7 @@ std::map<DownloadManager*, ManagerDestructionObserver*>*
 }  // namespace
 
 DownloadsDownloadFunction::DownloadsDownloadFunction() {}
+
 DownloadsDownloadFunction::~DownloadsDownloadFunction() {}
 
 bool DownloadsDownloadFunction::RunImpl() {
@@ -672,7 +800,13 @@ bool DownloadsDownloadFunction::RunImpl() {
     string16 filename16;
     EXTENSION_FUNCTION_VALIDATE(options_value->GetString(
         kFilenameKey, &filename16));
-    if (!ValidateFilename(filename16)) {
+#if defined(OS_WIN)
+    base::FilePath file_path(filename16);
+#elif defined(OS_POSIX)
+    base::FilePath file_path(*options.filename.get());
+#endif
+    if (!ValidateFilename(file_path) ||
+        (file_path.DirName().value() != base::FilePath::kCurrentDirectory)) {
       error_ = download_extension_errors::kGenericError;
       return false;
     }
@@ -732,6 +866,7 @@ void DownloadsDownloadFunction::OnStarted(
 }
 
 DownloadsSearchFunction::DownloadsSearchFunction() {}
+
 DownloadsSearchFunction::~DownloadsSearchFunction() {}
 
 bool DownloadsSearchFunction::RunImpl() {
@@ -769,6 +904,7 @@ bool DownloadsSearchFunction::RunImpl() {
 }
 
 DownloadsPauseFunction::DownloadsPauseFunction() {}
+
 DownloadsPauseFunction::~DownloadsPauseFunction() {}
 
 bool DownloadsPauseFunction::RunImpl() {
@@ -792,6 +928,7 @@ bool DownloadsPauseFunction::RunImpl() {
 }
 
 DownloadsResumeFunction::DownloadsResumeFunction() {}
+
 DownloadsResumeFunction::~DownloadsResumeFunction() {}
 
 bool DownloadsResumeFunction::RunImpl() {
@@ -806,7 +943,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())
@@ -815,6 +952,7 @@ bool DownloadsResumeFunction::RunImpl() {
 }
 
 DownloadsCancelFunction::DownloadsCancelFunction() {}
+
 DownloadsCancelFunction::~DownloadsCancelFunction() {}
 
 bool DownloadsCancelFunction::RunImpl() {
@@ -826,13 +964,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() {
@@ -861,20 +999,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() {
@@ -918,6 +1044,7 @@ void DownloadsAcceptDangerFunction::DangerPromptCallback(
 }
 
 DownloadsShowFunction::DownloadsShowFunction() {}
+
 DownloadsShowFunction::~DownloadsShowFunction() {}
 
 bool DownloadsShowFunction::RunImpl() {
@@ -936,6 +1063,7 @@ bool DownloadsShowFunction::RunImpl() {
 }
 
 DownloadsOpenFunction::DownloadsOpenFunction() {}
+
 DownloadsOpenFunction::~DownloadsOpenFunction() {}
 
 bool DownloadsOpenFunction::RunImpl() {
@@ -954,6 +1082,7 @@ bool DownloadsOpenFunction::RunImpl() {
 }
 
 DownloadsDragFunction::DownloadsDragFunction() {}
+
 DownloadsDragFunction::~DownloadsDragFunction() {}
 
 bool DownloadsDragFunction::RunImpl() {
@@ -1008,8 +1137,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(
@@ -1030,6 +1159,19 @@ void DownloadsGetFileIconFunction::OnIconURLExtracted(const std::string& url) {
   SendResponse(error_.empty());
 }
 
+ExtensionDownloadsEventRouter::DeterminerId::DeterminerId(
+    const std::string& ext_id,
+    int sub_id,
+    bool incognito,
+    const base::Time& install)
+    : extension_id(ext_id),
+      sub_event_id(sub_id),
+      include_incognito(incognito),
+      installed(install) {
+}
+
+ExtensionDownloadsEventRouter::DeterminerId::~DeterminerId() {}
+
 ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
     Profile* profile,
     DownloadManager* manager)
@@ -1037,12 +1179,230 @@ 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_));
 }
 
 ExtensionDownloadsEventRouter::~ExtensionDownloadsEventRouter() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
+// 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::OnDeterminingFilename (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,
+    bool include_incognito,
+    const std::string& ext_id,
+    int 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
+  // 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;
+  }
+  base::Time installed = extensions::ExtensionSystem::Get(
+      profile)->extension_service()->extension_prefs()->GetInstallTime(ext_id);
+  router->determiners_.push_back(DeterminerId(
+      ext_id, sub_event_id, include_incognito, installed));
+  return true;
+}
+
+bool ExtensionDownloadsEventRouter::RemoveFilenameDeterminer(
+    Profile* profile,
+    const std::string& ext_id,
+    int 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);
+      // Notify any items that may be waiting for callbacks from this
+      // determiner.
+      DownloadManager* manager = router->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->DeterminerRemoved(ext_id, sub_event_id);
+        }
+      }
+      return true;
+    }
+  }
+  DCHECK(false) << ext_id << " " << sub_event_id;
+  return false;
+}
+
+void ExtensionDownloadsEventRouter::OnDeterminingFilename(
+    DownloadItem* item,
+    const base::FilePath& suggested_path,
+    const base::Closure& no_change,
+    const FilenameChangedCallback& change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  bool any_determiners = false;
+
+  ExtensionDownloadsEventRouterData* data =
+      ExtensionDownloadsEventRouterData::Get(item);
+  data->ClearPendingDeterminers();

[asanka, 2013/02/22 18:55:48]

Is it possible for there to be a pending determiner for this download item?

It is possible for a single DownloadItem to determine the download target
multiple times due to download resumption. However, it shouldn't be possible for
these to overlap.

[benjhayden, 2013/02/22 20:43:23]

Yep, Randy suggested clearing pending determiners here just for the resumption
filename re-determination case.
If an onDeterminingFilename handler is holding off on suggest()ing a filename,
and the user pauses and resumes a download in a way that triggers
re-determination, then this will ClearPendingDeterminers.

+  data->set_filename_change_callbacks(no_change, change);
+  std::set<int> 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);
+    any_determiners = true;
+  }
+
+  if (profile_->IsOffTheRecord()) {
+    // If this DownloadItem is incognito and there are any
+    // incognito-spanning-mode extensions registered in the on-record EDER, then
+    // dispatch to them and wait for them as well.
+    ExtensionDownloadsEventRouter* router =
+        DownloadServiceFactory::GetForProfile(
+            profile_->GetOriginalProfile())->GetExtensionEventRouter();
+    for (DeterminerVector::const_iterator iter = router->determiners_.begin();
+        iter != router->determiners_.end(); ++iter) {
+      if (iter->include_incognito) {
+        data->AddPendingDeterminer(iter->extension_id,
+                                   iter->sub_event_id,
+                                   iter->installed);
+        sub_events.insert(iter->sub_event_id);
+        any_determiners = true;
+      }
+    }
+  }
+
+  std::string event_name(
+      extensions::event_names::kOnDownloadDeterminingFilename);
+  event_name += "/";
+  for (std::set<int>::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 + base::IntToString(*iter)).c_str(), false, json);
+  }
+
+  if (!any_determiners) {
+    no_change.Run();
+    return;
+  }
+}
+
+bool ExtensionDownloadsEventRouter::DetermineFilename(
+    Profile* profile,
+    bool include_incognito,
+    const std::string& ext_id,
+    int sub_event_id,
+    int download_id,
+    const base::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);
+}
+
+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;
+  }
+}
+
+// That's all the methods that have to do with filename determination. The rest
+// have to do with the other, less special events.
+
 void ExtensionDownloadsEventRouter::OnDownloadCreated(
     DownloadManager* manager, DownloadItem* download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -1052,6 +1412,7 @@ 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());
@@ -1071,7 +1432,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;
@@ -1111,7 +1472,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());
@@ -1123,33 +1486,35 @@ 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));
+  if (!extensions::ExtensionSystem::Get(profile_)->event_router())
+    return;
   scoped_ptr<base::ListValue> args(new base::ListValue());
   args->Append(arg);
   std::string json_args;
   base::JSONWriter::Write(args.get(), &json_args);
-  // 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() &&
-      !profile_->IsOffTheRecord()) {
-    DispatchEventInternal(
-        profile_->GetOffTheRecordProfile(),
-        event_name,
-        json_args,
-        scoped_ptr<base::ListValue>(args->DeepCopy()));
-  }
-  DispatchEventInternal(profile_, event_name, json_args, args.Pass());
+  scoped_ptr<extensions::Event> event(new extensions::Event(
+      event_name, args.Pass()));
+  // "restrict_to_profile" is not smart enough to avoid sharing off-record
+  // events with on-record extension renderers.

[Matt Perry, 2013/02/22 01:43:43]

This comment is a bit cryptic to me.

The main quirk with what you're doing is that on-record events are not
restricted to a profile, even in the split-mode case. I would add a comment to
that effect.

[benjhayden, 2013/02/22 20:43:23]

Done.

+  event->restrict_to_profile =
+      (include_incognito && !profile_->IsOffTheRecord()) ? NULL : profile_;
+  extensions::ExtensionSystem::Get(profile_)->event_router()->
+      BroadcastEvent(event.Pass());
+  ExtensionDownloadsEventRouter::DownloadsNotificationSource

[Matt Perry, 2013/02/22 01:43:43]

nit: you're already in this scope so you can drop the
ExtensionDownloadsEventRouter:: prefix

[benjhayden, 2013/02/22 20:43:23]

Done.

+    notification_source;
+  notification_source.event_name = event_name;
+  notification_source.profile = profile_;
+  content::Source<ExtensionDownloadsEventRouter::DownloadsNotificationSource>

[Matt Perry, 2013/02/22 01:43:43]

same here

[benjhayden, 2013/02/22 20:43:23]

Done.

+    content_source(&notification_source);
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT,
+      content_source,
+      content::Details<std::string>(&json_args));
 }