Save Page As MHTML

chrome/browser/download/save_package_file_picker.cc

Index: chrome/browser/download/save_package_file_picker.cc
diff --git a/chrome/browser/download/save_package_file_picker.cc b/chrome/browser/download/save_package_file_picker.cc
index e56c1bbf05a4e91ced5a14f18d55f5a77c3bdbb8..923a786b32e6727b225d50a878f915552ebccb1d 100644
--- a/chrome/browser/download/save_package_file_picker.cc
+++ b/chrome/browser/download/save_package_file_picker.cc
@@ -1,15 +1,20 @@
 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
+#include "chrome/browser/download/chrome_download_manager_delegate.h"
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/save_package_file_picker.h"
 
+#include "base/command_line.h"
+#include "base/metrics/histogram.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/prefs/pref_member.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/render_process_host.h"
@@ -54,26 +59,44 @@ const int kIndexToIDS[] = {
   0, IDS_SAVE_PAGE_DESC_HTML_ONLY, IDS_SAVE_PAGE_DESC_COMPLETE,
 };
 
+void OnSavePackageDownloadCreated(
+    content::DownloadItem* download) {
+  ChromeDownloadManagerDelegate::DisableSafeBrowsing(download);
+}
+
+}  // anonymous namespace
+
+bool SavePackageFilePicker::ShouldSaveAsMHTML() const {
+  return can_save_as_complete_ &&

[cbentzel, 2012/04/26 20:35:16]

Nit: could you just pass this in as an argument rather than adding state to the
FilePicker object?

[benjhayden, 2012/04/26 20:49:44]

This is used in the FileSelected() method. I could pass it along via |void*
params|, but this seems cleaner.

[achuithb, 2012/04/26 21:01:45]

Please don't use the void* params. I think they are currently unused, and we
should get rid of them altogether.

+         CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kSavePageAsMHTML);
 }
 
 SavePackageFilePicker::SavePackageFilePicker(
     content::WebContents* web_contents,
-    const FilePath& suggested_path,
-    const FilePath::StringType& default_extension,
+    const FilePath& suggested_path_const,
+    const FilePath::StringType& default_extension_const,
     bool can_save_as_complete,
     DownloadPrefs* download_prefs,
-    content::SaveFilePathPickedCallback callback)
+    const content::SaveFilePathPickedCallback& callback)
     : render_process_id_(web_contents->GetRenderProcessHost()->GetID()),
+      can_save_as_complete_(can_save_as_complete),
       callback_(callback) {
+  FilePath suggested_path = suggested_path_const;
+  FilePath::StringType default_extension = default_extension_const;
   int file_type_index = SavePackageTypeToIndex(
       static_cast<SavePageType>(download_prefs->save_file_type()));
   DCHECK_NE(-1, file_type_index);
 
   SelectFileDialog::FileTypeInfo file_type_info;
 
-  // If the contents can not be saved as complete-HTML, do not show the
-  // file filters.
-  if (can_save_as_complete) {
+  // TODO(benjhayden): Merge the first branch with the second when all of the
+  // platform-specific file selection dialog implementations fully support
+  // switching save-as file formats, and remove the flag/switch.
+  if (ShouldSaveAsMHTML()) {
+    default_extension = FILE_PATH_LITERAL("mhtml");
+    suggested_path = suggested_path.ReplaceExtension(default_extension);
+  } else if (can_save_as_complete) {
     bool add_extra_extension = false;
     FilePath::StringType extra_extension;
     if (!suggested_path.Extension().empty() &&
@@ -83,40 +106,50 @@ SavePackageFilePicker::SavePackageFilePicker(
       extra_extension = suggested_path.Extension().substr(1);
     }
 
-    file_type_info.extensions.resize(2);
+    static const size_t kNumberExtensions = arraysize(kIndexToIDS) - 1;
+    file_type_info.extensions.resize(kNumberExtensions);
+    file_type_info.extension_description_overrides.resize(kNumberExtensions);
+
+    // Indices into kIndexToIDS are 1-based whereas indices into
+    // file_type_info.extensions are 0-based. Hence the '-1's.
+    // If you switch these resize()/direct-assignment patterns to push_back(),
+    // then you risk breaking FileSelected()'s use of |index|.
+
+    file_type_info.extension_description_overrides[
+      kSelectFileHtmlOnlyIndex - 1] = l10n_util::GetStringUTF16(kIndexToIDS[
+          kSelectFileHtmlOnlyIndex]);
     file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
         FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
         FILE_PATH_LITERAL("html"));
-
     if (add_extra_extension) {
       file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
           extra_extension);
     }
 
-    file_type_info.extension_description_overrides.push_back(
-        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileCompleteIndex - 1]));
+    file_type_info.extension_description_overrides[
+      kSelectFileCompleteIndex - 1] = l10n_util::GetStringUTF16(kIndexToIDS[
+          kSelectFileCompleteIndex]);
     file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
         FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
         FILE_PATH_LITERAL("html"));
-
     if (add_extra_extension) {
       file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
           extra_extension);
     }
 
-    file_type_info.extension_description_overrides.push_back(
-        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileCompleteIndex]));
     file_type_info.include_all_files = false;
   } else {
+    // The contents can not be saved as complete-HTML, so do not show the file
+    // filters.
     file_type_info.extensions.resize(1);
-    file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
+    file_type_info.extensions[0].push_back(
         suggested_path.Extension());
 
-    if (!file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1][0].empty()) {
+    if (!file_type_info.extensions[0][0].empty()) {
       // Drop the .
-      file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1][0].erase(0, 1);
+      file_type_info.extensions[0][0].erase(0, 1);
     }
 
     file_type_info.include_all_files = true;
@@ -137,7 +170,8 @@ SavePackageFilePicker::SavePackageFilePicker(
                                     NULL);
   } else {
     // Just use 'suggested_path' instead of opening the dialog prompt.
-    callback.Run(suggested_path, kIndexToSaveType[file_type_index]);
+    // Go through FileSelected() for consistency.
+    FileSelected(suggested_path, file_type_index, NULL);
   }
 }
 
@@ -150,20 +184,27 @@ void SavePackageFilePicker::SetShouldPromptUser(bool should_prompt) {
 
 void SavePackageFilePicker::FileSelected(const FilePath& path,
                                          int index,
-                                         void* params) {
-  // The option index is not zero-based.
-  DCHECK(index >= kSelectFileHtmlOnlyIndex &&
-         index <= kSelectFileCompleteIndex);
-
+                                         void* unused_params) {
   RenderProcessHost* process = RenderProcessHost::FromID(render_process_id_);
   if (process) {
-    SavePageType save_type = kIndexToSaveType[index];
-    Profile* profile =
-        Profile::FromBrowserContext(process->GetBrowserContext());
-    PrefService* prefs = profile->GetPrefs();
-    if (select_file_dialog_ &&
-        select_file_dialog_->HasMultipleFileTypeChoices())
-      prefs->SetInteger(prefs::kSaveFileType, save_type);
+    SavePageType save_type = content::SAVE_PAGE_TYPE_UNKNOWN;
+    PrefService* prefs = Profile::FromBrowserContext(
+        process->GetBrowserContext())->GetPrefs();
+    if (ShouldSaveAsMHTML()) {
+      save_type = content::SAVE_PAGE_TYPE_AS_MHTML;
+    } else {
+      // The option index is not zero-based.
+      DCHECK(index >= kSelectFileHtmlOnlyIndex &&
+             index <= kSelectFileCompleteIndex);
+      save_type = kIndexToSaveType[index];
+      if (select_file_dialog_ &&
+          select_file_dialog_->HasMultipleFileTypeChoices())
+        prefs->SetInteger(prefs::kSaveFileType, save_type);
+    }
+
+    UMA_HISTOGRAM_ENUMERATION("Download.SavePageType",
+                              save_type,
+                              content::SAVE_PAGE_TYPE_MAX);
 
     StringPrefMember save_file_path;
     save_file_path.Init(prefs::kSaveFileDefaultDirectory, prefs, NULL);
@@ -175,16 +216,15 @@ void SavePackageFilePicker::FileSelected(const FilePath& path,
     // If user change the default saving directory, we will remember it just
     // like IE and FireFox.
     if (!process->GetBrowserContext()->IsOffTheRecord() &&
-        save_file_path.GetValue() != path_string) {
+        save_file_path.GetValue() != path_string)
       save_file_path.SetValue(path_string);
-    }
 
-    callback_.Run(path, save_type);
+    callback_.Run(path, save_type, base::Bind(&OnSavePackageDownloadCreated));
   }
 
   delete this;
 }
 
-void SavePackageFilePicker::FileSelectionCanceled(void* params) {
+void SavePackageFilePicker::FileSelectionCanceled(void* unused_params) {
   delete this;
 }