Bug 236989 - Ensure persisted subdocuments correctly fixup relative links. r=bzbarsky
authorjohn <jmdetloff@gmail.com>
Wed, 02 Sep 2015 23:53:38 -0700
changeset 294893 849943de58017e76de754f34a3eca9a33e8a3d53
parent 294892 c93cd72f8d4057f536fd87cda8088828bd1bd5ef
child 294894 23c3b1d231cea4d21e98ebc58cd437a06b76a58e
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs236989
milestone43.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 236989 - Ensure persisted subdocuments correctly fixup relative links. r=bzbarsky The URI of the document being serialized must be taken into account within GetLocalURI on URIData. This method previously returned its path relative to the document in which it is first visited, resulting in a bug when subdocuments stored in different locations also linked to it. By passing in mTargetBaseURI GetLocalURI can ensure it is returning a path relative to the document currently being serialized.
embedding/components/webbrowserpersist/nsWebBrowserPersist.cpp
embedding/components/webbrowserpersist/nsWebBrowserPersist.h
--- a/embedding/components/webbrowserpersist/nsWebBrowserPersist.cpp
+++ b/embedding/components/webbrowserpersist/nsWebBrowserPersist.cpp
@@ -76,38 +76,42 @@ struct nsWebBrowserPersist::WalkData
 };
 
 // Information about a DOM document
 struct nsWebBrowserPersist::DocData
 {
     nsCOMPtr<nsIURI> mBaseURI;
     nsCOMPtr<nsIWebBrowserPersistDocument> mDocument;
     nsCOMPtr<nsIURI> mFile;
-    nsCOMPtr<nsIURI> mDataPath;
-    bool mDataPathIsRelative;
-    nsCString mRelativePathToData;
     nsCString mCharset;
 };
 
 // Information about a URI
 struct nsWebBrowserPersist::URIData
 {
     bool mNeedsPersisting;
     bool mSaved;
     bool mIsSubFrame;
     bool mDataPathIsRelative;
     bool mNeedsFixup;
     nsString mFilename;
     nsString mSubFrameExt;
     nsCOMPtr<nsIURI> mFile;
     nsCOMPtr<nsIURI> mDataPath;
+    nsCOMPtr<nsIURI> mRelativeDocumentURI;
     nsCString mRelativePathToData;
     nsCString mCharset;
 
-    nsresult GetLocalURI(nsCString& aSpec);
+    nsresult GetLocalURI(nsIURI *targetBaseURI, nsCString& aSpecOut);
+};
+
+struct nsWebBrowserPersist::URIFixupData
+{
+    nsRefPtr<FlatURIMap> mFlatMap;
+    nsCOMPtr<nsIURI> mTargetBaseURI;
 };
 
 // Information about the output stream
 struct nsWebBrowserPersist::OutputData
 {
     nsCOMPtr<nsIURI> mFile;
     nsCOMPtr<nsIURI> mOriginalLocation;
     nsCOMPtr<nsIOutputStream> mStream;
@@ -638,30 +642,37 @@ nsWebBrowserPersist::SerializeNextFile()
     }
 
     mCurrentBaseURI = docData->mBaseURI;
     mCurrentCharset = docData->mCharset;
     mTargetBaseURI = docData->mFile;
 
     // Save the document, fixing it up with the new URIs as we do
 
-    if (!mFlatURIMap) {
-        nsAutoCString targetBaseSpec;
-        if (mTargetBaseURI) {
-            rv = mTargetBaseURI->GetSpec(targetBaseSpec);
-            if (NS_FAILED(rv)) {
-                SendErrorStatusChange(true, rv, nullptr, nullptr);
-                EndDownload(rv);
-                return;
-            }
+    nsAutoCString targetBaseSpec;
+    if (mTargetBaseURI) {
+        rv = mTargetBaseURI->GetSpec(targetBaseSpec);
+        if (NS_FAILED(rv)) {
+            SendErrorStatusChange(true, rv, nullptr, nullptr);
+            EndDownload(rv);
+            return;
         }
-        nsRefPtr<FlatURIMap> flatMap = new FlatURIMap(targetBaseSpec);
-        mURIMap.EnumerateRead(EnumCopyURIsToFlatMap, flatMap);
-        mFlatURIMap = flatMap.forget();
     }
+    
+    // mFlatURIMap must be rebuilt each time through SerializeNextFile, as
+    // mTargetBaseURI is used to create the relative URLs and will be different
+    // with each serialized document.
+    nsRefPtr<FlatURIMap> flatMap = new FlatURIMap(targetBaseSpec);
+    
+    URIFixupData fixupData;
+    fixupData.mFlatMap = flatMap;
+    fixupData.mTargetBaseURI = mTargetBaseURI;
+    
+    mURIMap.EnumerateRead(EnumCopyURIsToFlatMap, &fixupData);
+    mFlatURIMap = flatMap.forget();
 
     nsCOMPtr<nsIFile> localFile;
     GetLocalFileFromURI(docData->mFile, getter_AddRefs(localFile));
     if (localFile) {
         // if we're not replacing an existing file but the file
         // exists, something is wrong
         bool fileExists = false;
         rv = localFile->Exists(&fileExists);
@@ -1191,17 +1202,18 @@ nsresult nsWebBrowserPersist::GetValidUR
         *aURI = objAsURI;
         NS_ADDREF(*aURI);
         return NS_OK;
     }
 
     return NS_ERROR_FAILURE;
 }
 
-nsresult nsWebBrowserPersist::GetLocalFileFromURI(nsIURI *aURI, nsIFile **aLocalFile) const
+/* static */ nsresult
+nsWebBrowserPersist::GetLocalFileFromURI(nsIURI *aURI, nsIFile **aLocalFile)
 {
     nsresult rv;
 
     nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
     if (NS_FAILED(rv))
         return rv;
 
     nsCOMPtr<nsIFile> file;
@@ -1548,16 +1560,17 @@ nsresult nsWebBrowserPersist::SaveDocume
         // 3. Store the document in a list and wait for URI persistence to finish
         // 4. After URI persistence completes save the list of documents,
         //    fixing it up as it goes out to file.
 
         mCurrentDataPathIsRelative = false;
         mCurrentDataPath = aDataPath;
         mCurrentRelativePathToData = "";
         mCurrentThingsToPersist = 0;
+        mTargetBaseURI = aFile;
 
         // Determine if the specified data path is relative to the
         // specified file, (e.g. c:\docs\htmldata is relative to
         // c:\docs\myfile.htm, but not to d:\foo\data.
 
         // Starting with the data dir work back through its parents
         // checking if one of them matches the base directory.
 
@@ -1613,36 +1626,30 @@ nsresult nsWebBrowserPersist::SaveDocume
         // filenames of saved URIs are known, the documents can be fixed up and
         // saved
 
         DocData *docData = new DocData;
         docData->mBaseURI = mCurrentBaseURI;
         docData->mCharset = mCurrentCharset;
         docData->mDocument = aDocument;
         docData->mFile = aFile;
-        docData->mRelativePathToData = mCurrentRelativePathToData;
-        docData->mDataPath = mCurrentDataPath;
-        docData->mDataPathIsRelative = mCurrentDataPathIsRelative;
         mDocList.AppendElement(docData);
 
         // Walk the DOM gathering a list of externally referenced URIs in the uri map
         nsCOMPtr<nsIWebBrowserPersistResourceVisitor> visit =
             new OnWalk(this, aFile, localDataPath);
         return aDocument->ReadResources(visit);
     }
     else
     {
         DocData *docData = new DocData;
         docData->mBaseURI = mCurrentBaseURI;
         docData->mCharset = mCurrentCharset;
         docData->mDocument = aDocument;
         docData->mFile = aFile;
-        docData->mRelativePathToData = nullptr;
-        docData->mDataPath = nullptr;
-        docData->mDataPathIsRelative = false;
         mDocList.AppendElement(docData);
 
         // Not walking DOMs, so go directly to serialization.
         SerializeNextFile();
         return NS_OK;
     }
 }
 
@@ -2508,19 +2515,20 @@ nsWebBrowserPersist::EnumCleanupUploadLi
     return PL_DHASH_NEXT;
 }
 
 /* static */ PLDHashOperator
 nsWebBrowserPersist::EnumCopyURIsToFlatMap(const nsACString &aKey,
                                           URIData *aData,
                                           void* aClosure)
 {
-    FlatURIMap* theMap = static_cast<FlatURIMap*>(aClosure);
+    URIFixupData *fixupData = static_cast<URIFixupData*>(aClosure);
+    FlatURIMap* theMap = fixupData->mFlatMap;
     nsAutoCString mapTo;
-    nsresult rv = aData->GetLocalURI(mapTo);
+    nsresult rv = aData->GetLocalURI(fixupData->mTargetBaseURI, mapTo);
     if (NS_SUCCEEDED(rv) || !mapTo.IsVoid()) {
         theMap->Add(aKey, mapTo);
     }
     return PL_DHASH_NEXT;
 }
 
 nsresult
 nsWebBrowserPersist::StoreURI(
@@ -2570,17 +2578,17 @@ nsWebBrowserPersist::StoreURI(
     {
         *aData = data;
     }
 
     return NS_OK;
 }
 
 nsresult
-nsWebBrowserPersist::URIData::GetLocalURI(nsCString& aSpecOut)
+nsWebBrowserPersist::URIData::GetLocalURI(nsIURI *targetBaseURI, nsCString& aSpecOut)
 {
     aSpecOut.SetIsVoid(true);
     if (!mNeedsFixup) {
         return NS_OK;
     }
     nsresult rv;
     nsCOMPtr<nsIURI> fileAsURI;
     if (mFile) {
@@ -2594,29 +2602,52 @@ nsWebBrowserPersist::URIData::GetLocalUR
     }
 
     // remove username/password if present
     fileAsURI->SetUserPass(EmptyCString());
 
     // reset node attribute
     // Use relative or absolute links
     if (mDataPathIsRelative) {
-        nsCOMPtr<nsIURL> url(do_QueryInterface(fileAsURI));
-        if (!url) {
-            return NS_ERROR_FAILURE;
+        bool isEqual = false;
+        if (NS_SUCCEEDED(mRelativeDocumentURI->Equals(targetBaseURI, &isEqual)) && isEqual) {
+            nsCOMPtr<nsIURL> url(do_QueryInterface(fileAsURI));
+            if (!url) {
+                return NS_ERROR_FAILURE;
+            }
+            
+            nsAutoCString filename;
+            url->GetFileName(filename);
+            
+            nsAutoCString rawPathURL(mRelativePathToData);
+            rawPathURL.Append(filename);
+            
+            nsAutoCString buf;
+            aSpecOut = NS_EscapeURL(rawPathURL, esc_FilePath, buf);
+        } else {
+            nsAutoCString rawPathURL;
+            
+            nsCOMPtr<nsIFile> dataFile;
+            rv = GetLocalFileFromURI(mFile, getter_AddRefs(dataFile));
+            NS_ENSURE_SUCCESS(rv, rv);
+            
+            nsCOMPtr<nsIFile> docFile;
+            rv = GetLocalFileFromURI(targetBaseURI, getter_AddRefs(docFile));
+            NS_ENSURE_SUCCESS(rv, rv);
+            
+            nsCOMPtr<nsIFile> parentDir;
+            rv = docFile->GetParent(getter_AddRefs(parentDir));
+            NS_ENSURE_SUCCESS(rv, rv);
+            
+            rv = dataFile->GetRelativePath(parentDir, rawPathURL);
+            NS_ENSURE_SUCCESS(rv, rv);
+            
+            nsAutoCString buf;
+            aSpecOut = NS_EscapeURL(rawPathURL, esc_FilePath, buf);
         }
-
-        nsAutoCString filename;
-        url->GetFileName(filename);
-
-        nsAutoCString rawPathURL(mRelativePathToData);
-        rawPathURL.Append(filename);
-
-        nsAutoCString buf;
-        aSpecOut = NS_EscapeURL(rawPathURL, esc_FilePath, buf);
     } else {
         fileAsURI->GetSpec(aSpecOut);
     }
     if (mIsSubFrame) {
         AppendUTF16toUTF8(mSubFrameExt, aSpecOut);
     }
 
     return NS_OK;
@@ -2790,16 +2821,17 @@ nsWebBrowserPersist::MakeAndStoreLocalFi
     data->mNeedsPersisting = aNeedsPersisting;
     data->mNeedsFixup = true;
     data->mFilename = filename;
     data->mSaved = false;
     data->mIsSubFrame = false;
     data->mDataPath = mCurrentDataPath;
     data->mDataPathIsRelative = mCurrentDataPathIsRelative;
     data->mRelativePathToData = mCurrentRelativePathToData;
+    data->mRelativeDocumentURI = mTargetBaseURI;
     data->mCharset = mCurrentCharset;
 
     if (aNeedsPersisting)
         mCurrentThingsToPersist++;
 
     mURIMap.Put(spec, data);
     if (aData)
     {
--- a/embedding/components/webbrowserpersist/nsWebBrowserPersist.h
+++ b/embedding/components/webbrowserpersist/nsWebBrowserPersist.h
@@ -74,28 +74,29 @@ private:
         const char16_t *aContentType, char16_t **aExt);
 
     struct CleanupData;
     struct DocData;
     struct OutputData;
     struct UploadData;
     struct URIData;
     struct WalkData;
+    struct URIFixupData;
 
     class OnWalk;
     class OnWrite;
     class FlatURIMap;
     friend class OnWalk;
     friend class OnWrite;
 
     nsresult SaveDocumentDeferred(mozilla::UniquePtr<WalkData>&& aData);
     void Cleanup();
     void CleanupLocalFiles();
     nsresult GetValidURIFromObject(nsISupports *aObject, nsIURI **aURI) const;
-    nsresult GetLocalFileFromURI(nsIURI *aURI, nsIFile **aLocalFile) const;
+    static nsresult GetLocalFileFromURI(nsIURI *aURI, nsIFile **aLocalFile);
     static nsresult AppendPathToURI(nsIURI *aURI, const nsAString & aPath);
     nsresult MakeAndStoreLocalFilenameInURIMap(
         nsIURI *aURI, bool aNeedsPersisting, URIData **aData);
     nsresult MakeOutputStream(
         nsIURI *aFile, nsIOutputStream **aOutputStream);
     nsresult MakeOutputStreamFromFile(
         nsIFile *aFile, nsIOutputStream **aOutputStream);
     nsresult MakeOutputStreamFromURI(nsIURI *aURI, nsIOutputStream  **aOutStream);