Bug 1500219: Part 1 - Add ability to send and receive UniquePtr<T> with IPDL (r=jld)
authorDavid Parks <dparks@mozilla.com>
Fri, 26 Oct 2018 17:09:19 +0000
changeset 443207 a887c9b3dad3dd4d37cb0fb14d7aa15fc9d24118
parent 443206 fda8140290255a6eb2654e1f282173eab4f1296c
child 443208 c71388ece3975bb6b814f2a4d16ca15315384441
push id34944
push userncsoregi@mozilla.com
push dateSat, 27 Oct 2018 09:49:55 +0000
treeherdermozilla-central@49d47a692ca4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1500219
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1500219: Part 1 - Add ability to send and receive UniquePtr<T> with IPDL (r=jld) This patch adds the ability to use UniquePtr<T> in IPDL messages if T is serializable. The behavior works as expected -- a UniquePtr in a Send is cleared and is passed by move. Some design points: * The UniquePtr identification is done in the parser. This is because the parser immediately normalizes CxxTemplateInst -- an old version of the patch did hacky string-parsing to pull it apart. I, instead, created CxxUniquePtrInst. * The implementation allows passing to Send... by move or by reference. This is valid UniquePtr API behavior but passing by reference is not really useful in this case (but not harmful). This could probably piggy-back on the "moveonly" IPDL work but that is newer than this work and will require some refactoring. Differential Revision: https://phabricator.services.mozilla.com/D9143
ipc/glue/IPDLParamTraits.h
ipc/ipdl/ipdl/ast.py
ipc/ipdl/ipdl/builtin.py
ipc/ipdl/ipdl/lower.py
ipc/ipdl/ipdl/parser.py
ipc/ipdl/ipdl/type.py
--- a/ipc/glue/IPDLParamTraits.h
+++ b/ipc/glue/IPDLParamTraits.h
@@ -1,12 +1,13 @@
 #ifndef mozilla_ipc_IPDLParamTraits_h
 #define mozilla_ipc_IPDLParamTraits_h
 
 #include "chrome/common/ipc_message_utils.h"
+#include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 namespace ipc {
 
 class IProtocol;
 
 //
 // IPDLParamTraits are an extended version of ParamTraits. Unlike ParamTraits,
@@ -157,12 +158,51 @@ private:
   // call, rather than writing each element individually.  We deliberately do
   // not use mozilla::IsPod here because it is perfectly reasonable to have
   // a data structure T for which IsPod<T>::value is true, yet also have a
   // {IPDL,}ParamTraits<T> specialization.
   static const bool sUseWriteBytes = (mozilla::IsIntegral<T>::value ||
                                       mozilla::IsFloatingPoint<T>::value);
 };
 
+template<typename T>
+struct IPDLParamTraits<mozilla::UniquePtr<T>>
+{
+  typedef mozilla::UniquePtr<T> paramType;
+
+  // Allow UniquePtr<T>& and UniquePtr<T>&&
+  template<typename ParamTypeRef>
+  static void Write(IPC::Message* aMsg, IProtocol* aActor, ParamTypeRef&& aParam)
+  {
+    // write bool true if inner object is null
+    WriteParam(aMsg, aParam == nullptr);
+    if (aParam) {
+      WriteIPDLParam(aMsg, aActor, *aParam.get());
+      aParam = nullptr;
+    }
+  }
+
+  static bool Read(const IPC::Message* aMsg, PickleIterator* aIter,
+                   IProtocol* aActor, paramType* aResult)
+  {
+    MOZ_ASSERT(aResult);
+    bool isNull;
+    *aResult = nullptr;
+    if (!ReadParam(aMsg, aIter, &isNull)) {
+      return false;
+    }
+    if (isNull) {
+      return true;
+    }
+    T* obj = new T();
+    if (!ReadIPDLParam(aMsg, aIter, aActor, obj)) {
+      delete obj;
+      return false;
+    }
+    aResult->reset(obj);
+    return true;
+  }
+};
+
 } // namespace ipc
 } // namespace mozilla
 
 #endif // defined(mozilla_ipc_IPDLParamTraits_h)
--- a/ipc/ipdl/ipdl/ast.py
+++ b/ipc/ipdl/ipdl/ast.py
@@ -323,16 +323,17 @@ class Param(Node):
 
 
 class TypeSpec(Node):
     def __init__(self, loc, spec):
         Node.__init__(self, loc)
         self.spec = spec                # QualifiedId
         self.array = 0                  # bool
         self.nullable = 0               # bool
+        self.uniqueptr = 0              # bool
 
     def basename(self):
         return self.spec.baseid
 
     def __str__(self): return str(self.spec)
 
 
 class QualifiedId:              # FIXME inherit from node?
--- a/ipc/ipdl/ipdl/builtin.py
+++ b/ipc/ipdl/ipdl/builtin.py
@@ -35,28 +35,30 @@ Types = (
     # Mozilla types: "less" standard things we know how serialize/deserialize
     'nsresult',
     'nsString',
     'nsCString',
     'nsDependentSubstring',
     'nsDependentCSubstring',
     'mozilla::ipc::Shmem',
     'mozilla::ipc::ByteBuf',
+    'mozilla::UniquePtr',
     'mozilla::ipc::FileDescriptor'
 )
 
 
 HeaderIncludes = (
     'mozilla/Attributes.h',
     'IPCMessageStart.h',
     'ipc/IPCMessageUtils.h',
     'mozilla/RefPtr.h',
     'nsString.h',
     'nsTArray.h',
     'mozilla/ipc/ProtocolUtils.h',
     'nsTHashtable.h',
     'mozilla/OperatorNewExtensions.h',
+    'mozilla/UniquePtr.h',
 )
 
 CppIncludes = (
     'nsIFile.h',
     'GeckoProfiler.h',
 )
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -558,16 +558,19 @@ class _ConvertToCxxType(TypeVisitor):
         return Type(self.typename(s))
 
     def visitFDType(self, s):
         return Type(self.typename(s))
 
     def visitEndpointType(self, s):
         return Type(self.typename(s))
 
+    def visitUniquePtrType(self, s):
+        return Type(self.typename(s))
+
     def visitProtocolType(self, p): assert 0
 
     def visitMessageType(self, m): assert 0
 
     def visitVoidType(self, v): assert 0
 
     def visitStateType(self, st): assert 0
 
@@ -601,27 +604,35 @@ def _cxxConstRefType(ipdltype, side):
     if ipdltype.isCxx() and ipdltype.isMoveonly():
         t.ref = 1
         return t
     if ipdltype.isCxx() and ipdltype.isRefcounted():
         # Use T* instead of const RefPtr<T>&
         t = t.T
         t.ptr = 1
         return t
+    if ipdltype.isUniquePtr():
+        t.ref = 1
+        return t
     t.const = 1
     t.ref = 1
     return t
 
 
+def _cxxTypeCanMoveSend(ipdltype):
+    return ipdltype.isUniquePtr()
+
+
 def _cxxTypeNeedsMove(ipdltype):
     return ((ipdltype.isIPDL() and (ipdltype.isArray() or
                                     ipdltype.isShmem() or
                                     ipdltype.isByteBuf() or
                                     ipdltype.isEndpoint())) or
-            (ipdltype.isCxx() and ipdltype.isMoveonly()))
+            (ipdltype.isCxx() and ipdltype.isMoveonly())
+            or ipdltype.isUniquePtr())
 
 
 def _cxxTypeCanMove(ipdltype):
     return not (ipdltype.isIPDL() and ipdltype.isActor())
 
 
 def _cxxMoveRefType(ipdltype, side):
     t = _cxxBareType(ipdltype, side)
@@ -1404,16 +1415,18 @@ with some new IPDL/C++ nodes that are tu
                                Typedef(Type('mozilla::ipc::ProtocolId'),
                                        'ProtocolId'),
                                Typedef(Type('mozilla::ipc::Transport'),
                                        'Transport'),
                                Typedef(Type('mozilla::ipc::Endpoint'),
                                        'Endpoint', ['FooSide']),
                                Typedef(Type('mozilla::ipc::TransportDescriptor'),
                                        'TransportDescriptor'),
+                               Typedef(Type('mozilla::UniquePtr'),
+                                       'UniquePtr', ['T']),
                                Typedef(Type('mozilla::ipc::ResponseRejectReason'),
                                        'ResponseRejectReason')])
         self.protocolName = None
 
     def visitTranslationUnit(self, tu):
         if tu not in self.visitedTus:
             self.visitedTus.add(tu)
             ipdl.ast.Visitor.visitTranslationUnit(self, tu)
@@ -2266,16 +2279,21 @@ before this struct.  Some types generate
         self.maybeTypedef('mozilla::ipc::ByteBuf', 'ByteBuf')
 
     def visitFDType(self, s):
         if s in self.visited:
             return
         self.visited.add(s)
         self.maybeTypedef('mozilla::ipc::FileDescriptor', 'FileDescriptor')
 
+    def visitUniquePtrType(self, s):
+        if s in self.visited:
+            return
+        self.visited.add(s)
+
     def visitVoidType(self, v): assert 0
 
     def visitMessageType(self, v): assert 0
 
     def visitProtocolType(self, v): assert 0
 
     def visitStateType(self, v): assert 0
 
@@ -3067,17 +3085,17 @@ class _GenerateProtocolActorCode(ipdl.as
         ])
 
         cf.addthing(traitsdefn)
 
     def visitUsingStmt(self, using):
         if using.header is None:
             return
 
-        if using.canBeForwardDeclared():
+        if using.canBeForwardDeclared() and not using.decl.type.isUniquePtr():
             spec = using.type.spec
 
             self.usingDecls.extend([
                 _makeForwardDeclForQClass(spec.baseid, spec.quals,
                                           cls=using.isClass(),
                                           struct=using.isStruct()),
                 Whitespace.NL
             ])
@@ -3818,16 +3836,17 @@ class _GenerateProtocolActorCode(ipdl.as
     # serialization/deserialization and dispatching handlers for
     # received messages.
     ##
     def visitMessageDecl(self, md):
         isctor = md.decl.type.isCtor()
         isdtor = md.decl.type.isDtor()
         decltype = md.decl.type
         sendmethod = None
+        movesendmethod = None
         promisesendmethod = None
         recvlbl, recvcase = None, None
 
         def addRecvCase(lbl, case):
             if decltype.isAsync():
                 self.asyncSwitch.addcase(lbl, case)
             elif decltype.isSync():
                 self.syncSwitch.addcase(lbl, case)
@@ -3846,27 +3865,29 @@ class _GenerateProtocolActorCode(ipdl.as
                 sendmethod, (recvlbl, recvcase) = self.genAsyncCtor(md)
             elif isctor:
                 sendmethod = self.genBlockingCtorMethod(md)
             elif isdtor and isasync:
                 sendmethod, (recvlbl, recvcase) = self.genAsyncDtor(md)
             elif isdtor:
                 sendmethod = self.genBlockingDtorMethod(md)
             elif isasync:
-                sendmethod, promisesendmethod, (recvlbl, recvcase) = \
+                sendmethod, movesendmethod, promisesendmethod, (recvlbl, recvcase) = \
                     self.genAsyncSendMethod(md)
             else:
-                sendmethod = self.genBlockingSendMethod(md)
+                sendmethod, movesendmethod = self.genBlockingSendMethod(md)
 
         # XXX figure out what to do here
         if isdtor and md.decl.type.constructedType().isToplevel():
             sendmethod = None
 
         if sendmethod is not None:
             self.cls.addstmts([sendmethod, Whitespace.NL])
+        if movesendmethod is not None:
+            self.cls.addstmts([movesendmethod, Whitespace.NL])
         if promisesendmethod is not None:
             self.cls.addstmts([promisesendmethod, Whitespace.NL])
         if recvcase is not None:
             addRecvCase(recvlbl, recvcase)
             recvlbl, recvcase = None, None
 
         if self.receivesMessage(md):
             if isctor:
@@ -4113,39 +4134,57 @@ class _GenerateProtocolActorCode(ipdl.as
         ifresolve.addelsestmts(desrej)
         ifresolve.addelsestmts(rejectcallback)
         case.addstmts(prologue)
         case.addstmts(getcallback)
         case.addstmt(ifresolve)
         case.addstmt(StmtReturn(_Result.Processed))
         return (lbl, case)
 
+    @staticmethod
+    def hasMoveableParams(md):
+        for param in md.decl.type.params:
+            if _cxxTypeCanMoveSend(param):
+                return True
+        return False
+
     def genAsyncSendMethod(self, md):
         method = MethodDefn(self.makeSendMethodDecl(md))
         msgvar, stmts = self.makeMessage(md, errfnSend)
         retvar, sendstmts = self.sendAsync(md, msgvar)
 
         method.addstmts(stmts
                         + [Whitespace.NL]
                         + self.genVerifyMessage(md.decl.type.verify, md.params,
                                                 errfnSend, ExprVar('msg__'))
                         + sendstmts
                         + [StmtReturn(retvar)])
 
+        if self.hasMoveableParams(md):
+            movemethod = MethodDefn(self.makeSendMethodDecl(md, paramsems='move'))
+            movemethod.addstmts(stmts
+                                + [Whitespace.NL]
+                                + self.genVerifyMessage(md.decl.type.verify, md.params,
+                                                        errfnSend, ExprVar('msg__'))
+                                + sendstmts
+                                + [StmtReturn(retvar)])
+        else:
+            movemethod = None
+
         # Add the promise overload if we need one.
         if md.returns:
             promisemethod = MethodDefn(self.makeSendMethodDecl(md, promise=True))
             stmts = self.sendAsyncWithPromise(md)
             promisemethod.addstmts(stmts)
 
             (lbl, case) = self.genRecvAsyncReplyCase(md)
         else:
             (promisemethod, lbl, case) = (None, None, None)
 
-        return method, promisemethod, (lbl, case)
+        return method, movemethod, promisemethod, (lbl, case)
 
     def genBlockingSendMethod(self, md, fromActor=None):
         method = MethodDefn(self.makeSendMethodDecl(md))
 
         msgvar, serstmts = self.makeMessage(md, errfnSend, fromActor)
         replyvar = self.replyvar
 
         sendok, sendstmts = self.sendBlocking(md, msgvar, replyvar)
@@ -4162,17 +4201,33 @@ class _GenerateProtocolActorCode(ipdl.as
             + [Whitespace.NL,
                 StmtDecl(Decl(Type('Message'), replyvar.name))]
             + sendstmts
             + [failif]
             + desstmts
             + [Whitespace.NL,
                 StmtReturn.TRUE])
 
-        return method
+        if self.hasMoveableParams(md):
+            movemethod = MethodDefn(self.makeSendMethodDecl(md, paramsems='move'))
+            movemethod.addstmts(
+                serstmts
+                + self.genVerifyMessage(md.decl.type.verify, md.params, errfnSend,
+                                        ExprVar('msg__'))
+                + [Whitespace.NL,
+                    StmtDecl(Decl(Type('Message'), replyvar.name))]
+                + sendstmts
+                + [failif]
+                + desstmts
+                + [Whitespace.NL,
+                    StmtReturn.TRUE])
+        else:
+            movemethod = None
+
+        return method, movemethod
 
     def genCtorRecvCase(self, md):
         lbl = CaseLabel(md.pqMsgId())
         case = StmtBlock()
         actorvar = md.actorDecl().var()
         actorhandle = self.handlevar
 
         stmts = self.deserializeMessage(md, self.side, errfnRecv,
@@ -4690,32 +4745,32 @@ class _GenerateProtocolActorCode(ipdl.as
         ])
         return [failif]
 
     def makeDtorMethodDecl(self, md):
         decl = self.makeSendMethodDecl(md)
         decl.methodspec = MethodSpec.STATIC
         return decl
 
-    def makeSendMethodDecl(self, md, promise=False):
+    def makeSendMethodDecl(self, md, promise=False, paramsems='in'):
         implicit = md.decl.type.hasImplicitActorParam()
         if md.decl.type.isAsync() and md.returns:
             if promise:
                 returnsems = 'promise'
                 rettype = _refptr(Type(md.promiseName()))
             else:
                 returnsems = 'callback'
                 rettype = Type.VOID
         else:
             assert not promise
             returnsems = 'out'
             rettype = Type.BOOL
         decl = MethodDecl(
             md.sendMethod().name,
-            params=md.makeCxxParams(paramsems='in', returnsems=returnsems,
+            params=md.makeCxxParams(paramsems, returnsems=returnsems,
                                     side=self.side, implicit=implicit),
             warn_unused=(self.side == 'parent' and returnsems != 'callback'),
             ret=rettype)
         if md.decl.type.isCtor():
             decl.ret = md.actorDecl().bareType(self.side)
         return decl
 
     def logMessage(self, md, msgptr, pfx, actor=None, receiving=False):
--- a/ipc/ipdl/ipdl/parser.py
+++ b/ipc/ipdl/ipdl/parser.py
@@ -132,16 +132,17 @@ reserved = set((
     'prio',
     'protocol',
     'refcounted',
     'moveonly',
     'returns',
     'struct',
     'sync',
     'union',
+    'UniquePtr',
     'upto',
     'using',
     'verify'))
 tokens = [
     'COLONCOLON', 'ID', 'STRING',
 ] + [r.upper() for r in reserved]
 
 t_COLONCOLON = '::'
@@ -662,22 +663,28 @@ def p_Type(p):
     """Type : MaybeNullable BasicType"""
     # only actor types are nullable; we check this in the type checker
     p[2].nullable = p[1]
     p[0] = p[2]
 
 
 def p_BasicType(p):
     """BasicType : CxxID
-                 | CxxID '[' ']'"""
+                 | CxxID '[' ']'
+                 | CxxUniquePtrInst"""
     # ID == CxxType; we forbid qnames here,
     # in favor of the |using| declaration
     if not isinstance(p[1], TypeSpec):
-        loc, id = p[1]
+        assert (len(p[1]) == 2) or (len(p[1]) == 3)
+        if 2 == len(p[1]):
+            # p[1] is CxxID. isunique = 0
+            p[1] = p[1] + (0,)
+        loc, id, isunique = p[1]
         p[1] = TypeSpec(loc, QualifiedId(loc, id))
+        p[1].uniqueptr = isunique
     if 4 == len(p):
         p[1].array = 1
     p[0] = p[1]
 
 
 def p_MaybeNullable(p):
     """MaybeNullable : NULLABLE
                      | """
@@ -719,12 +726,17 @@ def p_CxxID(p):
         p[0] = (locFromTok(p, 1), str(p[1]))
 
 
 def p_CxxTemplateInst(p):
     """CxxTemplateInst : ID '<' ID '>'"""
     p[0] = (locFromTok(p, 1), str(p[1]) + '<' + str(p[3]) + '>')
 
 
+def p_CxxUniquePtrInst(p):
+    """CxxUniquePtrInst : UNIQUEPTR '<' ID '>'"""
+    p[0] = (locFromTok(p, 1), str(p[3]), 1)
+
+
 def p_error(t):
     lineno, value = _safeLinenoValue(t)
     _error(Loc(Parser.current.filename, lineno),
            "bad syntax near `%s'", value)
--- a/ipc/ipdl/ipdl/type.py
+++ b/ipc/ipdl/ipdl/type.py
@@ -75,16 +75,19 @@ class TypeVisitor:
         c.shmem.accept(self)
 
     def visitFDType(self, s, *args):
         pass
 
     def visitEndpointType(self, s, *args):
         pass
 
+    def visitUniquePtrType(self, s, *args):
+        pass
+
 
 class Type:
     def __cmp__(self, o):
         return cmp(self.fullname(), o.fullname())
 
     def __eq__(self, o):
         return (self.__class__ == o.__class__
                 and self.fullname() == o.fullname())
@@ -99,16 +102,19 @@ class Type:
 
     def isIPDL(self):
         return False
     # Is this type neither compound nor an array?
 
     def isAtom(self):
         return False
 
+    def isUniquePtr(self):
+        return False
+
     def typename(self):
         return self.__class__.__name__
 
     def name(self): raise Exception, 'NYI'
 
     def fullname(self): raise Exception, 'NYI'
 
     def accept(self, visitor, *args):
@@ -486,16 +492,29 @@ class EndpointType(IPDLType):
 
     def name(self):
         return self.qname.baseid
 
     def fullname(self):
         return str(self.qname)
 
 
+class UniquePtrType(Type):
+    def __init__(self, innertype):
+        self.innertype = innertype
+
+    def isUniquePtr(self): return True
+
+    def name(self):
+        return 'UniquePtr<' + self.innertype.fullname() + '>'
+
+    def fullname(self):
+        return 'mozilla::UniquePtr<' + self.innertype.fullname() + '>'
+
+
 def iteractortypes(t, visited=None):
     """Iterate over any actor(s) buried in |type|."""
     if visited is None:
         visited = set()
 
     # XXX |yield| semantics makes it hard to use TypeVisitor
     if not t.isIPDL():
         return
@@ -838,17 +857,20 @@ class GatherDecls(TcheckVisitor):
             if cdecl is None:
                 self.error(c.loc, "unknown component type `%s' of union `%s'",
                            str(c), ud.name)
                 continue
             utype.components.append(self._canonicalType(cdecl.type, c))
 
     def visitUsingStmt(self, using):
         fullname = str(using.type)
-        if using.type.basename() == fullname:
+        if (using.type.basename() == fullname) or using.type.uniqueptr:
+            # Prevent generation of typedefs.  If basename == fullname then
+            # there is nothing to typedef.  With UniquePtrs, basenames
+            # are generic so typedefs would be illegal.
             fullname = None
         if fullname == 'mozilla::ipc::Shmem':
             ipdltype = ShmemType(using.type.spec)
         elif fullname == 'mozilla::ipc::ByteBuf':
             ipdltype = ByteBufType(using.type.spec)
         elif fullname == 'mozilla::ipc::FileDescriptor':
             ipdltype = FDType(using.type.spec)
         else:
@@ -1046,16 +1068,19 @@ class GatherDecls(TcheckVisitor):
             self.error(
                 loc,
                 "`nullable' qualifier for type `%s' makes no sense",
                 itype.name())
 
         if typespec.array:
             itype = ArrayType(itype)
 
+        if typespec.uniqueptr:
+            itype = UniquePtrType(itype)
+
         return itype
 
 
 # -----------------------------------------------------------------------------
 
 def checkcycles(p, stack=None):
     cycles = []