Bug 1469348 - Fix the problem of download file failed on Mac. r=paolo
authorAlphan Chen <alchen@mozilla.com>
Fri, 17 Aug 2018 16:40:02 +0000
changeset 487457 1906a8b16d63c4eae48b1f59bbaf0994eac35658
parent 487456 f66ffcb2e82af2f8c16b899af352834bd5bc34a9
child 487458 89117b6b5799666c38926cd566f230e19b050528
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo
bugs1469348
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1469348 - Fix the problem of download file failed on Mac. r=paolo In this bug, we will focus on the problem of cannot download PDF file from google drawing successfully on MAC. After investigating, I found that "SetTarget()" is called twice with two different addresses. However, they both point to the same file. We will assign the first target to "mInitialTarget" and the second one to "mRenamedTarget". This problem happened when doing the second "SetTarget()". After canceling the existing AsyncCopy, we will schedule a new AsyncCopy. However, we only assign the mActualTarget with mRenamedTarget when they point to different files. In this case, the two different addresses point to the same file. So the mActualTarget is still the same as mInitialTarget. After completion of the AsyncCopy, we try to do "CheckCompletion". But it will always return false due to "mRenamedTarget exists" and "mActualTarget" is not the same as "mRenamedTarget". The solution is quite easy. We should always update mActualTarget with renameTarget, even if they point to the same file. Differential Revision: https://phabricator.services.mozilla.com/D3400
netwerk/base/BackgroundFileSaver.cpp
--- a/netwerk/base/BackgroundFileSaver.cpp
+++ b/netwerk/base/BackgroundFileSaver.cpp
@@ -485,20 +485,26 @@ BackgroundFileSaver::ProcessStateChange(
         // Move the file.  If this fails, we still reference the original file
         // in mActualTarget, so that it is deleted if requested.  If this
         // succeeds, the nsIFile instance referenced by mActualTarget mutates
         // and starts pointing to the new file, but we'll discard the reference.
         rv = mActualTarget->MoveTo(renamedTargetParentDir, renamedTargetName);
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
-      // Now we can update the actual target file name.
-      mActualTarget = renamedTarget;
-      mActualTargetKeepPartial = renamedTargetKeepPartial;
+      // We should not only update the mActualTarget with renameTarget when
+      // they point to the different files.
+      // In this way, if mActualTarget and renamedTarget point to the same file
+      // with different addresses, "CheckCompletion()" will return false forever.
     }
+
+    // Update mActualTarget with renameTarget,
+    // even if they point to the same file.
+    mActualTarget = renamedTarget;
+    mActualTargetKeepPartial = renamedTargetKeepPartial;
   }
 
   // Notify if the target file name actually changed.
   if (!equalToCurrent) {
     // We must clone the nsIFile instance because mActualTarget is not
     // immutable, it may change if the target is renamed later.
     nsCOMPtr<nsIFile> actualTargetToNotify;
     rv = mActualTarget->Clone(getter_AddRefs(actualTargetToNotify));