Check for a different mime type before adding an extension.

This fixes failing test 'Linux Chromium OS ASan LSan Tests', which
caused the previous CL to be reverted by
https://chromium-review.googlesource.com/c/chromium/src/+/1203353

The issue was due to using EXPECT_EQ instead of EXPECT_STREQ to compare
raw strings (which unfortunately passes with default args).

      then confirmed that EXPECT_STREQ fixes it.

Bug: 844252
Test: Reproduced the issue with gn args is_asan=true is_lsan=true and
Change-Id: I07720ba57f80b0a19c21d2ee3308809e6a8ce582
Reviewed-on: https://chromium-review.googlesource.com/1203754Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Alex Lau <alexlau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589106}

chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util.cc

@@ -217,6 +217,30 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType(
   return std::vector<base::FilePath::StringType>();
 }
 
+std::string StripMimeSubType(const std::string& mime_type) {
+  if (mime_type.empty())
+    return mime_type;
+  size_t index = mime_type.find_first_of('/', 0);
+  if (index == 0 || index == mime_type.size() - 1 ||
+      index == std::string::npos) {
+    // This looks malformed, return an empty string.
+    return std::string();
+  }
+  return mime_type.substr(0, index);
+}
+
+// This is based on net/base/mime_util.cc: net::FindMimeType.
+std::string FindArcMimeTypeFromExtension(const std::string& ext) {
+  for (const auto& mapping : kAndroidMimeTypeMappings) {
+    std::vector<base::StringPiece> extensions = base::SplitStringPiece(
+        mapping.extensions, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+    auto iter = std::find(extensions.begin(), extensions.end(), ext);
+    if (iter != extensions.end())
+      return mapping.mime_type;
+  }
+  return std::string();
+}
+
 // TODO(crbug.com/675868): Consolidate with the similar logic for Drive.
 base::FilePath::StringType GetFileNameForDocument(
     const mojom::DocumentPtr& document) {
@@ -243,10 +267,23 @@ base::FilePath::StringType GetFileNameForDocument(
     extension = extension.substr(1);  // Strip the leading dot.
   std::vector<base::FilePath::StringType> possible_extensions =
       GetExtensionsForArcMimeType(document->mime_type);
+
   if (!possible_extensions.empty() &&
       !base::ContainsValue(possible_extensions, extension)) {
-    filename =
-        base::FilePath(filename).AddExtension(possible_extensions[0]).value();
+    // Lookup the extension in the hardcoded map before appending an extension,
+    // as some extensions (eg. 3gp) are typed differently by Android. Only
+    // append the suggested extension if the lookup fails (i.e. no valid mime
+    // type returned), or the returned mime type is of a different category.
+    // TODO(crbug.com/878221): Fix discrepancy in MIME types and extensions
+    // between the hard coded map and the Android content provider.
+    std::string missed_possible_mime_type =
+        FindArcMimeTypeFromExtension(extension);
+    if (missed_possible_mime_type.empty() ||
+        StripMimeSubType(document->mime_type) !=
+            StripMimeSubType(missed_possible_mime_type)) {
+      filename =
+          base::FilePath(filename).AddExtension(possible_extensions[0]).value();
+    }
   }
 
   return filename;

chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util.h

@@ -75,6 +75,14 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType(
 base::FilePath::StringType GetFileNameForDocument(
     const mojom::DocumentPtr& document);
 
+// Returns the provided MIME type without the subtype component.
+std::string StripMimeSubType(const std::string& mime_type);
+
+// Finds the first matching mime type with |ext| as a valid extension from the
+// internal list of Android mime types. On success, the first matching MIME type
+// is returned. On failure, nullptr is returned.
+std::string FindArcMimeTypeFromExtension(const std::string& ext);
+
 }  // namespace arc
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_UTIL_H_

chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc

@@ -243,6 +243,49 @@ TEST(ArcDocumentsProviderUtilTest, GetFileNameForDocument) {
             GetFileNameForDocument(MakeDocument("kitten", "image/png")));
   EXPECT_EQ("kitten",
             GetFileNameForDocument(MakeDocument("kitten", "abc/xyz")));
+
+  // Check that files with a mime type different than expected appends a
+  // possible extension when a different mime type with a different category is
+  // found in the Android mime types map.
+  EXPECT_EQ("file.txt.3gp",
+            GetFileNameForDocument(MakeDocument("file.txt", "video/3gpp")));
+
+  // Check that files with a mime type different than expected don't append
+  // an extension when a different mime type with the same category is
+  // found in the Android mime types map.
+  EXPECT_EQ("file.3gp",
+            GetFileNameForDocument(MakeDocument("file.3gp", "video/mp4")));
+}
+
+TEST(ArcDocumentsProviderUtilTest, StripMimeSubType) {
+  // Check that the category type is returned for a valid mime type.
+  EXPECT_EQ("video", StripMimeSubType("video/mp4"));
+  // Check that an empty string is returned for malformed mime types.
+  EXPECT_EQ("", StripMimeSubType(""));
+  EXPECT_EQ("", StripMimeSubType("video/"));
+  EXPECT_EQ("", StripMimeSubType("/"));
+  EXPECT_EQ("", StripMimeSubType("/abc"));
+  EXPECT_EQ("", StripMimeSubType("/abc/xyz"));
+}
+
+TEST(ArcDocumentsProviderUtilTest, FindArcMimeTypeFromExtension) {
+  // Test that a lone possible extension returns the correct type.
+  EXPECT_EQ("application/msword", FindArcMimeTypeFromExtension("doc"));
+  // Test that the first extension in the comma delimited list of extensions
+  // returns the correct type.
+  EXPECT_EQ("video/3gpp", FindArcMimeTypeFromExtension("3gp"));
+  // Test that the second extension in the comma delimited list of extensions
+  // returns the correct type.
+  EXPECT_EQ("audio/mpeg", FindArcMimeTypeFromExtension("mpga"));
+  // Test that matching suffixes (m4a) return null without a full match.
+  EXPECT_EQ("", FindArcMimeTypeFromExtension("4a"));
+  // Test that matching prefixes (imy) return null without a full match.
+  EXPECT_EQ("", FindArcMimeTypeFromExtension("im"));
+  // Test that ambiguous prefixes (mp4, mpg) return null.
+  EXPECT_EQ("", FindArcMimeTypeFromExtension("mp"));
+  // Test that invalid mime types return null.
+  EXPECT_EQ("", FindArcMimeTypeFromExtension(""));
+  EXPECT_EQ("", FindArcMimeTypeFromExtension("invalid"));
 }
 
 }  // namespace