From c932d1e3d92adf5b14755af2c2ce1f618cc465f5 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 8 Apr 2015 22:50:45 -0400 Subject: [PATCH] Bug 1149235 part 2. Switch to using linked lists for nsScriptLoadRequest. r=sicking --- dom/base/nsScriptLoader.cpp | 136 +++++++++++++++++++++++++----------- dom/base/nsScriptLoader.h | 70 +++++++++++++++++-- 2 files changed, 159 insertions(+), 47 deletions(-) diff --git a/dom/base/nsScriptLoader.cpp b/dom/base/nsScriptLoader.cpp index 0245a3b3028d..3fd82579834e 100644 --- a/dom/base/nsScriptLoader.cpp +++ b/dom/base/nsScriptLoader.cpp @@ -66,6 +66,35 @@ using namespace mozilla::dom; // context, but it will AddRef and Release it on other threads. NS_IMPL_ISUPPORTS0(nsScriptLoadRequest) +nsScriptLoadRequestList::~nsScriptLoadRequestList() +{ + Clear(); +} + +void +nsScriptLoadRequestList::Clear() +{ + while (!isEmpty()) { + nsRefPtr first = StealFirst(); + // And just let it go out of scope and die. + } +} + +#ifdef DEBUG +bool +nsScriptLoadRequestList::Contains(nsScriptLoadRequest* aElem) +{ + for (nsScriptLoadRequest* req = getFirst(); + req; req = req->getNext()) { + if (req == aElem) { + return true; + } + } + + return false; +} +#endif // DEBUG + ////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////// @@ -93,20 +122,25 @@ nsScriptLoader::~nsScriptLoader() mParserBlockingRequest->FireScriptAvailable(NS_ERROR_ABORT); } - for (uint32_t i = 0; i < mXSLTRequests.Length(); i++) { - mXSLTRequests[i]->FireScriptAvailable(NS_ERROR_ABORT); + for (nsScriptLoadRequest* req = mXSLTRequests.getFirst(); req; + req = req->getNext()) { + req->FireScriptAvailable(NS_ERROR_ABORT); } - for (uint32_t i = 0; i < mDeferRequests.Length(); i++) { - mDeferRequests[i]->FireScriptAvailable(NS_ERROR_ABORT); + for (nsScriptLoadRequest* req = mDeferRequests.getFirst(); req; + req = req->getNext()) { + req->FireScriptAvailable(NS_ERROR_ABORT); } - for (uint32_t i = 0; i < mAsyncRequests.Length(); i++) { - mAsyncRequests[i]->FireScriptAvailable(NS_ERROR_ABORT); + for (nsScriptLoadRequest* req = mAsyncRequests.getFirst(); req; + req = req->getNext()) { + req->FireScriptAvailable(NS_ERROR_ABORT); } - for (uint32_t i = 0; i < mNonAsyncExternalScriptInsertedRequests.Length(); i++) { - mNonAsyncExternalScriptInsertedRequests[i]->FireScriptAvailable(NS_ERROR_ABORT); + for(nsScriptLoadRequest* req = mNonAsyncExternalScriptInsertedRequests.getFirst(); + req; + req = req->getNext()) { + req->FireScriptAvailable(NS_ERROR_ABORT); } // Unblock the kids, in case any of them moved to a different document @@ -591,6 +625,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) request->mJSVersion = version; if (aElement->GetScriptAsync()) { + request->mIsAsync = true; mAsyncRequests.AppendElement(request); if (!request->mLoading) { // The script is available already. Run it ASAP when the event @@ -603,6 +638,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) // Violate the HTML5 spec in order to make LABjs and the "order" plug-in // for RequireJS work with their Gecko-sniffed code path. See // http://lists.w3.org/Archives/Public/public-html/2010Oct/0088.html + request->mIsNonAsyncScriptInserted = true; mNonAsyncExternalScriptInsertedRequests.AppendElement(request); if (!request->mLoading) { // The script is available already. Run it ASAP when the event @@ -631,6 +667,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) // Need to maintain order for XSLT-inserted scripts NS_ASSERTION(!mParserBlockingRequest, "Parser-blocking scripts and XSLT scripts in the same doc!"); + request->mIsXSLT = true; mXSLTRequests.AppendElement(request); if (!request->mLoading) { // The script is available already. Run it ASAP when the event @@ -651,7 +688,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) // Web page. NS_ASSERTION(!mParserBlockingRequest, "There can be only one parser-blocking script at a time"); - NS_ASSERTION(mXSLTRequests.IsEmpty(), + NS_ASSERTION(mXSLTRequests.isEmpty(), "Parser-blocking scripts and XSLT scripts in the same doc!"); mParserBlockingRequest = request; ProcessPendingRequestsAsync(); @@ -661,7 +698,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) // The script will be run when it loads or the style sheet loads. NS_ASSERTION(!mParserBlockingRequest, "There can be only one parser-blocking script at a time"); - NS_ASSERTION(mXSLTRequests.IsEmpty(), + NS_ASSERTION(mXSLTRequests.isEmpty(), "Parser-blocking scripts and XSLT scripts in the same doc!"); mParserBlockingRequest = request; return true; @@ -687,7 +724,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) request->mLineNo = aElement->GetScriptLineNumber(); if (aElement->GetParserCreated() == FROM_PARSER_XSLT && - (!ReadyToExecuteScripts() || !mXSLTRequests.IsEmpty())) { + (!ReadyToExecuteScripts() || !mXSLTRequests.isEmpty())) { // Need to maintain order for XSLT-inserted scripts NS_ASSERTION(!mParserBlockingRequest, "Parser-blocking scripts and XSLT scripts in the same doc!"); @@ -706,7 +743,7 @@ nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) NS_ASSERTION(!mParserBlockingRequest, "There can be only one parser-blocking script at a time"); mParserBlockingRequest = request; - NS_ASSERTION(mXSLTRequests.IsEmpty(), + NS_ASSERTION(mXSLTRequests.isEmpty(), "Parser-blocking scripts and XSLT scripts in the same doc!"); return true; } @@ -1134,39 +1171,39 @@ nsScriptLoader::ProcessPendingRequests() } while (ReadyToExecuteScripts() && - !mXSLTRequests.IsEmpty() && - !mXSLTRequests[0]->mLoading) { - request.swap(mXSLTRequests[0]); - mXSLTRequests.RemoveElementAt(0); + !mXSLTRequests.isEmpty() && + !mXSLTRequests.getFirst()->mLoading) { + request = mXSLTRequests.StealFirst(); ProcessRequest(request); } - uint32_t i = 0; - while (mEnabled && i < mAsyncRequests.Length()) { - if (!mAsyncRequests[i]->mLoading) { - request.swap(mAsyncRequests[i]); - mAsyncRequests.RemoveElementAt(i); + // The refcounting here is dumb, but it's going away in the next + // patch in the queue. + nsRefPtr req = mAsyncRequests.getFirst(); + while (mEnabled && req && req->isInList()) { + if (!req->mLoading) { + nsRefPtr next = req->getNext(); + request = mAsyncRequests.Steal(req); ProcessRequest(request); + req.swap(next); continue; } - ++i; + req = req->getNext(); } - while (mEnabled && !mNonAsyncExternalScriptInsertedRequests.IsEmpty() && - !mNonAsyncExternalScriptInsertedRequests[0]->mLoading) { + while (mEnabled && !mNonAsyncExternalScriptInsertedRequests.isEmpty() && + !mNonAsyncExternalScriptInsertedRequests.getFirst()->mLoading) { // Violate the HTML5 spec and execute these in the insertion order in // order to make LABjs and the "order" plug-in for RequireJS work with // their Gecko-sniffed code path. See // http://lists.w3.org/Archives/Public/public-html/2010Oct/0088.html - request.swap(mNonAsyncExternalScriptInsertedRequests[0]); - mNonAsyncExternalScriptInsertedRequests.RemoveElementAt(0); + request = mNonAsyncExternalScriptInsertedRequests.StealFirst(); ProcessRequest(request); } - if (mDocumentParsingDone && mXSLTRequests.IsEmpty()) { - while (!mDeferRequests.IsEmpty() && !mDeferRequests[0]->mLoading) { - request.swap(mDeferRequests[0]); - mDeferRequests.RemoveElementAt(0); + if (mDocumentParsingDone && mXSLTRequests.isEmpty()) { + while (!mDeferRequests.isEmpty() && !mDeferRequests.getFirst()->mLoading) { + request = mDeferRequests.StealFirst(); ProcessRequest(request); } } @@ -1178,9 +1215,9 @@ nsScriptLoader::ProcessPendingRequests() } if (mDocumentParsingDone && mDocument && - !mParserBlockingRequest && mAsyncRequests.IsEmpty() && - mNonAsyncExternalScriptInsertedRequests.IsEmpty() && - mXSLTRequests.IsEmpty() && mDeferRequests.IsEmpty()) { + !mParserBlockingRequest && mAsyncRequests.isEmpty() && + mNonAsyncExternalScriptInsertedRequests.isEmpty() && + mXSLTRequests.isEmpty() && mDeferRequests.isEmpty()) { if (MaybeRemovedDeferRequests()) { return ProcessPendingRequests(); } @@ -1386,11 +1423,27 @@ nsScriptLoader::OnStreamComplete(nsIStreamLoader* aLoader, mDocument->AddBlockedTrackingNode(cont); } - if (mDeferRequests.RemoveElement(request) || - mAsyncRequests.RemoveElement(request) || - mNonAsyncExternalScriptInsertedRequests.RemoveElement(request) || - mXSLTRequests.RemoveElement(request)) { - FireScriptAvailable(rv, request); + if (request->mIsDefer) { + if (request->isInList()) { + nsRefPtr req = mDeferRequests.Steal(request); + FireScriptAvailable(rv, req); + } + } else if (request->mIsAsync) { + if (request->isInList()) { + nsRefPtr req = mAsyncRequests.Steal(request); + FireScriptAvailable(rv, req); + } + } else if (request->mIsNonAsyncScriptInserted) { + if (request->isInList()) { + nsRefPtr req = + mNonAsyncExternalScriptInsertedRequests.Steal(request); + FireScriptAvailable(rv, req); + } + } else if (request->mIsXSLT) { + if (request->isInList()) { + nsRefPtr req = mXSLTRequests.Steal(request); + FireScriptAvailable(rv, req); + } } else if (mParserBlockingRequest == request) { mParserBlockingRequest = nullptr; UnblockParser(request); @@ -1562,9 +1615,10 @@ nsScriptLoader::PreloadURI(nsIURI *aURI, const nsAString &aCharset, void nsScriptLoader::AddDeferRequest(nsScriptLoadRequest* aRequest) { + aRequest->mIsDefer = true; mDeferRequests.AppendElement(aRequest); - if (mDeferEnabled && mDeferRequests.Length() == 1 && mDocument && - !mBlockingDOMContentLoaded) { + if (mDeferEnabled && aRequest == mDeferRequests.getFirst() && + mDocument && !mBlockingDOMContentLoaded) { MOZ_ASSERT(mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_LOADING); mBlockingDOMContentLoaded = true; mDocument->BlockDOMContentLoaded(); @@ -1574,7 +1628,7 @@ nsScriptLoader::AddDeferRequest(nsScriptLoadRequest* aRequest) bool nsScriptLoader::MaybeRemovedDeferRequests() { - if (mDeferRequests.Length() == 0 && mDocument && + if (mDeferRequests.isEmpty() && mDocument && mBlockingDOMContentLoaded) { mBlockingDOMContentLoaded = false; mDocument->UnblockDOMContentLoaded(); diff --git a/dom/base/nsScriptLoader.h b/dom/base/nsScriptLoader.h index ce6ae1a07a97..2504d8ebc159 100644 --- a/dom/base/nsScriptLoader.h +++ b/dom/base/nsScriptLoader.h @@ -18,9 +18,10 @@ #include "nsIDocument.h" #include "nsIStreamLoader.h" #include "mozilla/CORSMode.h" +#include "mozilla/LinkedList.h" #include "mozilla/net/ReferrerPolicy.h" -class nsScriptLoadRequest; +class nsScriptLoadRequestList; class nsIURI; namespace JS { @@ -37,12 +38,20 @@ class AutoJSAPI; // Per-request data structure ////////////////////////////////////////////////////////////// -class nsScriptLoadRequest final : public nsISupports { +class nsScriptLoadRequest final : public nsISupports, + private mozilla::LinkedListElement +{ ~nsScriptLoadRequest() { js_free(mScriptTextBuf); } + typedef LinkedListElement super; + + // Allow LinkedListElement to cast us to itself as needed. + friend class mozilla::LinkedListElement; + friend class nsScriptLoadRequestList; + public: nsScriptLoadRequest(nsIScriptElement* aElement, uint32_t aVersion, @@ -51,6 +60,10 @@ public: mLoading(true), mIsInline(true), mHasSourceMapURL(false), + mIsDefer(false), + mIsAsync(false), + mIsNonAsyncScriptInserted(false), + mIsXSLT(false), mScriptTextBuf(nullptr), mScriptTextLength(0), mJSVersion(aVersion), @@ -76,10 +89,17 @@ public: return mElement == nullptr; } + using super::getNext; + using super::isInList; + nsCOMPtr mElement; bool mLoading; // Are we still waiting for a load to complete? bool mIsInline; // Is the script inline or loaded? bool mHasSourceMapURL; // Does the HTTP header have a source map url? + bool mIsDefer; // True if we live in mDeferRequests. + bool mIsAsync; // True if we live in mAsyncRequests. + bool mIsNonAsyncScriptInserted; // True if we live in mNonAsyncExternalScriptInsertedRequests + bool mIsXSLT; // True if we live in mXSLTRequests. nsString mSourceMapURL; // Holds source map url for loaded scripts char16_t* mScriptTextBuf; // Holds script text for non-inline scripts. Don't size_t mScriptTextLength; // use nsString so we can give ownership to jsapi. @@ -92,6 +112,44 @@ public: mozilla::net::ReferrerPolicy mReferrerPolicy; }; +class nsScriptLoadRequestList : private mozilla::LinkedList +{ + typedef mozilla::LinkedList super; + +public: + ~nsScriptLoadRequestList(); + + void Clear(); + +#ifdef DEBUG + bool Contains(nsScriptLoadRequest* aElem); +#endif // DEBUG + + using super::getFirst; + using super::isEmpty; + + void AppendElement(nsScriptLoadRequest* aElem) + { + MOZ_ASSERT(!aElem->isInList()); + NS_ADDREF(aElem); + insertBack(aElem); + } + + MOZ_WARN_UNUSED_RESULT + already_AddRefed Steal(nsScriptLoadRequest* aElem) + { + aElem->removeFrom(*this); + return dont_AddRef(aElem); + } + + MOZ_WARN_UNUSED_RESULT + already_AddRefed StealFirst() + { + MOZ_ASSERT(!isEmpty()); + return Steal(getFirst()); + } +}; + ////////////////////////////////////////////////////////////// // Script loader implementation ////////////////////////////////////////////////////////////// @@ -396,10 +454,10 @@ private: nsIDocument* mDocument; // [WEAK] nsCOMArray mObservers; - nsTArray > mNonAsyncExternalScriptInsertedRequests; - nsTArray > mAsyncRequests; - nsTArray > mDeferRequests; - nsTArray > mXSLTRequests; + nsScriptLoadRequestList mNonAsyncExternalScriptInsertedRequests; + nsScriptLoadRequestList mAsyncRequests; + nsScriptLoadRequestList mDeferRequests; + nsScriptLoadRequestList mXSLTRequests; nsRefPtr mParserBlockingRequest; // In mRequests, the additional information here is stored by the element.