[Offline Pages] Adding util method for unique filename generation.

Adding a utility method for generating unique filenames for offline page
based on title and url of the page.

(Re-uploading a change by Romax with fixed merge conflicts)

Bug: 757073
Change-Id: I03e8e5cc03a640de2abee8e6e903b6bdce53af24

TBR: dimich@chromium.org, dtrainor@chromium.org
Change-Id: I03e8e5cc03a640de2abee8e6e903b6bdce53af24
Reviewed-on: https://chromium-review.googlesource.com/911837Reviewed-by: Peter Williamson <petewil@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535898}

components/filename_generation/filename_generation.cc

@@ -51,6 +51,7 @@ const base::FilePath::CharType* ExtensionForMimeType(
       {"application/xhtml+xml", FILE_PATH_LITERAL("xhtml")},
       {"text/plain", FILE_PATH_LITERAL("txt")},
       {"text/css", FILE_PATH_LITERAL("css")},
+      {"multipart/related", FILE_PATH_LITERAL("mhtml")},
   };
   for (const auto& extension : kExtensions) {
     if (contents_mime_type == extension.mime_type)
@@ -91,6 +92,16 @@ base::FilePath EnsureMimeExtension(const base::FilePath& name,
     return base::FilePath(name.value() + FILE_PATH_LITERAL(".") +
                           suggested_extension);
   }
+
+  // Special treatment for MHTML: we would always want to add ".mhtml" as the
+  // extension even if there's another recognized mime_type based on |ext|.
+  // For example: the name is "page.html", we would like to have
+  // "page.html.mhtml" instead of "page.html".
+  if (contents_mime_type == "multipart/related" &&
+      mime_type != "multipart/related") {
+    return base::FilePath(name.value() + FILE_PATH_LITERAL(".mhtml"));
+  }
+
   return name;
 }
 

components/filename_generation/filename_generation_unittest.cc

@@ -102,6 +102,10 @@ TEST(FilenameGenerationTest, MAYBE_TestEnsureMimeExtension) {
     {FPL("filename"), FPL("filename.txt"), "text/plain"},
     {FPL("filename.css"), FPL("filename.css"), "text/css"},
     {FPL("filename"), FPL("filename.css"), "text/css"},
+    {FPL("filename.mhtml"), FPL("filename.mhtml"), "multipart/related"},
+    {FPL("filename.html"), FPL("filename.html.mhtml"), "multipart/related"},
+    {FPL("filename.txt"), FPL("filename.txt.mhtml"), "multipart/related"},
+    {FPL("filename"), FPL("filename.mhtml"), "multipart/related"},
     {FPL("filename.abc"), FPL("filename.abc"), "unknown/unknown"},
     {FPL("filename"), FPL("filename"), "unknown/unknown"},
   };
@@ -116,19 +120,19 @@ TEST(FilenameGenerationTest, MAYBE_TestEnsureMimeExtension) {
   }
 }
 
-// Test that the suggested names generated by SavePackage are reasonable:
+// Test that the suggested names generated are reasonable:
 // If the name is a URL, retrieve only the path component since the path name
 // generation code will turn the entire URL into the file name leading to bad
 // extension names. For example, a page with no title and a URL:
 // http://www.foo.com/a/path/name.txt will turn into file:
 // "http www.foo.com a path name.txt", when we want to save it as "name.txt".
 
-static const struct SuggestedSaveNameTestCase {
+static const struct GenerateFilenameTestCase {
   const char* page_url;
   const base::string16 page_title;
   const base::FilePath::CharType* expected_name;
   bool ensure_html_extension;
-} kSuggestedSaveNames[] = {
+} kGenerateFilenameCases[] = {
     // Title overrides the URL.
     {"http://foo.com", base::ASCIIToUTF16("A page title"),
      FPL("A page title") FPL_HTML_EXTENSION, true},
@@ -157,19 +161,20 @@ static const struct SuggestedSaveNameTestCase {
     {"http://foo.com", base::ASCIIToUTF16("http://www.foo.com/path/title.txt"),
      FPL("http___www.foo.com_path_title.txt"), false},
 };
+
 // Crashing on Windows, see http://crbug.com/79365
 #if defined(OS_WIN)
-#define MAYBE_TestSuggestedSaveNames DISABLED_TestSuggestedSaveNames
+#define MAYBE_TestGenerateFilename DISABLED_TestGenerateFilename
 #else
-#define MAYBE_TestSuggestedSaveNames TestSuggestedSaveNames
+#define MAYBE_TestGenerateFilename TestGenerateFilename
 #endif
-TEST(FilenameGenerationTest, MAYBE_TestSuggestedSaveNames) {
-  for (size_t i = 0; i < arraysize(kSuggestedSaveNames); ++i) {
+TEST(FilenameGenerationTest, MAYBE_TestGenerateFilename) {
+  for (size_t i = 0; i < arraysize(kGenerateFilenameCases); ++i) {
     base::FilePath save_name = GenerateFilename(
-        kSuggestedSaveNames[i].page_title,
-        GURL(kSuggestedSaveNames[i].page_url),
-        kSuggestedSaveNames[i].ensure_html_extension, std::string());
-    EXPECT_EQ(kSuggestedSaveNames[i].expected_name, save_name.value())
+        kGenerateFilenameCases[i].page_title,
+        GURL(kGenerateFilenameCases[i].page_url),
+        kGenerateFilenameCases[i].ensure_html_extension, std::string());
+    EXPECT_EQ(kGenerateFilenameCases[i].expected_name, save_name.value())
         << "Test case " << i;
   }
 }

components/offline_pages/core/BUILD.gn

@@ -83,6 +83,7 @@ static_library("core") {
   deps = [
     ":switches",
     "//base",
+    "//components/filename_generation",
     "//components/keyed_service/core",
     "//crypto",
     "//net",

components/offline_pages/core/DEPS

 include_rules = [
+  "+components/filename_generation",
   "+components/keyed_service",
   "+components/offline_items_collection",
   "+components/ukm",

components/offline_pages/core/model/offline_page_model_utils.cc

@@ -6,7 +6,14 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "build/build_config.h"
+#include "components/filename_generation/filename_generation.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/offline_page_item.h"
 
@@ -51,6 +58,46 @@ std::string AddHistogramSuffix(const std::string& name_space,
   return adjusted_histogram_name;
 }
 
+base::FilePath GenerateUniqueFilenameForOfflinePage(
+    const base::string16& title,
+    const GURL& url,
+    const base::FilePath& target_dir) {
+  std::string kMHTMLMimeType = "multipart/related";
+
+  // Get the suggested file name based on title and url.
+  base::FilePath suggested_path =
+      target_dir.Append(filename_generation::GenerateFilename(
+          title, url, false /* can_save_as_complete */, kMHTMLMimeType));
+
+  // Find a unique name based on |suggested_path|.
+  int uniquifier =
+      base::GetUniquePathNumber(suggested_path, base::FilePath::StringType());
+  base::FilePath::StringType suffix;
+  if (uniquifier > 0)
+#if defined(OS_WIN)
+    suffix = base::StringPrintf(L" (%d)", uniquifier);
+#else   // defined(OS_WIN)
+    suffix = base::StringPrintf(" (%d)", uniquifier);
+#endif  // defined(OS_WIN)
+
+  // Truncation.
+  int max_path_component_length =
+      base::GetMaximumPathComponentLength(target_dir);
+  if (max_path_component_length != -1) {
+    int limit = max_path_component_length -
+                suggested_path.Extension().length() - suffix.length();
+    if (limit <= 0 ||
+        !filename_generation::TruncateFilename(&suggested_path, limit))
+      return base::FilePath();
+  }
+
+  // Adding uniquifier suffix if needed.
+  if (uniquifier > 0)
+    suggested_path = suggested_path.InsertBeforeExtension(suffix);
+
+  return suggested_path;
+}
+
 }  // namespace model_utils
 
 }  // namespace offline_pages

components/offline_pages/core/model/offline_page_model_utils.h

@@ -7,6 +7,14 @@
 
 #include <string>
 
+#include "base/strings/string16.h"
+
+class GURL;
+
+namespace base {
+class FilePath;
+}  // namespace base
+
 namespace offline_pages {
 
 enum class OfflinePagesNamespaceEnumeration;
@@ -20,6 +28,11 @@ OfflinePagesNamespaceEnumeration ToNamespaceEnum(const std::string& name_space);
 std::string AddHistogramSuffix(const std::string& name_space,
                                const char* histogram_name);
 
+base::FilePath GenerateUniqueFilenameForOfflinePage(
+    const base::string16& title,
+    const GURL& url,
+    const base::FilePath& target_dir);
+
 }  // namespace model_utils
 
 }  // namespace offline_pages

components/offline_pages/core/model/offline_page_model_utils_unittest.cc

@@ -4,8 +4,15 @@
 
 #include "components/offline_pages/core/model/offline_page_model_utils.h"
 
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace offline_pages {
 
@@ -30,4 +37,62 @@ TEST(OfflinePageModelUtilsTest, ToNamespaceEnum) {
             OfflinePagesNamespaceEnumeration::BROWSER_ACTIONS);
 }
 
+static const struct GenerateUniqueFilenameTestCase {
+  const base::string16 page_title;
+  const GURL page_url;
+  const base::FilePath::CharType* expected_basename;
+} kGenerateUniqueFilenameCases[] = {
+    {base::ASCIIToUTF16("wikipedia.org-Main_Page"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org-Main_Page.mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org-Main_Page"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org-Main_Page (1).mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org-Main_Page"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org-Main_Page (2).mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org-Main_Page.mhtml"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org-Main_Page (3).mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org-Main_Page"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org-Main_Page (4).mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org.mhtml")},
+    {base::ASCIIToUTF16("wikipedia.org"),
+     GURL("http://www.wikipedia.org/Main_Page"),
+     FILE_PATH_LITERAL("wikipedia.org (1).mhtml")},
+    {base::UTF8ToUTF16("bücher.com"), GURL("http://xn--bcher-kva.com"),
+     FILE_PATH_LITERAL("bücher.com.mhtml")},
+    {base::ASCIIToUTF16("http://foo.com/path/title.html"),
+     GURL("http://foo.com"),
+     FILE_PATH_LITERAL("http___foo.com_path_title.html.mhtml")},
+    {base::ASCIIToUTF16("foo.com/foo-%40.html"),
+     GURL("http://foo.com/foo-%40.html"),
+     FILE_PATH_LITERAL("foo-@.html.mhtml")},
+    {base::ASCIIToUTF16("Viva%40%40%40-TestTitle"),
+     GURL("http://foo.com/%40.html"),
+     FILE_PATH_LITERAL("Viva%40%40%40-TestTitle.mhtml")},
+};
+
+// Crashing on Windows, see http://crbug.com/79365
+#if defined(OS_WIN)
+#define MAYBE_TestGenerateUniqueFilename DISABLED_TestGenerateUniqueFilename
+#else
+#define MAYBE_TestGenerateUniqueFilename TestGenerateUniqueFilename
+#endif
+TEST(OfflinePageModelUtilsTest, MAYBE_TestGenerateUniqueFilename) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  for (const auto& test_case : kGenerateUniqueFilenameCases) {
+    base::FilePath path = model_utils::GenerateUniqueFilenameForOfflinePage(
+        test_case.page_title, test_case.page_url, temp_dir.GetPath());
+    // Writing a dummy file so the uniquifier can increase.
+    base::WriteFile(path, nullptr, 0);
+    EXPECT_EQ(path.BaseName().value(), test_case.expected_basename);
+  }
+}
+
 }  // namespace offline_pages