From 4649687068c69ee2fea0f6d777949dfb5bca59cd Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Thu, 4 Jul 2019 13:31:06 -0700 Subject: [PATCH] #559: M1550498 M1548822 M1540759(partial) M1528481(+WeakPtr for Http2Stream) M1555523 M1552541 --- dom/base/nsGlobalWindow.cpp | 10 ++-- netwerk/protocol/http/Http2Session.cpp | 47 ++++++++++--------- netwerk/protocol/http/Http2Stream.h | 3 ++ netwerk/protocol/http/HttpChannelChild.cpp | 23 +++++++-- netwerk/protocol/http/nsHttpChannel.cpp | 3 +- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 14 ++++-- netwerk/protocol/http/nsHttpHandler.cpp | 2 +- netwerk/protocol/http/nsHttpHandler.h | 3 +- parser/html/javasrc/Tokenizer.java | 9 ++-- parser/html/nsHtml5Tokenizer.cpp | 7 +-- 10 files changed, 74 insertions(+), 47 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 50988de95..1ce9ca4ae 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -2126,11 +2126,13 @@ nsGlobalWindow::WouldReuseInnerWindow(nsIDocument* aNewDocument) } bool equal; - if (NS_SUCCEEDED(mDoc->NodePrincipal()->Equals(aNewDocument->NodePrincipal(), - &equal)) && - equal) { + if (NS_SUCCEEDED( + BasePrincipal::Cast(mDoc->NodePrincipal())-> + EqualsConsideringDomain(aNewDocument->NodePrincipal(), + &equal))) { + // Return the result. If true (bug 1552541): // The origin is the same. - return true; + return equal; } return false; diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index 4260bcfc1..702d48176 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -1673,8 +1673,9 @@ Http2Session::RecvPushPromise(Http2Session *self) RefPtr transactionBuffer = new Http2PushTransactionBuffer(); transactionBuffer->SetConnection(self); - Http2PushedStream *pushedStream = - new Http2PushedStream(transactionBuffer, self, associatedStream, promisedID); + nsAutoPtr pushedStream( + new Http2PushedStream(transactionBuffer, self, associatedStream, promisedID) + ); rv = pushedStream->ConvertPushHeaders(&self->mDecompressor, self->mDecompressBuffer, @@ -1683,7 +1684,6 @@ Http2Session::RecvPushPromise(Http2Session *self) if (rv == NS_ERROR_NOT_IMPLEMENTED) { LOG3(("Http2Session::PushPromise Semantics not Implemented\n")); self->GenerateRstStream(REFUSED_STREAM_ERROR, promisedID); - delete pushedStream; return NS_OK; } @@ -1691,7 +1691,6 @@ Http2Session::RecvPushPromise(Http2Session *self) // This means the decompression completed ok, but there was a problem with // the decoded headers. Reset the stream and go away. self->GenerateRstStream(PROTOCOL_ERROR, promisedID); - delete pushedStream; return NS_OK; } else if (NS_FAILED(rv)) { // This is fatal to the session. @@ -1699,14 +1698,17 @@ Http2Session::RecvPushPromise(Http2Session *self) return rv; } + WeakPtr pushedWeak = pushedStream.forget(); + // Ownership of the pushed stream is by the transaction hash, just as it // is for a client initiated stream. Errors that aren't fatal to the // whole session must call cleanupStream() after this point in order // to remove the stream from that hash. - self->mStreamTransactionHash.Put(transactionBuffer, pushedStream); - self->mPushedStreams.AppendElement(pushedStream); + self->mStreamTransactionHash.Put(transactionBuffer, pushedWeak); + self->mPushedStreams.AppendElement( + static_cast(pushedWeak.get())); - if (self->RegisterStreamID(pushedStream, promisedID) == kDeadStreamID) { + if (self->RegisterStreamID(pushedWeak, promisedID) == kDeadStreamID) { LOG3(("Http2Session::RecvPushPromise registerstreamid failed\n")); self->mGoAwayReason = INTERNAL_ERROR; return NS_ERROR_FAILURE; @@ -1718,23 +1720,24 @@ Http2Session::RecvPushPromise(Http2Session *self) // Fake the request side of the pushed HTTP transaction. Sets up hash // key and origin uint32_t notUsed; - pushedStream->ReadSegments(nullptr, 1, ¬Used); + pushedWeak->ReadSegments(nullptr, 1, ¬Used); nsAutoCString key; - if (!pushedStream->GetHashKey(key)) { + if (!static_cast(pushedWeak.get())->GetHashKey(key)) { LOG3(("Http2Session::RecvPushPromise one of :authority :scheme :path missing from push\n")); - self->CleanupStream(pushedStream, NS_ERROR_FAILURE, PROTOCOL_ERROR); + self->CleanupStream(pushedWeak, NS_ERROR_FAILURE, PROTOCOL_ERROR); self->ResetDownstreamState(); return NS_OK; } + // does the pushed origin belong on this connection? RefPtr associatedURL, pushedURL; rv = Http2Stream::MakeOriginURL(associatedStream->Origin(), associatedURL); if (NS_SUCCEEDED(rv)) { - rv = Http2Stream::MakeOriginURL(pushedStream->Origin(), pushedURL); + rv = Http2Stream::MakeOriginURL(pushedWeak->Origin(), pushedURL); } LOG3(("Http2Session::RecvPushPromise %p checking %s == %s", self, - associatedStream->Origin().get(), pushedStream->Origin().get())); + associatedStream->Origin().get(), pushedWeak->Origin().get())); bool match = false; if (NS_SUCCEEDED(rv)) { rv = associatedURL->Equals(pushedURL, &match); @@ -1742,36 +1745,38 @@ Http2Session::RecvPushPromise(Http2Session *self) if (NS_FAILED(rv)) { // Fallback to string equality of origins. This won't be guaranteed to be as // liberal as we want it to be, but it will at least be safe - match = associatedStream->Origin().Equals(pushedStream->Origin()); + match = associatedStream->Origin().Equals(pushedWeak->Origin()); } if (!match) { LOG3(("Http2Session::RecvPushPromise %p pushed stream mismatched origin " "associated origin %s .. pushed origin %s\n", self, - associatedStream->Origin().get(), pushedStream->Origin().get())); - self->CleanupStream(pushedStream, NS_ERROR_FAILURE, REFUSED_STREAM_ERROR); + associatedStream->Origin().get(), pushedWeak->Origin().get())); + self->CleanupStream(pushedWeak, NS_ERROR_FAILURE, REFUSED_STREAM_ERROR); self->ResetDownstreamState(); return NS_OK; } - if (pushedStream->TryOnPush()) { + if (static_cast(pushedWeak.get())->TryOnPush()) { LOG3(("Http2Session::RecvPushPromise %p channel implements nsIHttpPushListener " - "stream %p will not be placed into session cache.\n", self, pushedStream)); + "stream %p will not be placed into session cache.\n", self, pushedWeak.get())); } else { LOG3(("Http2Session::RecvPushPromise %p place stream into session cache\n", self)); - if (!cache->RegisterPushedStreamHttp2(key, pushedStream)) { + if (!cache->RegisterPushedStreamHttp2( + key, static_cast(pushedWeak.get()))) { + // This only happens if they've already pushed us this item. LOG3(("Http2Session::RecvPushPromise registerPushedStream Failed\n")); - self->CleanupStream(pushedStream, NS_ERROR_FAILURE, INTERNAL_ERROR); + self->CleanupStream(pushedWeak, NS_ERROR_FAILURE, INTERNAL_ERROR); self->ResetDownstreamState(); return NS_OK; } } - pushedStream->SetHTTPState(Http2Stream::RESERVED_BY_REMOTE); + pushedWeak->SetHTTPState(Http2Stream::RESERVED_BY_REMOTE); static_assert(Http2Stream::kWorstPriority >= 0, "kWorstPriority out of range"); uint8_t priorityWeight = (nsISupportsPriority::PRIORITY_LOWEST + 1) - (Http2Stream::kWorstPriority - Http2Stream::kNormalPriority); - pushedStream->SetPriority(Http2Stream::kWorstPriority); + pushedWeak->SetPriority(Http2Stream::kWorstPriority); self->GeneratePriority(promisedID, priorityWeight); self->ResetDownstreamState(); return NS_OK; diff --git a/netwerk/protocol/http/Http2Stream.h b/netwerk/protocol/http/Http2Stream.h index 3cd2e424f..6c42c5742 100644 --- a/netwerk/protocol/http/Http2Stream.h +++ b/netwerk/protocol/http/Http2Stream.h @@ -13,6 +13,7 @@ #include "mozilla/UniquePtr.h" #include "nsAHttpTransaction.h" #include "nsISupportsPriority.h" +#include "mozilla/WeakPtr.h" class nsStandardURL; @@ -25,8 +26,10 @@ class Http2Decompressor; class Http2Stream : public nsAHttpSegmentReader , public nsAHttpSegmentWriter + , public SupportsWeakPtr { public: + MOZ_DECLARE_WEAKREFERENCE_TYPENAME(Http2Stream) NS_DECL_NSAHTTPSEGMENTREADER NS_DECL_NSAHTTPSEGMENTWRITER diff --git a/netwerk/protocol/http/HttpChannelChild.cpp b/netwerk/protocol/http/HttpChannelChild.cpp index 75e3adbc9..fc6dc34c8 100644 --- a/netwerk/protocol/http/HttpChannelChild.cpp +++ b/netwerk/protocol/http/HttpChannelChild.cpp @@ -553,7 +553,14 @@ void HttpChannelChild::DoOnStartRequest(nsIRequest* aRequest, nsISupports* aContext) { LOG(("HttpChannelChild::DoOnStartRequest [this=%p]\n", this)); - nsresult rv = mListener->OnStartRequest(aRequest, aContext); + nsresult rv; + if (MOZ_LIKELY(mListener)) { + nsCOMPtr listener(mListener); + rv = listener->OnStartRequest(aRequest, aContext); + } else { + rv = NS_ERROR_UNEXPECTED; + } + if (NS_FAILED(rv)) { Cancel(rv); return; @@ -817,9 +824,12 @@ HttpChannelChild::DoOnDataAvailable(nsIRequest* aRequest, nsISupports* aContext, if (mCanceled) return; - nsresult rv = mListener->OnDataAvailable(aRequest, aContext, aStream, offset, count); - if (NS_FAILED(rv)) { - Cancel(rv); + if (MOZ_LIKELY(mListener)) { + nsCOMPtr listener(mListener); + nsresult rv = listener->OnDataAvailable(aRequest, aContext, aStream, offset, count); + if (NS_FAILED(rv)) { + Cancel(rv); + } } } @@ -982,7 +992,10 @@ HttpChannelChild::DoOnStopRequest(nsIRequest* aRequest, nsresult aChannelStatus, nsChannelClassifier::SetBlockedTrackingContent(this); } - mListener->OnStopRequest(aRequest, aContext, mStatus); + if (MOZ_LIKELY(mListener)) { + nsCOMPtr listener(mListener); + listener->OnStopRequest(aRequest, aContext, mStatus); + } mListener = 0; mListenerContext = 0; diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 4d8ce5a8f..c5797496c 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -6008,7 +6008,8 @@ nsHttpChannel::OnStopRequest(nsIRequest *request, nsISupports *ctxt, nsresult st if (mListener) { MOZ_ASSERT(!mOnStartRequestCalled, "We should not call OnStartRequest twice."); - mListener->OnStartRequest(this, mListenerContext); + nsCOMPtr listener(mListener); + listener->OnStartRequest(this, mListenerContext); mOnStartRequestCalled = true; } else { NS_WARNING("OnStartRequest skipped because of null listener"); diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 200a067b8..2723ba1cb 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -375,8 +375,12 @@ nsHttpConnectionMgr::VerifyTraffic() nsresult nsHttpConnectionMgr::DoShiftReloadConnectionCleanup(nsHttpConnectionInfo *aCI) { + RefPtr ci; + if (aCI) { + ci = aCI->Clone(); + } return PostEvent(&nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup, - 0, aCI); + 0, ci); } class SpeculativeConnectArgs : public ARefBase @@ -505,9 +509,13 @@ nsHttpConnectionMgr::UpdateParam(nsParamName name, uint16_t value) } nsresult -nsHttpConnectionMgr::ProcessPendingQ(nsHttpConnectionInfo *ci) +nsHttpConnectionMgr::ProcessPendingQ(nsHttpConnectionInfo *aCI) { - LOG(("nsHttpConnectionMgr::ProcessPendingQ [ci=%s]\n", ci->HashKey().get())); + LOG(("nsHttpConnectionMgr::ProcessPendingQ [ci=%s]\n", aCI->HashKey().get())); + RefPtr ci; + if (aCI) { + ci = aCI->Clone(); + } return PostEvent(&nsHttpConnectionMgr::OnMsgProcessPendingQ, 0, ci); } diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 5a1761eeb..edc80ed42 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -2233,7 +2233,7 @@ nsHttpHandler::SpeculativeConnectInternal(nsIURI *aURI, nsAutoCString username; aURI->GetUsername(username); - nsHttpConnectionInfo *ci = + RefPtr ci = new nsHttpConnectionInfo(host, port, EmptyCString(), username, nullptr, usingSSL); ci->SetAnonymous(anonymous); diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 0e825700c..6d01f587c 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -233,7 +233,8 @@ public: uint32_t caps = 0) { TickleWifi(callbacks); - return mConnMgr->SpeculativeConnect(ci, callbacks, caps); + RefPtr clone = ci->Clone(); + return mConnMgr->SpeculativeConnect(clone, callbacks, caps); } // Alternate Services Maps are main thread only diff --git a/parser/html/javasrc/Tokenizer.java b/parser/html/javasrc/Tokenizer.java index ba2288c4b..d67902ec3 100644 --- a/parser/html/javasrc/Tokenizer.java +++ b/parser/html/javasrc/Tokenizer.java @@ -3782,12 +3782,9 @@ public class Tokenizer implements Locator { tokenHandler.characters( Tokenizer.LT_SOLIDUS, 0, 2); emitStrBuf(); - if (c == '\u0000') { - emitReplacementCharacter(buf, pos); - } else { - cstart = pos; // don't drop the - // character - } + cstart = pos; // don't drop the + // character + reconsume = true; state = transition(state, returnState, reconsume, pos); continue stateloop; } diff --git a/parser/html/nsHtml5Tokenizer.cpp b/parser/html/nsHtml5Tokenizer.cpp index e2280cec9..6dcabd2e0 100644 --- a/parser/html/nsHtml5Tokenizer.cpp +++ b/parser/html/nsHtml5Tokenizer.cpp @@ -2048,11 +2048,8 @@ nsHtml5Tokenizer::stateLoop(int32_t state, char16_t c, int32_t pos, char16_t* bu default: { tokenHandler->characters(nsHtml5Tokenizer::LT_SOLIDUS, 0, 2); emitStrBuf(); - if (c == '\0') { - emitReplacementCharacter(buf, pos); - } else { - cstart = pos; - } + cstart = pos; + reconsume = true; state = P::transition(mViewSource, returnState, reconsume, pos); NS_HTML5_CONTINUE(stateloop); }