Backed out changeset 3cfea5c3c067 (bug 1612403) for failing test_bug449141.html on a CLOSED TREE

dom/base/nsContentAreaDragDrop.cpp

@@ -181,7 +181,8 @@ nsresult CheckAndGetExtensionForMime(const nsCString& aExtension,
                                             getter_AddRefs(mimeInfo));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  mimeInfo->GetPrimaryExtension(*aPrimaryExtension);
+  rv = mimeInfo->GetPrimaryExtension(*aPrimaryExtension);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   if (aExtension.IsEmpty()) {
     *aIsValidExtension = false;
@@ -278,7 +279,7 @@ nsContentAreaDragDropDataProvider::GetFlavorData(nsITransferable* aTransferable,
                                          &isValidExtension, &primaryExtension);
         NS_ENSURE_SUCCESS(rv, rv);
 
-        if (!isValidExtension && !primaryExtension.IsEmpty()) {
+        if (!isValidExtension) {
           // The filename extension is missing or incompatible
           // with the MIME type, replace it with the primary
           // extension.
@@ -462,13 +463,12 @@ nsresult DragDataProducer::GetImageData(imgIContainer* aImage,
         // Fix the file extension in the URL
         nsAutoCString primaryExtension;
         mimeInfo->GetPrimaryExtension(primaryExtension);
-        if (!primaryExtension.IsEmpty()) {
-          rv = NS_MutateURI(imgUrl)
-                   .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
-                                           primaryExtension, nullptr))
-                   .Finalize(imgUrl);
-          NS_ENSURE_SUCCESS(rv, rv);
-        }
+
+        rv = NS_MutateURI(imgUrl)
+                 .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
+                                         primaryExtension, nullptr))
+                 .Finalize(imgUrl);
+        NS_ENSURE_SUCCESS(rv, rv);
       }
     }
 #endif /* defined(XP_MACOSX) */

dom/base/nsCopySupport.cpp

@@ -639,13 +639,12 @@ static nsresult AppendImagePromise(nsITransferable* aTransferable,
     // Fix the file extension in the URL
     nsAutoCString primaryExtension;
     mimeInfo->GetPrimaryExtension(primaryExtension);
-    if (!primaryExtension.IsEmpty()) {
-      rv = NS_MutateURI(imgUri)
-               .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
-                                       primaryExtension, nullptr))
-               .Finalize(imgUrl);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
+
+    rv = NS_MutateURI(imgUri)
+             .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
+                                     primaryExtension, nullptr))
+             .Finalize(imgUrl);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   nsAutoCString fileName;

uriloader/exthandler/HandlerServiceParent.cpp

@@ -170,18 +170,13 @@ NS_IMETHODIMP ProxyMIMEInfo::SetFileExtensions(const nsACString& aExtensions) {
 /* boolean extensionExists (in AUTF8String aExtension); */
 NS_IMETHODIMP ProxyMIMEInfo::ExtensionExists(const nsACString& aExtension,
                                              bool* _retval) {
-  *_retval = mProxyHandlerInfo->Extensions().Contains(
-      aExtension, nsCaseInsensitiveCStringArrayComparator());
+  *_retval = mProxyHandlerInfo->Extensions().Contains(aExtension);
   return NS_OK;
 }
 
 /* void appendExtension (in AUTF8String aExtension); */
 NS_IMETHODIMP ProxyMIMEInfo::AppendExtension(const nsACString& aExtension) {
-  if (!aExtension.IsEmpty() &&
-      !mProxyHandlerInfo->Extensions().Contains(
-          aExtension, nsCaseInsensitiveCStringArrayComparator())) {
-    mProxyHandlerInfo->Extensions().AppendElement(aExtension);
-  }
+  mProxyHandlerInfo->Extensions().AppendElement(aExtension);
   return NS_OK;
 }
 
@@ -190,7 +185,6 @@ NS_IMETHODIMP ProxyMIMEInfo::GetPrimaryExtension(
     nsACString& aPrimaryExtension) {
   const auto& extensions = mProxyHandlerInfo->Extensions();
   if (extensions.IsEmpty()) {
-    aPrimaryExtension.Truncate();
     return NS_ERROR_FAILURE;
   }
   aPrimaryExtension = extensions[0];

uriloader/exthandler/android/nsMIMEInfoAndroid.cpp

@@ -238,16 +238,15 @@ nsMIMEInfoAndroid::GetFileExtensions(nsIUTF8StringEnumerator** aResult) {
 NS_IMETHODIMP
 nsMIMEInfoAndroid::SetFileExtensions(const nsACString& aExtensions) {
   mExtensions.Clear();
-  nsACString::const_iterator start, end;
-  aExtensions.BeginReading(start);
-  aExtensions.EndReading(end);
-  while (start != end) {
-    nsACString::const_iterator cursor = start;
-    mozilla::Unused << FindCharInReadable(',', cursor, end);
-    AddUniqueExtension(Substring(start, cursor));
-    // If a comma was found, skip it for the next search.
-    start = cursor != end ? ++cursor : cursor;
+  nsCString extList(aExtensions);
+
+  int32_t breakLocation = -1;
+  while ((breakLocation = extList.FindChar(',')) != -1) {
+    mExtensions.AppendElement(
+        Substring(extList.get(), extList.get() + breakLocation));
+    extList.Cut(0, breakLocation + 1);
   }
+  if (!extList.IsEmpty()) mExtensions.AppendElement(extList);
   return NS_OK;
 }
 
@@ -269,43 +268,38 @@ nsMIMEInfoAndroid::ExtensionExists(const nsACString& aExtension,
   return NS_OK;
 }
 
-void nsMIMEInfoAndroid::AddUniqueExtension(const nsACString& aExtension) {
-  if (!aExtension.IsEmpty() &&
-      !mExtensions.Contains(aExtension,
-                            nsCaseInsensitiveCStringArrayComparator())) {
-    mExtensions.AppendElement(aExtension);
-  }
-}
-
 NS_IMETHODIMP
 nsMIMEInfoAndroid::AppendExtension(const nsACString& aExtension) {
-  MOZ_ASSERT(!aExtension.IsEmpty(), "No extension");
-  AddUniqueExtension(aExtension);
+  mExtensions.AppendElement(aExtension);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoAndroid::GetPrimaryExtension(nsACString& aPrimaryExtension) {
-  if (!mExtensions.Length()) {
-    aPrimaryExtension.Truncate();
-    return NS_ERROR_NOT_INITIALIZED;
-  }
+  if (!mExtensions.Length()) return NS_ERROR_NOT_INITIALIZED;
+
   aPrimaryExtension = mExtensions[0];
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoAndroid::SetPrimaryExtension(const nsACString& aExtension) {
-  if (MOZ_UNLIKELY(aExtension.IsEmpty())) {
-    // Don't assert since Java may return an empty extension for unknown types.
-    return NS_ERROR_INVALID_ARG;
+  uint32_t extCount = mExtensions.Length();
+  uint8_t i;
+  bool found = false;
+  for (i = 0; i < extCount; i++) {
+    const nsCString& ext = mExtensions[i];
+    if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) {
+      found = true;
+      break;
+    }
   }
-  int32_t i = mExtensions.IndexOf(aExtension, 0,
-                                  nsCaseInsensitiveCStringArrayComparator());
-  if (i != -1) {
+  if (found) {
     mExtensions.RemoveElementAt(i);
   }
+
   mExtensions.InsertElementAt(0, aExtension);
+
   return NS_OK;
 }
 

uriloader/exthandler/android/nsMIMEInfoAndroid.h

@@ -30,11 +30,6 @@ class nsMIMEInfoAndroid final : public nsIMIMEInfo {
  private:
   ~nsMIMEInfoAndroid() {}
 
-  /**
-   * Internal helper to avoid adding duplicates.
-   */
-  void AddUniqueExtension(const nsACString& aExtension);
-
   virtual MOZ_MUST_USE nsresult LaunchDefaultWithFile(nsIFile* aFile);
   virtual MOZ_MUST_USE nsresult LoadUriInternal(nsIURI* aURI);
   nsCOMPtr<nsIMutableArray> mHandlerApps;

uriloader/exthandler/nsMIMEInfoImpl.cpp

@@ -95,49 +95,56 @@ nsMIMEInfoBase::GetFileExtensions(nsIUTF8StringEnumerator** aResult) {
 
 NS_IMETHODIMP
 nsMIMEInfoBase::ExtensionExists(const nsACString& aExtension, bool* _retval) {
-  MOZ_ASSERT(!aExtension.IsEmpty(), "no extension");
-  *_retval = mExtensions.Contains(aExtension,
-                                  nsCaseInsensitiveCStringArrayComparator());
+  NS_ASSERTION(!aExtension.IsEmpty(), "no extension");
+  bool found = false;
+  uint32_t extCount = mExtensions.Length();
+  if (extCount < 1) return NS_OK;
+
+  for (uint8_t i = 0; i < extCount; i++) {
+    const nsCString& ext = mExtensions[i];
+    if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) {
+      found = true;
+      break;
+    }
+  }
+
+  *_retval = found;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoBase::GetPrimaryExtension(nsACString& _retval) {
-  if (!mExtensions.Length()) {
-    _retval.Truncate();
-    return NS_ERROR_NOT_INITIALIZED;
-  }
+  if (!mExtensions.Length()) return NS_ERROR_NOT_INITIALIZED;
+
   _retval = mExtensions[0];
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoBase::SetPrimaryExtension(const nsACString& aExtension) {
-  if (MOZ_UNLIKELY(aExtension.IsEmpty())) {
-    MOZ_ASSERT(false, "No extension");
-    return NS_ERROR_INVALID_ARG;
+  NS_ASSERTION(!aExtension.IsEmpty(), "no extension");
+  uint32_t extCount = mExtensions.Length();
+  uint8_t i;
+  bool found = false;
+  for (i = 0; i < extCount; i++) {
+    const nsCString& ext = mExtensions[i];
+    if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) {
+      found = true;
+      break;
+    }
   }
-  int32_t i = mExtensions.IndexOf(aExtension, 0,
-                                  nsCaseInsensitiveCStringArrayComparator());
-  if (i != -1) {
+  if (found) {
     mExtensions.RemoveElementAt(i);
   }
-  mExtensions.InsertElementAt(0, aExtension);
-  return NS_OK;
-}
 
-void nsMIMEInfoBase::AddUniqueExtension(const nsACString& aExtension) {
-  if (!aExtension.IsEmpty() &&
-      !mExtensions.Contains(aExtension,
-                            nsCaseInsensitiveCStringArrayComparator())) {
-    mExtensions.AppendElement(aExtension);
-  }
+  mExtensions.InsertElementAt(0, aExtension);
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoBase::AppendExtension(const nsACString& aExtension) {
-  MOZ_ASSERT(!aExtension.IsEmpty(), "No extension");
-  AddUniqueExtension(aExtension);
+  mExtensions.AppendElement(aExtension);
   return NS_OK;
 }
 
@@ -185,16 +192,15 @@ nsMIMEInfoBase::Equals(nsIMIMEInfo* aMIMEInfo, bool* _retval) {
 NS_IMETHODIMP
 nsMIMEInfoBase::SetFileExtensions(const nsACString& aExtensions) {
   mExtensions.Clear();
-  nsACString::const_iterator start, end;
-  aExtensions.BeginReading(start);
-  aExtensions.EndReading(end);
-  while (start != end) {
-    nsACString::const_iterator cursor = start;
-    mozilla::Unused << FindCharInReadable(',', cursor, end);
-    AddUniqueExtension(Substring(start, cursor));
-    // If a comma was found, skip it for the next search.
-    start = cursor != end ? ++cursor : cursor;
+  nsCString extList(aExtensions);
+
+  int32_t breakLocation = -1;
+  while ((breakLocation = extList.FindChar(',')) != -1) {
+    mExtensions.AppendElement(
+        Substring(extList.get(), extList.get() + breakLocation));
+    extList.Cut(0, breakLocation + 1);
   }
+  if (!extList.IsEmpty()) mExtensions.AppendElement(extList);
   return NS_OK;
 }
 

uriloader/exthandler/nsMIMEInfoImpl.h

@@ -137,11 +137,6 @@ class nsMIMEInfoBase : public nsIMIMEInfo {
    */
   static nsresult GetLocalFileFromURI(nsIURI* aURI, nsIFile** aFile);
 
-  /**
-   * Internal helper to avoid adding duplicates.
-   */
-  void AddUniqueExtension(const nsACString& aExtension);
-
   // member variables
   nsTArray<nsCString>
       mExtensions;  ///< array of file extensions associated w/ this MIME obj

uriloader/exthandler/tests/unit/test_handlerService_store.js

@@ -53,7 +53,8 @@ function assertAllHandlerInfosMatchTestData() {
 
   // It's important that the MIME types we check here do not exist at the
   // operating system level, otherwise the list of handlers and file extensions
-  // will be merged. The current implementation avoids duplicate entries.
+  // will be merged. The current implementation adds each saved file extension
+  // even if one already exists in the system, resulting in duplicate entries.
 
   HandlerServiceTestUtils.assertHandlerInfoMatches(handlerInfos.shift(), {
     type: "example/type.handleinternally",
@@ -414,8 +415,8 @@ add_task(async function test_store_fileExtensions_lowercase() {
 });
 
 /**
- * Tests that appendExtension doesn't add duplicates, and that anyway duplicates
- * from possibleApplicationHandlers are removed when saving and reloading.
+ * Tests that duplicates added with "appendExtension" or present in
+ * "possibleApplicationHandlers" are removed when saving and reloading.
  */
 add_task(async function test_store_no_duplicates() {
   await deleteHandlerStore();
@@ -430,10 +431,6 @@ add_task(async function test_store_no_duplicates() {
   handlerInfo.appendExtension("extension_test2");
   handlerInfo.appendExtension("extension_test1");
   handlerInfo.appendExtension("EXTENSION_test1");
-  Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), [
-    "extension_test1",
-    "extension_test2",
-  ]);
   gHandlerService.store(handlerInfo);
 
   await unloadHandlerStore();
@@ -452,21 +449,6 @@ add_task(async function test_store_no_duplicates() {
   });
 });
 
-/**
- * Tests that setFileExtensions doesn't add duplicates.
- */
-add_task(async function test_setFileExtensions_no_duplicates() {
-  await deleteHandlerStore();
-
-  let handlerInfo = getKnownHandlerInfo("example/new");
-  handlerInfo.setFileExtensions("a,b,A,b,c,a");
-  let expected = ["a", "b", "c"];
-  Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), expected);
-  // Test empty extensions, also at begin and end.
-  handlerInfo.setFileExtensions(",a,,b,A,c,");
-  Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), expected);
-});
-
 /**
  * Tests that "store" deletes properties that have their default values from
  * the data store.