mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1600211 - Send OnDataAvailable and OnStopRequest on the main-thread channel when in multi-part mode to avoid complicated races across the two channels. r=mayhemer 

This also removes OnStartRequestSent from PHttpBackgroundChannel, since there should never be any messages sent earlier on this channel, so we can just assume the waiting state initially.

Differential Revision: https://phabricator.services.mozilla.com/D55219

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Matt Woodrow 2019-12-04 03:18:38 +00:00
Родитель 92e976c088
Коммит a26e9542d2
10 изменённых файлов: 111 добавлений и 68 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

11
netwerk/protocol/http/HttpBackgroundChannelChild.cpp
Просмотреть файл

@ -102,19 +102,10 @@ bool HttpBackgroundChannelChild::IsWaitingOnStartRequest() {
MOZ_ASSERT(OnSocketThread());
// 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::RecvOnTransportAndData(
const nsresult& aChannelStatus, const nsresult& aTransportStatus,
const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {

6
netwerk/protocol/http/HttpBackgroundChannelChild.h
Просмотреть файл

@ -56,8 +56,6 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild {
IPCResult RecvDivertMessages();
IPCResult RecvOnStartRequestSent();
void ActorDestroy(ActorDestroyReason aWhy) override;
private:
@ -86,10 +84,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.

24
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::OnTransportAndData(
const nsresult& aChannelStatus, const nsresult& aTransportStatus,
const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {

3
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 OnTransportAndData message over background channel.
bool OnTransportAndData(const nsresult& aChannelStatus,
const nsresult& aTransportStatus,

41
netwerk/protocol/http/HttpChannelChild.cpp
Просмотреть файл

@ -425,7 +425,10 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest(
// mEventQ.
MutexAutoLock lock(mBgChildMutex);
if (mBgChild) {
// We don't need to notify the background channel if this is a multipart
// stream, since all messages will be sent over the main-thread IPDL in
// that case.
if (mBgChild && !aMultiPartID) {
MOZ_RELEASE_ASSERT(gSocketTransportService);
DebugOnly<nsresult> rv = gSocketTransportService->Dispatch(
NewRunnableMethod(
@ -549,6 +552,36 @@ void HttpChannelChild::OnStartRequest(
DoOnStartRequest(this, nullptr);
}
mozilla::ipc::IPCResult HttpChannelChild::RecvOnTransportAndData(
const nsresult& aChannelStatus, const nsresult& aTransportStatus,
const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {
AUTO_PROFILER_LABEL("HttpChannelChild::RecvOnTransportAndData", NETWORK);
LOG(("HttpChannelChild::RecvOnTransportAndData [this=%p]\n", this));
mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
this, [self = UnsafePtr<HttpChannelChild>(this), aChannelStatus,
aTransportStatus, aOffset, aCount, aData]() {
self->OnTransportAndData(aChannelStatus, aTransportStatus, aOffset,
aCount, aData);
}));
return IPC_OK();
}
mozilla::ipc::IPCResult HttpChannelChild::RecvOnStopRequest(
const nsresult& aChannelStatus, const ResourceTimingStructArgs& aTiming,
const TimeStamp& aLastActiveTabOptHit,
const nsHttpHeaderArray& aResponseTrailers) {
AUTO_PROFILER_LABEL("HttpChannelChild::RecvOnStopRequest", NETWORK);
LOG(("HttpChannelChild::RecvOnStopRequest [this=%p]\n", this));
mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
this, [self = UnsafePtr<HttpChannelChild>(this), aChannelStatus, aTiming,
aResponseTrailers]() {
self->OnStopRequest(aChannelStatus, aTiming, aResponseTrailers);
}));
return IPC_OK();
}
class SyntheticDiversionListener final : public nsIStreamListener {
RefPtr<HttpChannelChild> mChannel;
@ -665,6 +698,9 @@ void HttpChannelChild::ProcessOnTransportAndData(
const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {
LOG(("HttpChannelChild::ProcessOnTransportAndData [this=%p]\n", this));
MOZ_ASSERT(OnSocketThread());
MOZ_ASSERT(
!mMultiPartID,
"Should only send ODA on the main-thread channel when using multi-part!");
MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
"Should not be receiving any more callbacks from parent!");
mEventQ->RunOrEnqueue(
@ -890,6 +926,9 @@ void HttpChannelChild::ProcessOnStopRequest(
const nsHttpHeaderArray& aResponseTrailers) {
LOG(("HttpChannelChild::ProcessOnStopRequest [this=%p]\n", this));
MOZ_ASSERT(OnSocketThread());
MOZ_ASSERT(
!mMultiPartID,
"Should only send ODA on the main-thread channel when using multi-part!");
MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
"Should not be receiving any more callbacks from parent!");

9
netwerk/protocol/http/HttpChannelChild.h
Просмотреть файл

@ -148,6 +148,15 @@ class HttpChannelChild final : public PHttpChannelChild,
const ResourceTimingStructArgs& aTiming,
const bool& aAllRedirectsSameOrigin, const Maybe<uint32_t>& aMultiPartID,
const bool& aIsLastPartOfMultiPart) override;
mozilla::ipc::IPCResult RecvOnTransportAndData(
const nsresult& aChannelStatus, const nsresult& aTransportStatus,
const uint64_t& aOffset, const uint32_t& aCount,
const nsCString& aData) override;
mozilla::ipc::IPCResult RecvOnStopRequest(
const nsresult& aChannelStatus, const ResourceTimingStructArgs& aTiming,
const TimeStamp& aLastActiveTabOptHit,
const nsHttpHeaderArray& aResponseTrailers) override;
mozilla::ipc::IPCResult RecvFailedAsyncOpen(const nsresult& status) override;
mozilla::ipc::IPCResult RecvRedirect1Begin(
const uint32_t& registrarId, const URIParams& newURI,

47
netwerk/protocol/http/HttpChannelParent.cpp
Просмотреть файл

@ -84,7 +84,8 @@ HttpChannelParent::HttpChannelParent(const PBrowserOrId& iframeEmbedding,
mWillSynthesizeResponse(false),
mCacheNeedFlowControlInitialized(false),
mNeedFlowControl(true),
mSuspendedForFlowControl(false) {
mSuspendedForFlowControl(false),
mIsMultiPart(false) {
LOG(("Creating HttpChannelParent [this=%p]\n", this));
// Ensure gHttpHandler is initialized: we need the atom table up and running.
@ -1331,6 +1332,7 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
nsCOMPtr<nsIMultiPartChannel> multiPartChannel =
do_QueryInterface(aRequest);
if (multiPartChannel) {
mIsMultiPart = true;
nsCOMPtr<nsIChannel> baseChannel;
multiPartChannel->GetBaseChannel(getter_AddRefs(baseChannel));
chan = do_QueryObject(baseChannel);
@ -1341,6 +1343,8 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
multiPartChannel->GetIsLastPart(&isLastPartOfMultiPart);
}
}
MOZ_ASSERT(multiPartID || !mIsMultiPart, "Changed multi-part state?");
if (!chan) {
LOG((" aRequest is not HttpBaseChannel"));
NS_ERROR(
@ -1497,16 +1501,6 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
}
requestHead->Exit();
// OnStartRequest is sent to content process successfully.
// Notify PHttpBackgroundChannelChild that all following IPC mesasges
// should be run after OnStartRequest is handled.
if (NS_SUCCEEDED(rv)) {
MOZ_ASSERT(mBgParent);
if (!mBgParent->OnStartRequestSent()) {
rv = NS_ERROR_UNEXPECTED;
}
}
return rv;
}
@ -1531,10 +1525,20 @@ HttpChannelParent::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
// is ready to send OnStopRequest.
MOZ_ASSERT(mIPCClosed || mBgParent);
if (mIPCClosed || !mBgParent ||
!mBgParent->OnStopRequest(
aStatusCode, GetTimingAttributes(mChannel),
responseTrailer ? *responseTrailer : nsHttpHeaderArray())) {
// If we're handling a multi-part stream, then send this directly
// over PHttpChannel to make synchronization easier.
if (!mIPCClosed && mIsMultiPart) {
// See the child code for why we do this.
TimeStamp lastActTabOpt = nsHttp::GetLastActiveTabLoadOptimizationHit();
if (!SendOnStopRequest(
aStatusCode, GetTimingAttributes(mChannel), lastActTabOpt,
responseTrailer ? *responseTrailer : nsHttpHeaderArray())) {
return NS_ERROR_UNEXPECTED;
}
} else if (mIPCClosed || !mBgParent ||
!mBgParent->OnStopRequest(
aStatusCode, GetTimingAttributes(mChannel),
responseTrailer ? *responseTrailer : nsHttpHeaderArray())) {
return NS_ERROR_UNEXPECTED;
}
@ -1622,9 +1626,16 @@ HttpChannelParent::OnDataAvailable(nsIRequest* aRequest,
// is ready to send OnTransportAndData.
MOZ_ASSERT(mIPCClosed || mBgParent);
if (mIPCClosed || !mBgParent ||
!mBgParent->OnTransportAndData(channelStatus, transportStatus, aOffset,
toRead, data)) {
// If we're handling a multi-part stream, then send this directly
// over PHttpChannel to make synchronization easier.
if (!mIPCClosed && mIsMultiPart) {
if (!SendOnTransportAndData(channelStatus, transportStatus, aOffset,
toRead, data)) {
return NS_ERROR_UNEXPECTED;
}
} else if (mIPCClosed || !mBgParent ||
!mBgParent->OnTransportAndData(channelStatus, transportStatus,
aOffset, toRead, data)) {
return NS_ERROR_UNEXPECTED;
}

7
netwerk/protocol/http/HttpChannelParent.h
Просмотреть файл

@ -349,6 +349,13 @@ class HttpChannelParent final : public nsIInterfaceRequestor,
uint8_t mNeedFlowControl : 1;
uint8_t mSuspendedForFlowControl : 1;
// Set to true if we get OnStartRequest called with an nsIMultiPartChannel,
// and expect multiple OnStartRequest calls.
// When this happens we send OnTransportAndData and OnStopRequest over
// PHttpChannel instead of PHttpBackgroundChannel to make synchronizing all
// the parts easier.
uint8_t mIsMultiPart : 1;
// Number of events to wait before actually invoking AsyncOpen on the main
// channel. For each asynchronous step required before InvokeAsyncOpen, should
// increase 1 to mAsyncOpenBarrier and invoke TryInvokeAsyncOpen after

12
netwerk/protocol/http/PHttpBackgroundChannel.ipdl
Просмотреть файл

@ -21,20 +21,20 @@ 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();
// Combines a single OnDataAvailable and its associated OnProgress &
// OnStatus calls into one IPDL message
// This is duplicated on PHttpChannel, which gets used for multi-part
// streams to make synchronization when we get OnStartRequest multiple
// times easier.
async OnTransportAndData(nsresult channelStatus,
nsresult transportStatus,
uint64_t offset,
uint32_t count,
nsCString data);
// This is duplicated on PHttpChannel, which gets used for multi-part
// streams to make synchronization when we get OnStartRequest multiple
// times easier.
async OnStopRequest(nsresult channelStatus,
ResourceTimingStructArgs timing,
TimeStamp lastActiveTabOptimization,

19
netwerk/protocol/http/PHttpChannel.ipdl
Просмотреть файл

@ -132,6 +132,25 @@ child:
uint32_t? multiPartId,
bool isLastPartOfMultiPart);
// Combines a single OnDataAvailable and its associated OnProgress &
// OnStatus calls into one IPDL message
// This is a duplicate of the message on PHttpBackgroundChannel, and gets
// used for multi-pary streams to make synchronization when we get OnStartRequest
// multiple times easier.
async OnTransportAndData(nsresult channelStatus,
nsresult transportStatus,
uint64_t offset,
uint32_t count,
nsCString data);
// This is a duplicate of the message on PHttpBackgroundChannel, and gets
// used for multi-pary streams to make synchronization when we get OnStartRequest
// multiple times easier.
async OnStopRequest(nsresult channelStatus,
ResourceTimingStructArgs timing,
TimeStamp lastActiveTabOptimization,
nsHttpHeaderArray responseTrailers);
// Used to cancel child channel if we hit errors during creating and
// AsyncOpen of nsHttpChannel on the parent.
async FailedAsyncOpen(nsresult status);