diff --git a/netwerk/protocol/http/HttpBackgroundChannelChild.cpp b/netwerk/protocol/http/HttpBackgroundChannelChild.cpp index 9d8d3c57b06a..534e73715d8f 100644 --- a/netwerk/protocol/http/HttpBackgroundChannelChild.cpp +++ b/netwerk/protocol/http/HttpBackgroundChannelChild.cpp @@ -132,33 +132,15 @@ bool HttpBackgroundChannelChild::CreateBackgroundChannel() { return true; } -bool HttpBackgroundChannelChild::IsWaitingOnStartRequest( - bool aDataFromSocketProcess) { +bool HttpBackgroundChannelChild::IsWaitingOnStartRequest() { MOZ_ASSERT(OnSocketThread()); - // When data is from socket process, it is possible that both mStartSent and - // mStartReceived are false here. We need to wait until OnStartRequest sent - // from parent process. - // TODO: We can remove this code when diversion is removed in bug 1604448. - if (aDataFromSocketProcess) { - return !mStartReceived; - } - // Need to wait for OnStartRequest if it is sent by // parent process but not received by content process. - return (mStartSent && !mStartReceived); + return !mStartReceived; } // PHttpBackgroundChannelChild -IPCResult HttpBackgroundChannelChild::RecvOnStartRequestSent() { - LOG(("HttpBackgroundChannelChild::RecvOnStartRequestSent [this=%p]\n", this)); - MOZ_ASSERT(OnSocketThread()); - MOZ_ASSERT(!mStartSent); // Should only receive this message once. - - mStartSent = true; - return IPC_OK(); -} - IPCResult HttpBackgroundChannelChild::RecvOnStartRequest( const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead, const nsHttpHeaderArray& aRequestHeaders, @@ -171,11 +153,6 @@ IPCResult HttpBackgroundChannelChild::RecvOnStartRequest( return IPC_OK(); } - // TODO: OnStartRequest is off-main-thread so it's unnecessary to dispatch - // another IPC message for sync reason. Directly call here to behave the same - // as before. This is no longer needed and removed in the next patches. - RecvOnStartRequestSent(); - mChannelChild->ProcessOnStartRequest(aResponseHead, aUseResponseHead, aRequestHeaders, aArgs); // Allow to queue other runnable since OnStartRequest Event already hits the @@ -208,7 +185,8 @@ IPCResult HttpBackgroundChannelChild::RecvOnTransportAndData( return IPC_OK(); } - if (IsWaitingOnStartRequest(aDataFromSocketProcess)) { + // Bug 1641336: Race only happens if the data is from socket process. + if (IsWaitingOnStartRequest()) { LOG((" > pending until OnStartRequest [offset=%" PRIu64 " count=%" PRIu32 "]\n", aOffset, aCount)); diff --git a/netwerk/protocol/http/HttpBackgroundChannelChild.h b/netwerk/protocol/http/HttpBackgroundChannelChild.h index b24bc23dcdc7..241a9bd71a2b 100644 --- a/netwerk/protocol/http/HttpBackgroundChannelChild.h +++ b/netwerk/protocol/http/HttpBackgroundChannelChild.h @@ -73,8 +73,6 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild { IPCResult RecvDivertMessages(); - IPCResult RecvOnStartRequestSent(); - void ActorDestroy(ActorDestroyReason aWhy) override; void CreateDataBridge(); @@ -93,12 +91,7 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild { // OnStartRequestReceived. // return true after both RecvOnStartRequestSend and OnStartRequestReceived // are invoked. - // When ODA message is from socket process, it is possible that both - // RecvOnStartRequestSent and OnStartRequestReceived are not invoked, but - // RecvOnTransportAndData is already invoked. In this case, we only need to - // check if OnStartRequestReceived is invoked to make sure ODA doesn't happen - // before OnStartRequest. - bool IsWaitingOnStartRequest(bool aDataFromSocketProcess = false); + bool IsWaitingOnStartRequest(); // Associated HttpChannelChild for handling the channel events. // Will be removed while failed to create background channel, @@ -110,10 +103,6 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild { // Should only access on STS thread. bool mStartReceived = false; - // True if OnStartRequest is sent by HttpChannelParent. - // Should only access on STS thread. - bool mStartSent = false; - // Store pending messages that require to be handled after OnStartRequest. // Should be flushed after OnStartRequest is received and handled. // Should only access on STS thread. diff --git a/netwerk/protocol/http/HttpBackgroundChannelParent.cpp b/netwerk/protocol/http/HttpBackgroundChannelParent.cpp index 5ff1a63cb2d7..41b13491ac49 100644 --- a/netwerk/protocol/http/HttpBackgroundChannelParent.cpp +++ b/netwerk/protocol/http/HttpBackgroundChannelParent.cpp @@ -142,30 +142,6 @@ void HttpBackgroundChannelParent::OnChannelClosed() { Unused << NS_WARN_IF(NS_FAILED(rv)); } -bool HttpBackgroundChannelParent::OnStartRequestSent() { - LOG(("HttpBackgroundChannelParent::OnStartRequestSent [this=%p]\n", this)); - AssertIsInMainProcess(); - - if (NS_WARN_IF(!mIPCOpened)) { - return false; - } - - if (!IsOnBackgroundThread()) { - MutexAutoLock lock(mBgThreadMutex); - nsresult rv = mBackgroundThread->Dispatch( - NewRunnableMethod( - "net::HttpBackgroundChannelParent::OnStartRequestSent", this, - &HttpBackgroundChannelParent::OnStartRequestSent), - NS_DISPATCH_NORMAL); - - MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); - - return NS_SUCCEEDED(rv); - } - - return SendOnStartRequestSent(); -} - bool HttpBackgroundChannelParent::OnStartRequest( const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead, const nsHttpHeaderArray& aRequestHeaders, diff --git a/netwerk/protocol/http/HttpBackgroundChannelParent.h b/netwerk/protocol/http/HttpBackgroundChannelParent.h index 50f8e9d79e2d..5fbea5a737e1 100644 --- a/netwerk/protocol/http/HttpBackgroundChannelParent.h +++ b/netwerk/protocol/http/HttpBackgroundChannelParent.h @@ -39,9 +39,6 @@ class HttpBackgroundChannelParent final : public PHttpBackgroundChannelParent { // IPC channel. void OnChannelClosed(); - // To send OnStartRequestSend message over background channel. - bool OnStartRequestSent(); - // To send OnStartRequest message over background channel. bool OnStartRequest(const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead, diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index d37c1c3e701b..6f4489239adf 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -1535,8 +1535,6 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) { if (!mIPCClosed) { // TODO: For multipart channel, we would deliever everything across // pBackground as well. - // TODO: OnStartRequestSent is no longer needed since - // OnStartRequest/ODA/OnStopRequest are on the same thread. if (!mIsMultiPart) { ipcResult = mBgParent->OnStartRequest( *responseHead, useResponseHead, @@ -1554,19 +1552,6 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) { if (mIPCClosed || !ipcResult) { rv = NS_ERROR_UNEXPECTED; } - - // OnStartRequest is sent to content process successfully. - // Notify PHttpBackgroundChannelChild that all following IPC mesasges - // should be run after OnStartRequest is handled. - // We don't send this if it's multipart, since we don't use the - // background channel in that case. - if (NS_SUCCEEDED(rv) && !multiPartID) { - MOZ_ASSERT(mBgParent); - if (!mBgParent->OnStartRequestSent()) { - rv = NS_ERROR_UNEXPECTED; - } - } - return rv; } diff --git a/netwerk/protocol/http/PHttpBackgroundChannel.ipdl b/netwerk/protocol/http/PHttpBackgroundChannel.ipdl index d428e152b8a1..a447c8734c2f 100644 --- a/netwerk/protocol/http/PHttpBackgroundChannel.ipdl +++ b/netwerk/protocol/http/PHttpBackgroundChannel.ipdl @@ -20,12 +20,6 @@ async refcounted protocol PHttpBackgroundChannel manager PBackground; child: - // OnStartRequest is sent over main thread IPC. The following - // OnTransportAndData/OnStopRequest/OnProgress/OnStatus/FlushForDiversion/ - // DivertMessages needs to wait in content process until OnStartRequest - // is processed. For synchronizing the event sequence. - async OnStartRequestSent(); - async OnStartRequest(nsHttpResponseHead responseHead, bool useResponseHead, nsHttpHeaderArray requestHeaders,