Save Page As MHTML

chrome/browser/download/save_package_file_picker_chromeos.cc

Index: chrome/browser/download/save_package_file_picker_chromeos.cc
diff --git a/chrome/browser/download/save_package_file_picker_chromeos.cc b/chrome/browser/download/save_package_file_picker_chromeos.cc
index 5a5c2130c1154f430dbb79eeb86ad8023d859bb9..db2045ad0130d44477c5668ba85719a9e5708f65 100644
--- a/chrome/browser/download/save_package_file_picker_chromeos.cc
+++ b/chrome/browser/download/save_package_file_picker_chromeos.cc
@@ -13,16 +13,37 @@
 #include "chrome/browser/chromeos/gdata/gdata_system_service.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/platform_util.h"
-#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/download_item.h"
 #include "content/public/browser/web_contents.h"
 
-using content::BrowserThread;
+namespace {
+
+void OnSavePackageDownloadCreated(

[achuithb, 2012/04/24 22:06:09]

Could you add a function comment explaining that what this is a callback for and
why it's necessary?

[benjhayden, 2012/04/26 15:05:57]

Done.

+    const FilePath& true_path,

[achuithb, 2012/04/24 22:06:09]

Could we call this gdata_path instead?

[benjhayden, 2012/04/26 15:05:57]

Done.

+    content::DownloadItem* download) {
+  gdata::GDataDownloadObserver::SetGDataPath(download, true_path);
+  download->SetDisplayName(true_path.BaseName());
+  download->SetIsTemporary(true);
+}
+
+void ContinueSettingUpGDataDownload(

[achuithb, 2012/04/24 22:06:09]

Function comment please.

[benjhayden, 2012/04/26 15:05:57]

Done.

+    content::SaveFilePathPickedCallback callback,
+    FilePath* gdata_tmp_download_path,
+    const FilePath& true_path) {
+  callback.Run(*gdata_tmp_download_path, content::SAVE_PAGE_TYPE_AS_MHTML,
+               base::Bind(&OnSavePackageDownloadCreated, true_path));
+}
+
+}  // anonymous namespace
 
 SavePackageFilePickerChromeOS::SavePackageFilePickerChromeOS(
     content::WebContents* web_contents,
-    const FilePath& suggested_path)
-    : content::WebContentsObserver(web_contents) {
+    const FilePath& suggested_path,
+    content::SaveFilePathPickedCallback callback)
+    : content::WebContentsObserver(web_contents),
+      callback_(callback) {
   select_file_dialog_ = SelectFileDialog::Create(this);
   select_file_dialog_->SelectFile(SelectFileDialog::SELECT_SAVEAS_FILE,
                                   string16(),
@@ -39,91 +60,43 @@ SavePackageFilePickerChromeOS::SavePackageFilePickerChromeOS(
 SavePackageFilePickerChromeOS::~SavePackageFilePickerChromeOS() {
 }
 
-void SavePackageFilePickerChromeOS::FileSelected(const FilePath& selected_path,
-                                                 int index,
-                                                 void* params) {
+void SavePackageFilePickerChromeOS::FileSelected(
+    const FilePath& selected_path_const,
+    int index,
+    void* params) {
   if (!web_contents()) {
     delete this;
     return;
   }
-
-  FilePath path = selected_path;
-  file_util::NormalizeFileNameEncoding(&path);
-
-  gdata::GDataFileSystem* gdata_filesystem = GetGDataFileSystem();
-  if (gdata_filesystem && gdata::util::IsUnderGDataMountPoint(path)) {
-    FilePath gdata_tmp_download_dir =
-        gdata_filesystem->GetCacheDirectoryPath(
-            gdata::GDataRootDirectory::CACHE_TYPE_TMP_DOWNLOADS);
-
-    selected_path_ = path;
-    FilePath* gdata_tmp_download_path = new FilePath();
-    BrowserThread::GetBlockingPool()->PostTaskAndReply(FROM_HERE,
+  FilePath selected_path = selected_path_const;
+  file_util::NormalizeFileNameEncoding(&selected_path);
+  gdata::GDataSystemService* system_service =
+      gdata::GDataSystemServiceFactory::GetForProfile(
+          ProfileManager::GetDefaultProfile());
+  if (system_service && gdata::util::IsUnderGDataMountPoint(selected_path)) {
+    // GetGDataTempDownloadPath ->

[achuithb, 2012/04/24 22:06:09]

This is a bit cryptic. A line explaining that this is the sequence of events,
etc.

[benjhayden, 2012/04/26 15:05:57]

Done.

+    //   ContinueSettingUpGDataDownload ->
+    //     SavePackage::OnPathPicked ->
+    //       download_created_callback ->
+    //         OnSavePackageDownloadCreated
+    const FilePath gdata_tmp_download_dir =
+        system_service->file_system()->GetGDataTempDownloadFolderPath();
+    FilePath* gdata_tmp_download_path(new FilePath());
+    content::BrowserThread::GetBlockingPool()->PostTaskAndReply(FROM_HERE,
         base::Bind(&gdata::GDataDownloadObserver::GetGDataTempDownloadPath,
-                   gdata_tmp_download_dir,
-                   gdata_tmp_download_path),
-        base::Bind(&SavePackageFilePickerChromeOS::GenerateMHTML,
-                   base::Unretained(this),
-                   base::Owned(gdata_tmp_download_path)));
+                    gdata_tmp_download_dir,
+                    gdata_tmp_download_path),
+        base::Bind(&ContinueSettingUpGDataDownload,
+                    callback_,
+                    base::Owned(gdata_tmp_download_path),
+                    selected_path));
   } else {
-    DVLOG(1) << "SavePackageFilePickerChromeOS non-gdata file";
-    GenerateMHTML(&path);
+    callback_.Run(selected_path, content::SAVE_PAGE_TYPE_AS_MHTML,
+                  content::SaveFileDownloadCreatedCallback());
   }
-}
-
-void SavePackageFilePickerChromeOS::FileSelectionCanceled(void* params) {
   delete this;
 }
 
-void SavePackageFilePickerChromeOS::GenerateMHTML(const FilePath* mhtml_path) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!web_contents()) {
-    delete this;
-    return;
-  }
-
-  DVLOG(1) << "GenerateMHTML " << mhtml_path->value();
-  web_contents()->GenerateMHTML(*mhtml_path,
-      base::Bind(&SavePackageFilePickerChromeOS::OnMHTMLGenerated,
-                 base::Unretained(this)));
-}
-
-void SavePackageFilePickerChromeOS::OnMHTMLGenerated(const FilePath& src_path,
-                                                     int64 file_size) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!web_contents()) {
-    delete this;
-    return;
-  }
-
-  gdata::GDataFileSystem* gdata_filesystem = GetGDataFileSystem();
-  if (gdata_filesystem && !selected_path_.empty()) {
-    DVLOG(1) << "TransferFile from " << src_path.value()
-             << " to " << selected_path_.value();
-    gdata_filesystem->TransferFile(src_path,
-        gdata::util::ExtractGDataPath(selected_path_),
-        base::Bind(&SavePackageFilePickerChromeOS::OnTransferFile,
-                   base::Unretained(this)));
-  } else {
-    delete this;
-  }
-}
-
-void SavePackageFilePickerChromeOS::OnTransferFile(
-    base::PlatformFileError error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(error, base::PLATFORM_FILE_OK);
+void SavePackageFilePickerChromeOS::FileSelectionCanceled(void* params) {
   delete this;
 }
-
-gdata::GDataFileSystem*
-SavePackageFilePickerChromeOS::GetGDataFileSystem() {
-  DCHECK(web_contents());
-  Profile* profile = Profile::FromBrowserContext(
-      web_contents()->GetBrowserContext());
-  DCHECK(profile);
-  gdata::GDataSystemService* system_service =
-      gdata::GDataSystemServiceFactory::GetForProfile(profile);
-  // system_service is NULL in incognito.
-  return system_service ? system_service->file_system() : NULL;
-}