Skip to content

Commit

Permalink
Bug 1622451 - minimize stream copying across IPC boundaries r=asuth,baku
Browse files Browse the repository at this point in the history
Initially, IPCInternal{Request,Response} had contained IPCStreams which would
result in unecessary copying when sending the objects over IPC. The patch
makes these streams either:

1) ParentToParentStream (just a UUID)
2) ParentToChildStream (a PIPCBlobInputStream actor, acting as a handle)
3) ChildToParentStream (a real IPCStream)

These three types are union-ed together by the BodyStreamVariant IPDL structure.
This structure replaces the IPCStream members in IPCInternal{Request,Response}
so that, depending on the particular IPDL protocol, we can avoid cloning streams
and just pass handles/IDs instead.

As a side effect, this makes file-backed Response objects cloneable. Initially,
these Responses would be backed by an nsFileInputStream, which is not cloneable
outside the parent process. They are now backed by IPCBlobInputStreams, which
are cloneable.

One thing that's not really satisfactory (IMO), is the manual management of
IPCBlobInputStreamStorage so that no streams are leaked, e.g. if we store a
stream in the IPCBlobInputStreamStorage but fail to send an IPC message and
therefore fail to remove the stream from storage on the other side of the IPC
boundary (only parent-to-parent in this case).

Differential Revision: https://phabricator.services.mozilla.com/D73173
  • Loading branch information
perryjiang committed Apr 30, 2020
1 parent 0b1ab06 commit 1ad4e03
Show file tree
Hide file tree
Showing 15 changed files with 391 additions and 332 deletions.
28 changes: 25 additions & 3 deletions dom/fetch/FetchTypes.ipdlh
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ include IPCStream;
include ChannelInfo;
include PBackgroundSharedTypes;

include protocol PIPCBlobInputStream;

using HeadersGuardEnum from "mozilla/dom/FetchIPCTypes.h";
using ReferrerPolicy from "mozilla/dom/FetchIPCTypes.h";
using RequestCache from "mozilla/dom/FetchIPCTypes.h";
using RequestCredentials from "mozilla/dom/FetchIPCTypes.h";
using RequestMode from "mozilla/dom/FetchIPCTypes.h";
using RequestRedirect from "mozilla/dom/FetchIPCTypes.h";
using ResponseType from "mozilla/dom/FetchIPCTypes.h";
using struct nsID from "nsID.h";

namespace mozilla {
namespace dom {
Expand All @@ -22,12 +25,31 @@ struct HeadersEntry {
nsCString value;
};

struct ParentToParentStream {
// Used as a key for IPCBlobInputStreamStorage
nsID uuid;
};

struct ParentToChildStream {
PIPCBlobInputStream actor;
};

struct ChildToParentStream {
IPCStream stream;
};

union BodyStreamVariant {
ParentToParentStream;
ParentToChildStream;
ChildToParentStream;
};

struct IPCInternalRequest {
nsCString method;
nsCString[] urlList;
HeadersGuardEnum headersGuard;
HeadersEntry[] headers;
IPCStream? body;
BodyStreamVariant? body;
int64_t bodySize;
nsCString preferredAlternativeDataType;
uint32_t contentPolicyType;
Expand All @@ -49,11 +71,11 @@ struct IPCInternalResponse {
nsCString statusText;
HeadersGuardEnum headersGuard;
HeadersEntry[] headers;
IPCStream? body;
BodyStreamVariant? body;
int64_t bodySize;
nsresult errorCode;
nsCString alternativeDataType;
IPCStream? alternativeBody;
BodyStreamVariant? alternativeBody;
IPCChannelInfo channelInfo;
PrincipalInfo? principalInfo;
};
Expand Down
58 changes: 14 additions & 44 deletions dom/fetch/InternalRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@

#include "InternalRequest.h"

#include "nsIContentPolicy.h"
#include "mozilla/dom/Document.h"
#include "nsStreamUtils.h"

#include "mozilla/ErrorResult.h"
#include "mozilla/dom/Document.h"
#include "mozilla/dom/FetchTypes.h"
#include "mozilla/dom/IPCBlobInputStreamChild.h"
#include "mozilla/dom/ScriptSettings.h"

#include "mozilla/dom/WorkerCommon.h"
#include "mozilla/dom/WorkerPrivate.h"
#include "mozilla/ipc/IPCStreamUtils.h"
#include "mozilla/ipc/PBackgroundChild.h"
#include "nsIContentPolicy.h"
#include "nsStreamUtils.h"

namespace mozilla {
namespace dom {
Expand Down Expand Up @@ -146,7 +145,6 @@ InternalRequest::InternalRequest(const IPCInternalRequest& aIPCRequest)
mURLList(aIPCRequest.urlList()),
mHeaders(new InternalHeaders(aIPCRequest.headers(),
aIPCRequest.headersGuard())),
mBodyStream(mozilla::ipc::DeserializeIPCStream(aIPCRequest.body())),
mBodyLength(aIPCRequest.bodySize()),
mPreferredAlternativeDataType(aIPCRequest.preferredAlternativeDataType()),
mContentPolicyType(
Expand All @@ -163,49 +161,21 @@ InternalRequest::InternalRequest(const IPCInternalRequest& aIPCRequest)
mPrincipalInfo = MakeUnique<mozilla::ipc::PrincipalInfo>(
aIPCRequest.principalInfo().ref());
}
}

InternalRequest::~InternalRequest() = default;
const Maybe<BodyStreamVariant>& body = aIPCRequest.body();

template void InternalRequest::ToIPC<mozilla::ipc::PBackgroundChild>(
IPCInternalRequest* aIPCRequest, mozilla::ipc::PBackgroundChild* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoStream);

template <typename M>
void InternalRequest::ToIPC(
IPCInternalRequest* aIPCRequest, M* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoStream) {
MOZ_ASSERT(aIPCRequest);
MOZ_ASSERT(aManager);
MOZ_ASSERT(!mURLList.IsEmpty());

aIPCRequest->method() = mMethod;
aIPCRequest->urlList() = mURLList;
mHeaders->ToIPC(aIPCRequest->headers(), aIPCRequest->headersGuard());

if (mBodyStream) {
aAutoStream.reset(new mozilla::ipc::AutoIPCStream(aIPCRequest->body()));
DebugOnly<bool> ok = aAutoStream->Serialize(mBodyStream, aManager);
MOZ_ASSERT(ok);
}

aIPCRequest->bodySize() = mBodyLength;
aIPCRequest->preferredAlternativeDataType() = mPreferredAlternativeDataType;
aIPCRequest->contentPolicyType() = static_cast<uint32_t>(mContentPolicyType);
aIPCRequest->referrer() = mReferrer;
aIPCRequest->referrerPolicy() = mReferrerPolicy;
aIPCRequest->requestMode() = mMode;
aIPCRequest->requestCredentials() = mCredentialsMode;
aIPCRequest->cacheMode() = mCacheMode;
aIPCRequest->requestRedirect() = mRedirectMode;
aIPCRequest->integrity() = mIntegrity;
aIPCRequest->fragment() = mFragment;

if (mPrincipalInfo) {
aIPCRequest->principalInfo().emplace(*mPrincipalInfo);
// This constructor is (currently) only used for parent -> child communication
// (constructed on the child side).
if (body) {
MOZ_ASSERT(body->type() == BodyStreamVariant::TParentToChildStream);
mBodyStream = static_cast<IPCBlobInputStreamChild*>(
body->get_ParentToChildStream().actorChild())
->CreateStream();
}
}

InternalRequest::~InternalRequest() = default;

void InternalRequest::SetContentPolicyType(
nsContentPolicyType aContentPolicyType) {
mContentPolicyType = aContentPolicyType;
Expand Down
5 changes: 0 additions & 5 deletions dom/fetch/InternalRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ namespace mozilla {

namespace ipc {
class PrincipalInfo;
class AutoIPCStream;
} // namespace ipc

namespace dom {
Expand Down Expand Up @@ -90,10 +89,6 @@ class InternalRequest final {

explicit InternalRequest(const IPCInternalRequest& aIPCRequest);

template <typename M>
void ToIPC(IPCInternalRequest* aIPCRequest, M* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoStream);

already_AddRefed<InternalRequest> Clone();

void GetMethod(nsCString& aMethod) const { aMethod.Assign(mMethod); }
Expand Down
88 changes: 53 additions & 35 deletions dom/fetch/InternalResponse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "mozilla/Assertions.h"
#include "mozilla/RefPtr.h"
#include "mozilla/dom/FetchTypes.h"
#include "mozilla/dom/IPCBlobInputStreamStorage.h"
#include "mozilla/dom/InternalHeaders.h"
#include "mozilla/dom/cache/CacheTypes.h"
#include "mozilla/ipc/PBackgroundSharedTypes.h"
Expand All @@ -26,6 +27,22 @@ namespace {
// XXX This will be tweaked to something more meaningful in Bug 1383656.
const uint32_t kMaxRandomNumber = 102400;

nsCOMPtr<nsIInputStream> TakeStreamFromStorage(
const BodyStreamVariant& aVariant, int64_t aBodySize) {
MOZ_ASSERT(aVariant.type() == BodyStreamVariant::TParentToParentStream);
const auto& uuid = aVariant.get_ParentToParentStream().uuid();

IPCBlobInputStreamStorage* storage = IPCBlobInputStreamStorage::Get();

nsCOMPtr<nsIInputStream> body;
storage->GetStream(uuid, 0, aBodySize, getter_AddRefs(body));
MOZ_DIAGNOSTIC_ASSERT(body);

storage->ForgetStream(uuid);

return body;
}

} // namespace

InternalResponse::InternalResponse(uint16_t aStatus,
Expand Down Expand Up @@ -53,15 +70,20 @@ InternalResponse::InternalResponse(uint16_t aStatus,
response->mHeaders =
new InternalHeaders(aIPCResponse.headers(), aIPCResponse.headersGuard());

nsCOMPtr<nsIInputStream> body =
mozilla::ipc::DeserializeIPCStream(aIPCResponse.body());
response->SetBody(body, aIPCResponse.bodySize());
if (aIPCResponse.body()) {
auto bodySize = aIPCResponse.bodySize();
nsCOMPtr<nsIInputStream> body =
TakeStreamFromStorage(*aIPCResponse.body(), bodySize);
response->SetBody(body, bodySize);
}

response->SetAlternativeDataType(aIPCResponse.alternativeDataType());

nsCOMPtr<nsIInputStream> alternativeBody =
mozilla::ipc::DeserializeIPCStream(aIPCResponse.alternativeBody());
response->SetAlternativeBody(alternativeBody);
if (aIPCResponse.alternativeBody()) {
nsCOMPtr<nsIInputStream> alternativeBody = TakeStreamFromStorage(
*aIPCResponse.alternativeBody(), UNKNOWN_BODY_SIZE);
response->SetAlternativeBody(alternativeBody);
}

response->InitChannelInfo(aIPCResponse.channelInfo());

Expand Down Expand Up @@ -96,54 +118,50 @@ InternalResponse::InternalResponse(uint16_t aStatus,

InternalResponse::~InternalResponse() = default;

template void InternalResponse::ToIPC<mozilla::ipc::PBackgroundChild>(
IPCInternalResponse* aIPCResponse, mozilla::ipc::PBackgroundChild* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoBodyStream,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoAlternativeBodyStream);

template <typename M>
void InternalResponse::ToIPC(
IPCInternalResponse* aIPCResponse, M* aManager,
IPCInternalResponse* aIPCResponse, mozilla::ipc::PBackgroundChild* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoBodyStream,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoAlternativeBodyStream) {
MOZ_ASSERT(aIPCResponse);

aIPCResponse->type() = mType;
GetUnfilteredURLList(aIPCResponse->urlList());
aIPCResponse->status() = GetUnfilteredStatus();
aIPCResponse->statusText() = GetUnfilteredStatusText();
UnfilteredHeaders()->ToIPC(aIPCResponse->headers(),
aIPCResponse->headersGuard());
nsTArray<HeadersEntry> headers;
HeadersGuardEnum headersGuard;
UnfilteredHeaders()->ToIPC(headers, headersGuard);

Maybe<mozilla::ipc::PrincipalInfo> principalInfo =
mPrincipalInfo ? Some(*mPrincipalInfo) : Nothing();

// Note: all the arguments are copied rather than moved, which would be more
// efficient, because there's no move-friendly constructor generated.
*aIPCResponse =
IPCInternalResponse(mType, GetUnfilteredURLList(), GetUnfilteredStatus(),
GetUnfilteredStatusText(), headersGuard, headers,
Nothing(), static_cast<uint64_t>(UNKNOWN_BODY_SIZE),
mErrorCode, GetAlternativeDataType(), Nothing(),
mChannelInfo.AsIPCChannelInfo(), principalInfo);

nsCOMPtr<nsIInputStream> body;
int64_t bodySize;
GetUnfilteredBody(getter_AddRefs(body), &bodySize);

if (body) {
aAutoBodyStream.reset(
new mozilla::ipc::AutoIPCStream(aIPCResponse->body()));
aIPCResponse->body().emplace(ChildToParentStream());
aIPCResponse->bodySize() = bodySize;

aAutoBodyStream.reset(new mozilla::ipc::AutoIPCStream(
aIPCResponse->body()->get_ChildToParentStream().stream()));
DebugOnly<bool> ok = aAutoBodyStream->Serialize(body, aManager);
MOZ_ASSERT(ok);
}

aIPCResponse->bodySize() = bodySize;
aIPCResponse->errorCode() = mErrorCode;
aIPCResponse->alternativeDataType() = GetAlternativeDataType();

nsCOMPtr<nsIInputStream> alternativeBody = TakeAlternativeBody();
if (alternativeBody) {
aAutoAlternativeBodyStream.reset(
new mozilla::ipc::AutoIPCStream(aIPCResponse->alternativeBody()));
aIPCResponse->alternativeBody().emplace(ChildToParentStream());

aAutoAlternativeBodyStream.reset(new mozilla::ipc::AutoIPCStream(
aIPCResponse->alternativeBody()->get_ChildToParentStream().stream()));
DebugOnly<bool> ok =
aAutoAlternativeBodyStream->Serialize(alternativeBody, aManager);
MOZ_ASSERT(ok);
}

aIPCResponse->channelInfo() = mChannelInfo.AsIPCChannelInfo();

if (mPrincipalInfo) {
aIPCResponse->principalInfo().emplace(*mPrincipalInfo);
}
}

already_AddRefed<InternalResponse> InternalResponse::Clone(
Expand Down
14 changes: 11 additions & 3 deletions dom/fetch/InternalResponse.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@

namespace mozilla {
namespace ipc {
class PrincipalInfo;
class AutoIPCStream;
class PBackgroundChild;
class PrincipalInfo;
} // namespace ipc

namespace dom {
Expand All @@ -42,9 +43,10 @@ class InternalResponse final {
static RefPtr<InternalResponse> FromIPC(
const IPCInternalResponse& aIPCResponse);

template <typename M>
// Note: the AutoIPCStreams must outlive the IPCInternalResponse.
void ToIPC(
IPCInternalResponse* aIPCResponse, M* aManager,
IPCInternalResponse* aIPCResponse,
mozilla::ipc::PBackgroundChild* aManager,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoBodyStream,
UniquePtr<mozilla::ipc::AutoIPCStream>& aAutoAlternativeBodyStream);

Expand Down Expand Up @@ -111,6 +113,12 @@ class InternalResponse final {
return GetURLList(aURLList);
}

nsTArray<nsCString> GetUnfilteredURLList() const {
nsTArray<nsCString> list;
GetUnfilteredURLList(list);
return list;
}

void SetURLList(const nsTArray<nsCString>& aURLList) {
mURLList.Assign(aURLList);

Expand Down
4 changes: 0 additions & 4 deletions dom/file/ipc/IPCBlobInputStreamStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ NS_INTERFACE_MAP_END
NS_IMPL_ADDREF(IPCBlobInputStreamStorage)
NS_IMPL_RELEASE(IPCBlobInputStreamStorage)

IPCBlobInputStreamStorage::IPCBlobInputStreamStorage() = default;

IPCBlobInputStreamStorage::~IPCBlobInputStreamStorage() = default;

/* static */
IPCBlobInputStreamStorage* IPCBlobInputStreamStorage::Get() { return gStorage; }

Expand Down
4 changes: 2 additions & 2 deletions dom/file/ipc/IPCBlobInputStreamStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class IPCBlobInputStreamStorage final : public nsIObserver {
const nsID& aID);

private:
IPCBlobInputStreamStorage();
~IPCBlobInputStreamStorage();
IPCBlobInputStreamStorage() = default;
~IPCBlobInputStreamStorage() = default;

struct StreamData {
nsCOMPtr<nsIInputStream> mInputStream;
Expand Down
8 changes: 8 additions & 0 deletions dom/file/ipc/IPCBlobUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ nsresult SerializeInputStream(nsIInputStream* aInputStream, uint64_t aSize,
aActorParent, aManager);
}

nsresult SerializeInputStream(nsIInputStream* aInputStream, uint64_t aSize,
PIPCBlobInputStreamParent*& aActorParent,
PBackgroundParent* aManager) {
return SerializeInputStreamParent(aInputStream, aSize,
BackgroundParent::GetChildID(aManager),
aActorParent, aManager);
}

nsresult SerializeInputStream(nsIInputStream* aInputStream, uint64_t aSize,
IPCBlobStream& aIPCBlob,
ContentParent* aManager) {
Expand Down
4 changes: 4 additions & 0 deletions dom/file/ipc/IPCBlobUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ nsresult SerializeInputStream(nsIInputStream* aInputStream, uint64_t aSize,
PIPCBlobInputStreamParent*& aActorParent,
ContentParent* aManager);

nsresult SerializeInputStream(nsIInputStream* aInputStream, uint64_t aSize,
PIPCBlobInputStreamParent*& aActorParent,
mozilla::ipc::PBackgroundParent* aManager);

// WARNING: If you pass any actor which does not have P{Content,Background} as
// its toplevel protocol, this method will MOZ_CRASH.
nsresult SerializeUntyped(BlobImpl* aBlobImpl, mozilla::ipc::IProtocol* aActor,
Expand Down
Loading

0 comments on commit 1ad4e03

Please sign in to comment.