Save Page As MHTML

content/public/browser/download_manager_delegate.h

Index: content/public/browser/download_manager_delegate.h
diff --git a/content/public/browser/download_manager_delegate.h b/content/public/browser/download_manager_delegate.h
index 6cc557c6de7a88e5cdbb41f3d5fa28243a551653..5d29627017956ae6a20f7b78730805021c8d93ac 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -9,6 +9,7 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/file_path.h"
+#include "base/logging.h"
 #include "base/time.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/save_page_type.h"
@@ -19,13 +20,20 @@ class DownloadId;
 class DownloadItem;
 class WebContents;
 
-typedef base::Callback<void(const FilePath&, content::SavePageType)>
+// Called by SavePackage when it creates a DownloadItem.
+typedef base::Callback<void(DownloadItem*)>
+    SaveFileDownloadCreatedCallback;

[Randy Smith (Not in Mondays), 2012/04/27 18:24:04]

So I had a heck of a time figuring out how this thing was used ("A callback
taking a callback passed through a callback?  We must go deeper." :-}) so I'd
rather not do it if there's some other way to arrange it.   If I'm reading the
code correctly (which I am *not* confident of, which is the basic problem :-})
it looks as if this callback exists:
* On Non-Cros: to disable safe browsing on this particular download.
* On Cros: To set the display name, and set it to temporary.

Why does this need to happen in a callback created by the file picker?  The file
picker should pick the file (and that's pretty much it).  Can we just put these
in a conditional, maybe ifdef'd, in SavePackage where this type of logic more
arguably belongs?  It'll save future maintainers (including possibly yourself)
headache and blood pressure med costs.

[benjhayden, 2012/04/28 19:09:03]

Actually, my bp is on the low end. My doctor suggested eating *more*
cholesterol, but maintaining even more bad code would be just as good.

SavePackage is in content, GDataDownloadObserver is in chrome.
How does adding a new method
DownloadManagerDelegate::SavePackageDownloadItemCreated(DownloadItem*) look?

+
+// Called by SavePackageFilePicker when the user picks a filename.
+typedef base::Callback<void(const FilePath&,
+                            content::SavePageType,
+                            const SaveFileDownloadCreatedCallback&)>
     SaveFilePathPickedCallback;
 
 // Browser's download manager: manages all downloads and destination view.
 class CONTENT_EXPORT DownloadManagerDelegate {
  public:
-  virtual ~DownloadManagerDelegate() {}
+  virtual ~DownloadManagerDelegate();
 
   // Lets the delegate know that the download manager is shutting down.
   virtual void Shutdown() {}
@@ -109,14 +117,13 @@ class CONTENT_EXPORT DownloadManagerDelegate {
                           FilePath* download_save_dir) {}
 
   // Asks the user for the path to save a page. The delegate calls the callback
-  // to give the answer, except on ChromeOS, where the saving is done by
-  // SavePackageFilePickerChromeOS. TODO(achuith): Move ChromeOS save
-  // functionality to SavePackage.
+  // to give the answer.
   virtual void ChooseSavePath(WebContents* web_contents,
                               const FilePath& suggested_path,
                               const FilePath::StringType& default_extension,
                               bool can_save_as_complete,
-                              SaveFilePathPickedCallback callback) {}
+                              const SaveFilePathPickedCallback& callback) {
+  }
 };
 
 }  // namespace content