From 406f5cc2d8a1a1c8803d6152e1d4f5a3afc35059 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Fri, 24 May 2019 20:21:35 +0000 Subject: [PATCH] Bug 1553270 - Part 1: Store the protocol ID on IProtocol directly, r=froydnj This allows for the getter to be used in IProtocol's destructor, and generally brings IProtocol more in line with IToplevelProtocol. Differential Revision: https://phabricator.services.mozilla.com/D32042 --HG-- extra : moz-landing-system : lando --- dom/file/ipc/IPCBlobUtils.cpp | 2 +- dom/ipc/WindowGlobalParent.cpp | 4 ++-- dom/plugins/ipc/PluginModuleParent.cpp | 2 +- ipc/glue/IPCStreamUtils.cpp | 2 +- ipc/glue/InProcessImpl.cpp | 4 ++-- ipc/glue/ProtocolUtils.cpp | 5 ++--- ipc/glue/ProtocolUtils.h | 19 +++++++++---------- ipc/ipdl/ipdl/lower.py | 10 ++-------- toolkit/recordreplay/ipc/ParentForwarding.cpp | 4 ---- 9 files changed, 20 insertions(+), 32 deletions(-) diff --git a/dom/file/ipc/IPCBlobUtils.cpp b/dom/file/ipc/IPCBlobUtils.cpp index a6973b6b67e1..cb8cda8959e9 100644 --- a/dom/file/ipc/IPCBlobUtils.cpp +++ b/dom/file/ipc/IPCBlobUtils.cpp @@ -261,7 +261,7 @@ nsresult SerializeUntyped(BlobImpl* aBlobImpl, IProtocol* aActor, } // We always need the toplevel protocol - switch (manager->GetProtocolTypeId()) { + switch (manager->GetProtocolId()) { case PBackgroundMsgStart: if (manager->GetSide() == mozilla::ipc::ParentSide) { return SerializeInternal( diff --git a/dom/ipc/WindowGlobalParent.cpp b/dom/ipc/WindowGlobalParent.cpp index cc96fab06430..d151045022ad 100644 --- a/dom/ipc/WindowGlobalParent.cpp +++ b/dom/ipc/WindowGlobalParent.cpp @@ -98,7 +98,7 @@ void WindowGlobalParent::Init(const WindowGlobalInit& aInit) { if (mInProcess) { // In the in-process case, we can get it from the other side's // WindowGlobalChild. - MOZ_ASSERT(Manager()->GetProtocolTypeId() == PInProcessMsgStart); + MOZ_ASSERT(Manager()->GetProtocolId() == PInProcessMsgStart); RefPtr otherSide = GetChildActor(); if (otherSide && otherSide->WindowGlobal()) { // Get the toplevel window from the other side. @@ -110,7 +110,7 @@ void WindowGlobalParent::Init(const WindowGlobalInit& aInit) { } } else { // In the cross-process case, we can get the frame element from our manager. - MOZ_ASSERT(Manager()->GetProtocolTypeId() == PBrowserMsgStart); + MOZ_ASSERT(Manager()->GetProtocolId() == PBrowserMsgStart); frameElement = static_cast(Manager())->GetOwnerElement(); } diff --git a/dom/plugins/ipc/PluginModuleParent.cpp b/dom/plugins/ipc/PluginModuleParent.cpp index 9afd2033e2f8..7020b7fc97f0 100644 --- a/dom/plugins/ipc/PluginModuleParent.cpp +++ b/dom/plugins/ipc/PluginModuleParent.cpp @@ -885,7 +885,7 @@ PluginInstanceParent* PluginModuleChromeParent::GetManagingInstance( mozilla::ipc::IProtocol* aProtocol) { MOZ_ASSERT(aProtocol); mozilla::ipc::IProtocol* listener = aProtocol; - switch (listener->GetProtocolTypeId()) { + switch (listener->GetProtocolId()) { case PPluginInstanceMsgStart: // In this case, aProtocol is the instance itself. Just cast it. return static_cast(aProtocol); diff --git a/ipc/glue/IPCStreamUtils.cpp b/ipc/glue/IPCStreamUtils.cpp index 4f662f61f2bb..cdf9b08b0639 100644 --- a/ipc/glue/IPCStreamUtils.cpp +++ b/ipc/glue/IPCStreamUtils.cpp @@ -459,7 +459,7 @@ void IPDLParamTraits::Write(IPC::Message* aMsg, // protocols we support. IProtocol* actor = aActor; while (!found && actor) { - switch (actor->GetProtocolTypeId()) { + switch (actor->GetProtocolId()) { case PContentMsgStart: if (actor->GetSide() == mozilla::ipc::ParentSide) { ok = autoStream.Serialize( diff --git a/ipc/glue/InProcessImpl.cpp b/ipc/glue/InProcessImpl.cpp index 5742131aece1..042f9dca31e0 100644 --- a/ipc/glue/InProcessImpl.cpp +++ b/ipc/glue/InProcessImpl.cpp @@ -141,7 +141,7 @@ static IProtocol* GetOtherInProcessActor(IProtocol* aActor) { // Discover the manager of aActor which is PInProcess. IProtocol* current = aActor; while (current) { - if (current->GetProtocolTypeId() == PInProcessMsgStart) { + if (current->GetProtocolId() == PInProcessMsgStart) { break; // Found the correct actor. } current = current->Manager(); @@ -173,7 +173,7 @@ static IProtocol* GetOtherInProcessActor(IProtocol* aActor) { if (otherActor) { MOZ_ASSERT(otherActor->GetSide() != UnknownSide, "bad unknown side"); MOZ_ASSERT(otherActor->GetSide() != aActor->GetSide(), "Wrong side!"); - MOZ_ASSERT(otherActor->GetProtocolTypeId() == aActor->GetProtocolTypeId(), + MOZ_ASSERT(otherActor->GetProtocolId() == aActor->GetProtocolId(), "Wrong type of protocol!"); } diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index 85bbfb389469..ef65cf945134 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -406,7 +406,7 @@ Maybe IProtocol::ReadActor(const IPC::Message* aMessage, return Nothing(); } - if (listener->GetProtocolTypeId() != aProtocolTypeId) { + if (listener->GetProtocolId() != aProtocolTypeId) { MismatchedActorTypeError(aActorDescription); return Nothing(); } @@ -702,8 +702,7 @@ void IProtocol::DestroySubtree(ActorDestroyReason aWhy) { IToplevelProtocol::IToplevelProtocol(const char* aName, ProtocolId aProtoId, Side aSide) - : IProtocol(aSide, MakeUnique(aName, this, aSide)), - mProtocolId(aProtoId), + : IProtocol(aProtoId, aSide, MakeUnique(aName, this, aSide)), mOtherPid(mozilla::ipc::kInvalidProcessId), mIsMainThreadProtocol(false) {} diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index 59dcc39c7e9f..eb75547ebb01 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -153,6 +153,8 @@ enum class LinkStatus : uint8_t { Destroyed, }; +typedef IPCMessageStart ProtocolId; + class IToplevelProtocol; class ActorLifecycleProxy; @@ -268,8 +270,8 @@ class IProtocol : public HasResultCodes { typedef IPC::Message Message; typedef IPC::MessageInfo MessageInfo; - explicit IProtocol(Side aSide) - : IProtocol(aSide, MakeUnique(this)) {} + explicit IProtocol(ProtocolId aProtoId, Side aSide) + : IProtocol(aProtoId, aSide, MakeUnique(this)) {} int32_t Register(IProtocol* aRouted) { return mState->Register(aRouted); } int32_t RegisterID(IProtocol* aRouted, int32_t aId) { @@ -330,7 +332,7 @@ class IProtocol : public HasResultCodes { Message*& aReply) = 0; virtual Result OnCallReceived(const Message& aMessage, Message*& aReply) = 0; - virtual int32_t GetProtocolTypeId() = 0; + ProtocolId GetProtocolId() const { return mProtocolId; } int32_t Id() const { return mId; } IProtocol* Manager() const { return mManager; } @@ -364,8 +366,9 @@ class IProtocol : public HasResultCodes { ActorLifecycleProxy* GetLifecycleProxy() { return mLifecycleProxy; } protected: - IProtocol(Side aSide, UniquePtr aState) + IProtocol(ProtocolId aProtoId, Side aSide, UniquePtr aState) : mId(0), + mProtocolId(aProtoId), mSide(aSide), mLinkStatus(LinkStatus::Inactive), mLifecycleProxy(nullptr), @@ -433,7 +436,7 @@ class IProtocol : public HasResultCodes { // The actor has been freed after this method returns. virtual void ActorDealloc() { if (Manager()) { - Manager()->DeallocManagee(GetProtocolTypeId(), this); + Manager()->DeallocManagee(mProtocolId, this); } } @@ -442,6 +445,7 @@ class IProtocol : public HasResultCodes { private: int32_t mId; + ProtocolId mProtocolId; Side mSide; LinkStatus mLinkStatus; ActorLifecycleProxy* mLifecycleProxy; @@ -449,8 +453,6 @@ class IProtocol : public HasResultCodes { UniquePtr mState; }; -typedef IPCMessageStart ProtocolId; - #define IPC_OK() mozilla::ipc::IPCResult::Ok() #define IPC_FAIL(actor, why) \ mozilla::ipc::IPCResult::Fail(WrapNotNull(actor), __func__, (why)) @@ -553,8 +555,6 @@ class IToplevelProtocol : public IProtocol { Transport* GetTransport() const { return mTrans.get(); } - ProtocolId GetProtocolId() const { return mProtocolId; } - base::ProcessId OtherPid() const final; void SetOtherProcessId(base::ProcessId aOtherPid); @@ -684,7 +684,6 @@ class IToplevelProtocol : public IProtocol { private: base::ProcessId OtherPidMaybeInvalid() const; - ProtocolId mProtocolId; UniquePtr mTrans; base::ProcessId mOtherPid; bool mIsMainThreadProtocol; diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index 4d6c495c5be5..ff2e579f37bd 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -3388,7 +3388,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): ] else: ctor.memberinits = [ - ExprMemberInit(ExprVar('mozilla::ipc::IProtocol'), [side]) + ExprMemberInit(ExprVar('mozilla::ipc::IProtocol'), + [_protocolId(ptype), side]) ] ctor.addcode('MOZ_COUNT_CTOR(${clsname});\n', clsname=self.clsname) @@ -3603,13 +3604,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): clearsubtreevar = ExprVar('ClearSubtree') - # int32_t GetProtocolTypeId() { return PFoo; } - gettypetag = MethodDefn( - MethodDecl('GetProtocolTypeId', ret=_actorTypeTagType(), - methodspec=MethodSpec.OVERRIDE)) - gettypetag.addstmt(StmtReturn(_protocolId(ptype))) - self.cls.addstmts([gettypetag, Whitespace.NL]) - if ptype.isToplevel(): # OnChannelClose() onclose = MethodDefn(MethodDecl('OnChannelClose', diff --git a/toolkit/recordreplay/ipc/ParentForwarding.cpp b/toolkit/recordreplay/ipc/ParentForwarding.cpp index 71da45492c76..d3bf3bfae09a 100644 --- a/toolkit/recordreplay/ipc/ParentForwarding.cpp +++ b/toolkit/recordreplay/ipc/ParentForwarding.cpp @@ -340,10 +340,6 @@ class MiddlemanProtocol : public ipc::IToplevelProtocol { return MsgProcessed; } - virtual int32_t GetProtocolTypeId() override { - MOZ_CRASH("MiddlemanProtocol::GetProtocolTypeId"); - } - virtual void OnChannelClose() override { MOZ_RELEASE_ASSERT(mSide == ipc::ChildSide); BeginShutdown();