corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1658202 - move writes from onDataAvailable away from the main thread, r=valentin

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D88730
This commit is contained in:
Gijs Kruitbosch
2020-09-02 23:15:06 +00:00
parent 3c49292468
commit 68cfcafa50
2 changed files with 108 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
dom/webbrowserpersist/nsWebBrowserPersist.cpp
View File

@@ -39,6 +39,7 @@
#include "nsIAuthPrompt.h"
#include "nsIPrompt.h"
#include "nsIFormControl.h"
#include "nsIThreadRetargetableRequest.h"
#include "nsContentUtils.h"
#include "ftpCore.h"
@@ -53,6 +54,7 @@
#include "mozilla/dom/HTMLInputElement.h"
#include "mozilla/dom/HTMLSharedElement.h"
#include "mozilla/net/CookieJarSettings.h"
#include "mozilla/Mutex.h"
#include "mozilla/Printf.h"
#include "ReferrerInfo.h"
@@ -97,10 +99,21 @@ struct nsWebBrowserPersist::URIData {
};
// Information about the output stream
// Note that this data structure (and the map that nsWebBrowserPersist keeps,
// where these are values) is used from two threads: the main thread,
// and the background task thread.
// The background thread only writes to mStream (from OnDataAvailable), and
// this access is guarded using mStreamMutex. It reads the mFile member, which
// is only written to on the main thread when the object is constructed and
// from OnStartRequest (if mCalcFileExt), both guaranteed to happen before
// OnDataAvailable is fired.
// The main thread gets OnStartRequest, OnStopRequest, and progress sink events,
// and accesses the other members.
struct nsWebBrowserPersist::OutputData {
nsCOMPtr<nsIURI> mFile;
nsCOMPtr<nsIURI> mOriginalLocation;
nsCOMPtr<nsIOutputStream> mStream;
Mutex mStreamMutex;
int64_t mSelfProgress;
int64_t mSelfProgressMax;
bool mCalcFileExt;
@@ -108,10 +121,16 @@ struct nsWebBrowserPersist::OutputData {
OutputData(nsIURI* aFile, nsIURI* aOriginalLocation, bool aCalcFileExt)
: mFile(aFile),
mOriginalLocation(aOriginalLocation),
mStreamMutex("nsWebBrowserPersist::OutputData::mStreamMutex"),
mSelfProgress(0),
mSelfProgressMax(10000),
mCalcFileExt(aCalcFileExt) {}
~OutputData() {
// Gaining this lock in the destructor is pretty icky. It should be OK
// because the only other place we lock the mutex is in OnDataAvailable,
// which will never itself cause the OutputData instance to be
// destroyed.
MutexAutoLock lock(mStreamMutex);
if (mStream) {
mStream->Close();
}
@@ -267,6 +286,7 @@ const char* kWebBrowserPersistStringBundle =
nsWebBrowserPersist::nsWebBrowserPersist()
: mCurrentDataPathIsRelative(false),
mCurrentThingsToPersist(0),
mOutputMapMutex("nsWebBrowserPersist::mOutputMapMutex"),
mFirstAndOnlyUse(true),
mSavingDocument(false),
mCancel(false),
@@ -298,6 +318,7 @@ NS_INTERFACE_MAP_BEGIN(nsWebBrowserPersist)
NS_INTERFACE_MAP_ENTRY(nsIInterfaceRequestor)
NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableStreamListener)
NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
NS_INTERFACE_MAP_ENTRY(nsIProgressEventSink)
NS_INTERFACE_MAP_END
@@ -785,6 +806,20 @@ NS_IMETHODIMP nsWebBrowserPersist::OnStartRequest(nsIRequest* request) {
}
if (data && data->mFile) {
nsCOMPtr<nsIThreadRetargetableRequest> r = do_QueryInterface(request);
// Determine if we're uploading. Only use OMT onDataAvailable if not.
nsCOMPtr<nsIFile> localFile;
GetLocalFileFromURI(data->mFile, getter_AddRefs(localFile));
if (r && localFile) {
if (!mBackgroundQueue) {
NS_CreateBackgroundTaskQueue("WebBrowserPersist",
getter_AddRefs(mBackgroundQueue));
}
if (mBackgroundQueue) {
r->RetargetDeliveryTo(mBackgroundQueue);
}
}
// If PERSIST_FLAGS_AUTODETECT_APPLY_CONVERSION is set in mPersistFlags,
// try to determine whether this channel needs to apply Content-Encoding
// conversions.
@@ -830,8 +865,11 @@ NS_IMETHODIMP nsWebBrowserPersist::OnStartRequest(nsIRequest* request) {
bool isEqual = false;
if (NS_SUCCEEDED(data->mFile->Equals(data->mOriginalLocation, &isEqual)) &&
isEqual) {
// remove from output map
mOutputMap.Remove(keyPtr);
{
MutexAutoLock lock(mOutputMapMutex);
// remove from output map
mOutputMap.Remove(keyPtr);
}
// cancel; we don't need to know any more
// stop request will get called
@@ -851,6 +889,7 @@ NS_IMETHODIMP nsWebBrowserPersist::OnStopRequest(nsIRequest* request,
SendErrorStatusChange(true, status, request, data->mFile);
}
MutexAutoLock lock(mOutputMapMutex);
// This will automatically close the output stream
mOutputMap.Remove(keyPtr);
} else {
@@ -880,6 +919,14 @@ NS_IMETHODIMP nsWebBrowserPersist::OnStopRequest(nsIRequest* request,
// nsWebBrowserPersist::nsIStreamListener
//*****************************************************************************
// Note: this is supposed to (but not guaranteed to) fire on a background
// thread when used to save to local disk (channels not using local files will
// use the main thread).
// (Read) Access to mOutputMap is guarded via mOutputMapMutex.
// Access to individual OutputData::mStream is guarded via its mStreamMutex.
// mCancel is atomic, as is mPersistFlags (accessed via MakeOutputStream).
// If you end up touching this method and needing other member access, bear
// this in mind.
NS_IMETHODIMP
nsWebBrowserPersist::OnDataAvailable(nsIRequest* request,
nsIInputStream* aIStream, uint64_t aOffset,
@@ -892,6 +939,7 @@ nsWebBrowserPersist::OnDataAvailable(nsIRequest* request,
nsCOMPtr<nsIChannel> channel = do_QueryInterface(request);
NS_ENSURE_TRUE(channel, NS_ERROR_FAILURE);
MutexAutoLock lock(mOutputMapMutex);
nsCOMPtr<nsISupports> keyPtr = do_QueryInterface(request);
OutputData* data = mOutputMap.Get(keyPtr);
if (!data) {
@@ -902,6 +950,7 @@ nsWebBrowserPersist::OnDataAvailable(nsIRequest* request,
bool readError = true;
MutexAutoLock streamLock(data->mStreamMutex);
// Make the output stream
if (!data->mStream) {
rv = MakeOutputStream(data->mFile, getter_AddRefs(data->mStream));
@@ -973,6 +1022,8 @@ nsWebBrowserPersist::OnDataAvailable(nsIRequest* request,
data->mStream->Close();
data->mStream =
nullptr; // null out stream so we don't close it later
MOZ_ASSERT(NS_IsMainThread(),
"Uploads should be on the main thread.");
rv = StartUpload(storStream, data->mFile, contentType);
if (NS_FAILED(rv)) {
readError = false;
@@ -984,19 +1035,34 @@ nsWebBrowserPersist::OnDataAvailable(nsIRequest* request,
// Notify listener if an error occurred.
if (cancel) {
SendErrorStatusChange(readError, rv, readError ? request : nullptr,
data->mFile);
RefPtr<nsIRequest> req = readError ? request : nullptr;
nsCOMPtr<nsIURI> file = data->mFile;
RefPtr<Runnable> errorOnMainThread = NS_NewRunnableFunction(
"nsWebBrowserPersist::SendErrorStatusChange",
[self = RefPtr{this}, req, file, readError, rv]() {
self->SendErrorStatusChange(readError, rv, req, file);
});
NS_DispatchToMainThread(errorOnMainThread);
}
}
// Cancel reading?
if (cancel) {
EndDownload(NS_BINDING_ABORTED);
nsCOMPtr<nsIRunnable> endOnMainThread = NewRunnableMethod<nsresult>(
"nsWebBrowserPersist::EndDownload", this,
&nsWebBrowserPersist::EndDownload, NS_BINDING_ABORTED);
NS_DispatchToMainThread(endOnMainThread);
}
return NS_OK;
return cancel ? NS_BINDING_ABORTED : NS_OK;
}
//*****************************************************************************
// nsWebBrowserPersist::nsIThreadRetargetableStreamListener
//*****************************************************************************
NS_IMETHODIMP nsWebBrowserPersist::CheckListenerChain() { return NS_OK; }
//*****************************************************************************
// nsWebBrowserPersist::nsIProgressEventSink
//*****************************************************************************
@@ -1391,6 +1457,7 @@ nsresult nsWebBrowserPersist::SaveChannelInternal(nsIChannel* aChannel,
return NS_SUCCESS_DONT_FIXUP;
}
MutexAutoLock lock(mOutputMapMutex);
// Add the output transport to the output map with the channel as the key
nsCOMPtr<nsISupports> keyPtr = do_QueryInterface(aChannel);
mOutputMap.Put(keyPtr, new OutputData(aFile, mURI, aCalcFileExt));
@@ -1712,13 +1779,18 @@ void nsWebBrowserPersist::FinishSaveDocumentInternal(nsIURI* aFile,
void nsWebBrowserPersist::Cleanup() {
mURIMap.Clear();
for (auto iter = mOutputMap.Iter(); !iter.Done(); iter.Next()) {
nsClassHashtable<nsISupportsHashKey, OutputData> outputMapCopy;
{
MutexAutoLock lock(mOutputMapMutex);
mOutputMap.SwapElements(outputMapCopy);
}
for (auto iter = outputMapCopy.Iter(); !iter.Done(); iter.Next()) {
nsCOMPtr<nsIChannel> channel = do_QueryInterface(iter.Key());
if (channel) {
channel->Cancel(NS_BINDING_ABORTED);
}
}
mOutputMap.Clear();
outputMapCopy.Clear();
for (auto iter = mUploadList.Iter(); !iter.Done(); iter.Next()) {
nsCOMPtr<nsIChannel> channel = do_QueryInterface(iter.Key());
@@ -2122,6 +2194,7 @@ nsresult nsWebBrowserPersist::CalculateAndAppendFileExt(
return NS_OK;
}
// Note: the MakeOutputStream helpers can be called from a background thread.
nsresult nsWebBrowserPersist::MakeOutputStream(
nsIURI* aURI, nsIOutputStream** aOutputStream) {
nsresult rv;
@@ -2164,7 +2237,18 @@ nsresult nsWebBrowserPersist::MakeOutputStreamFromFile(
}
cleanupData->mFile = aFile;
cleanupData->mIsDirectory = false;
mCleanupList.AppendElement(cleanupData);
if (NS_IsMainThread()) {
mCleanupList.AppendElement(cleanupData);
} else {
// If we're on a background thread, add the cleanup back on the main
// thread.
RefPtr<Runnable> addCleanup = NS_NewRunnableFunction(
"nsWebBrowserPersist::AddCleanupToList",
[self = RefPtr{this}, cleanup = std::move(cleanupData)]() {
self->mCleanupList.AppendElement(cleanup);
});
NS_DispatchToMainThread(addCleanup);
}
}
return NS_OK;
@@ -2244,6 +2328,9 @@ nsresult nsWebBrowserPersist::FixRedirectedChannelEntry(
}
if (matchingKey) {
// We only get called from OnStartRequest, so this is always on the
// main thread. Make sure we don't pull the rug from under anything else.
MutexAutoLock lock(mOutputMapMutex);
// If a match was found, remove the data entry with the old channel
// key and re-add it with the new channel key.
mozilla::UniquePtr<OutputData> outputData;