Bug 1500219: Part 1 - Add ability to send and receive UniquePtr<T> wi…

…th 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

--HG--
extra : moz-landing-system : lando

ipc/glue/IPDLParamTraits.h

@@ -2,6 +2,7 @@
 #define mozilla_ipc_IPDLParamTraits_h
 
 #include "chrome/common/ipc_message_utils.h"
+#include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 namespace ipc {
@@ -162,6 +163,45 @@ struct IPDLParamTraits<nsTArray<T>>
                                       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
 

ipc/ipdl/ipdl/ast.py

@@ -328,6 +328,7 @@ def __init__(self, loc, spec):
         self.spec = spec                # QualifiedId
         self.array = 0                  # bool
         self.nullable = 0               # bool
+        self.uniqueptr = 0              # bool
 
     def basename(self):
         return self.spec.baseid

ipc/ipdl/ipdl/builtin.py

@@ -40,6 +40,7 @@
     'nsDependentCSubstring',
     'mozilla::ipc::Shmem',
     'mozilla::ipc::ByteBuf',
+    'mozilla::UniquePtr',
     'mozilla::ipc::FileDescriptor'
 )
 
@@ -54,6 +55,7 @@
     'mozilla/ipc/ProtocolUtils.h',
     'nsTHashtable.h',
     'mozilla/OperatorNewExtensions.h',
+    'mozilla/UniquePtr.h',
 )
 
 CppIncludes = (

ipc/ipdl/ipdl/lower.py

@@ -563,6 +563,9 @@ def visitFDType(self, 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
@@ -606,17 +609,25 @@ def _cxxConstRefType(ipdltype, side):
         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):
@@ -1409,6 +1420,8 @@ def __init__(self):
                                        'Endpoint', ['FooSide']),
                                Typedef(Type('mozilla::ipc::TransportDescriptor'),
                                        'TransportDescriptor'),
+                               Typedef(Type('mozilla::UniquePtr'),
+                                       'UniquePtr', ['T']),
                                Typedef(Type('mozilla::ipc::ResponseRejectReason'),
                                        'ResponseRejectReason')])
         self.protocolName = None
@@ -2271,6 +2284,11 @@ def visitFDType(self, s):
         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
@@ -3072,7 +3090,7 @@ 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([
@@ -3823,6 +3841,7 @@ def visitMessageDecl(self, md):
         isdtor = md.decl.type.isDtor()
         decltype = md.decl.type
         sendmethod = None
+        movesendmethod = None
         promisesendmethod = None
         recvlbl, recvcase = None, None
 
@@ -3851,17 +3870,19 @@ def addRecvCase(lbl, case):
             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:
@@ -4118,6 +4139,13 @@ def genRecvAsyncReplyCase(self, md):
         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)
@@ -4130,6 +4158,17 @@ def genAsyncSendMethod(self, md):
                         + 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))
@@ -4140,7 +4179,7 @@ def genAsyncSendMethod(self, 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))
@@ -4167,7 +4206,23 @@ def genBlockingSendMethod(self, md, fromActor=None):
             + [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())
@@ -4695,7 +4750,7 @@ def makeDtorMethodDecl(self, 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:
@@ -4710,7 +4765,7 @@ def makeSendMethodDecl(self, md, promise=False):
             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)

ipc/ipdl/ipdl/parser.py

@@ -137,6 +137,7 @@ def locFromTok(p, num):
     'struct',
     'sync',
     'union',
+    'UniquePtr',
     'upto',
     'using',
     'verify'))
@@ -667,12 +668,18 @@ def p_Type(p):
 
 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]
@@ -724,6 +731,11 @@ def p_CxxTemplateInst(p):
     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),

ipc/ipdl/ipdl/type.py

@@ -80,6 +80,9 @@ def visitFDType(self, s, *args):
     def visitEndpointType(self, s, *args):
         pass
 
+    def visitUniquePtrType(self, s, *args):
+        pass
+
 
 class Type:
     def __cmp__(self, o):
@@ -104,6 +107,9 @@ def isIPDL(self):
     def isAtom(self):
         return False
 
+    def isUniquePtr(self):
+        return False
+
     def typename(self):
         return self.__class__.__name__
 
@@ -491,6 +497,19 @@ 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:
@@ -843,7 +862,10 @@ def visitUnionDecl(self, ud):
 
     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)
@@ -1051,6 +1073,9 @@ def _canonicalType(self, itype, typespec):
         if typespec.array:
             itype = ArrayType(itype)
 
+        if typespec.uniqueptr:
+            itype = UniquePtrType(itype)
+
         return itype