Bug 1441651 - Part 1 - pass Shmem and ByteBuf by rvalref to ParamTraits; r=nika
authorAlex Gaynor <agaynor@mozilla.com>
Mon, 04 Mar 2019 16:07:37 +0000
changeset 520122 87858af7a3cc594ed1eb1cc86c86ee08f9c76d21
parent 520121 732aa8c4b98a266cf5f94d0cdb455a19009e10c8
child 520123 6e3eb5df50ff8607f0f60e8f7f8b9ed573f74fa4
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1441651
milestone67.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 1441651 - Part 1 - pass Shmem and ByteBuf by rvalref to ParamTraits; r=nika Differential Revision: https://phabricator.services.mozilla.com/D19953
gfx/ipc/GfxMessageUtils.h
ipc/glue/ByteBuf.h
ipc/glue/IPDLParamTraits.h
ipc/glue/Shmem.cpp
ipc/glue/Shmem.h
ipc/ipdl/ipdl/lower.py
--- a/gfx/ipc/GfxMessageUtils.h
+++ b/gfx/ipc/GfxMessageUtils.h
@@ -1163,19 +1163,19 @@ struct ParamTraits<mozilla::Array<T, Len
     }
     return true;
   }
 };
 
 template <>
 struct ParamTraits<mozilla::gfx::PaintFragment> {
   typedef mozilla::gfx::PaintFragment paramType;
-  static void Write(Message* aMsg, paramType& aParam) {
+  static void Write(Message* aMsg, paramType&& aParam) {
     WriteParam(aMsg, aParam.mSize);
-    WriteParam(aMsg, aParam.mRecording);
+    WriteParam(aMsg, std::move(aParam.mRecording));
     WriteParam(aMsg, aParam.mDependencies);
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter,
                    paramType* aResult) {
     return ReadParam(aMsg, aIter, &aResult->mSize) &&
            ReadParam(aMsg, aIter, &aResult->mRecording) &&
            ReadParam(aMsg, aIter, &aResult->mDependencies);
--- a/ipc/glue/ByteBuf.h
+++ b/ipc/glue/ByteBuf.h
@@ -66,17 +66,17 @@ class ByteBuf final {
 namespace IPC {
 
 template <>
 struct ParamTraits<mozilla::ipc::ByteBuf> {
   typedef mozilla::ipc::ByteBuf paramType;
 
   // this is where we transfer the memory from the ByteBuf to IPDL, avoiding a
   // copy
-  static void Write(Message* aMsg, paramType& aParam) {
+  static void Write(Message* aMsg, paramType&& aParam) {
     WriteParam(aMsg, aParam.mLen);
     // hand over ownership of the buffer to the Message
     aMsg->WriteBytesZeroCopy(aParam.mData, aParam.mLen, aParam.mCapacity);
     aParam.mData = nullptr;
     aParam.mCapacity = 0;
     aParam.mLen = 0;
   }
 
--- a/ipc/glue/IPDLParamTraits.h
+++ b/ipc/glue/IPDLParamTraits.h
@@ -95,23 +95,22 @@ static bool ReadIPDLParamList(const IPC:
 template <typename T>
 struct IPDLParamTraits<nsTArray<T>> {
   static inline void Write(IPC::Message* aMsg, IProtocol* aActor,
                            const nsTArray<T>& aParam) {
     WriteInternal(aMsg, aActor, aParam);
   }
 
   // Some serializers need to take a mutable reference to their backing object,
-  // such as Shmem segments and Byte Buffers. These serializers take the backing
-  // data and move it into the IPC layer for efficiency. They currently take
-  // these references as mutable lvalue references rather than rvalue
-  // references, (bug 1441651). This overload of Write on nsTArray is needed, as
-  // occasionally these types appear inside of IPDL arrays.
+  // such as Shmem segments and Byte Buffers. These serializers take the
+  // backing data and move it into the IPC layer for efficiency. This overload
+  // of Write on nsTArray is needed, as occasionally these types appear inside
+  // of IPDL arrays.
   static inline void Write(IPC::Message* aMsg, IProtocol* aActor,
-                           nsTArray<T>& aParam) {
+                           nsTArray<T>&& aParam) {
     WriteInternal(aMsg, aActor, aParam);
   }
 
   // This method uses infallible allocation so that an OOM failure will
   // show up as an OOM crash rather than an IPC FatalError.
   static bool Read(const IPC::Message* aMsg, PickleIterator* aIter,
                    IProtocol* aActor, nsTArray<T>* aResult) {
     uint32_t length;
@@ -158,17 +157,17 @@ struct IPDLParamTraits<nsTArray<T>> {
     WriteIPDLParam(aMsg, aActor, length);
 
     if (sUseWriteBytes) {
       auto pickledLength = CheckedInt<int>(length) * sizeof(T);
       MOZ_RELEASE_ASSERT(pickledLength.isValid());
       aMsg->WriteBytes(aParam.Elements(), pickledLength.value());
     } else {
       for (uint32_t index = 0; index < length; index++) {
-        WriteIPDLParam(aMsg, aActor, aParam.Elements()[index]);
+        WriteIPDLParam(aMsg, aActor, std::move(aParam.Elements()[index]));
       }
     }
   }
 
   // We write arrays of integer or floating-point data using a single pickling
   // 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
--- a/ipc/glue/Shmem.cpp
+++ b/ipc/glue/Shmem.cpp
@@ -430,17 +430,17 @@ IPC::Message* Shmem::ShareTo(PrivateIPDL
 }
 
 IPC::Message* Shmem::UnshareFrom(PrivateIPDLCaller, int32_t routingId) {
   AssertInvariants();
   return new ShmemDestroyed(routingId, mId);
 }
 
 void IPDLParamTraits<Shmem>::Write(IPC::Message* aMsg, IProtocol* aActor,
-                                   Shmem& aParam) {
+                                   Shmem&& aParam) {
   WriteIPDLParam(aMsg, aActor, aParam.mId);
 
   aParam.RevokeRights(Shmem::PrivateIPDLCaller());
   aParam.forget(Shmem::PrivateIPDLCaller());
 }
 
 bool IPDLParamTraits<Shmem>::Read(const IPC::Message* aMsg,
                                   PickleIterator* aIter, IProtocol* aActor,
--- a/ipc/glue/Shmem.h
+++ b/ipc/glue/Shmem.h
@@ -211,17 +211,17 @@ class Shmem final {
   size_t mSize;
   id_t mId;
 };
 
 template <>
 struct IPDLParamTraits<Shmem> {
   typedef Shmem paramType;
 
-  static void Write(IPC::Message* aMsg, IProtocol* aActor, paramType& aParam);
+  static void Write(IPC::Message* aMsg, IProtocol* aActor, paramType&& aParam);
   static bool Read(const IPC::Message* aMsg, PickleIterator* aIter,
                    IProtocol* aActor, paramType* aResult);
 
   static void Log(const paramType& aParam, std::wstring* aLog) {
     aLog->append(L"(shmem segment)");
   }
 };
 
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -575,27 +575,36 @@ def _cxxConstRefType(ipdltype, side):
     return t
 
 
 def _cxxTypeCanMoveSend(ipdltype):
     return ipdltype.isUniquePtr()
 
 
 def _cxxTypeNeedsMove(ipdltype):
+    if _cxxTypeNeedsMoveForSend(ipdltype):
+        return True
+
+    if ipdltype.isIPDL():
+        return ipdltype.isArray() or ipdltype.isEndpoint()
+
+    return False
+
+
+def _cxxTypeNeedsMoveForSend(ipdltype):
     if ipdltype.isUniquePtr():
         return True
 
     if ipdltype.isCxx():
         return ipdltype.isMoveonly()
 
     if ipdltype.isIPDL():
-        if ipdltype.isMaybe():
+        if ipdltype.isMaybe() or ipdltype.isArray():
             return _cxxTypeNeedsMove(ipdltype.basetype)
-        return (ipdltype.isArray() or
-                ipdltype.isShmem() or
+        return (ipdltype.isShmem() or
                 ipdltype.isByteBuf() or
                 ipdltype.isEndpoint())
 
     return False
 
 
 def _cxxTypeCanMove(ipdltype):
     return not (ipdltype.isIPDL() and ipdltype.isActor())
@@ -1791,32 +1800,34 @@ class _ParamTraits():
         return ifstmt
 
     @classmethod
     def fatalError(cls, reason):
         return StmtExpr(ExprCall(ExprSelect(cls.actor, '->', 'FatalError'),
                                  args=[ExprLiteral.String(reason)]))
 
     @classmethod
-    def write(cls, var, msgvar, actor):
+    def write(cls, var, msgvar, actor, ipdltype=None):
         # WARNING: This doesn't set AutoForActor for you, make sure this is
         # only called when the actor is already correctly set.
+        if ipdltype and _cxxTypeNeedsMoveForSend(ipdltype):
+            var = ExprMove(var)
         return ExprCall(ExprVar('WriteIPDLParam'), args=[msgvar, actor, var])
 
     @classmethod
     def checkedWrite(cls, ipdltype, var, msgvar, sentinelKey, actor):
         assert sentinelKey
         block = Block()
 
         # Assert we aren't serializing a null non-nullable actor
         if ipdltype and ipdltype.isIPDL() and ipdltype.isActor() and not ipdltype.nullable:
             block.addstmt(_abortIfFalse(var, 'NULL actor value passed to non-nullable param'))
 
         block.addstmts([
-            StmtExpr(cls.write(var, msgvar, actor)),
+            StmtExpr(cls.write(var, msgvar, actor, ipdltype)),
             Whitespace('// Sentinel = ' + repr(sentinelKey) + '\n', indent=True),
             StmtExpr(ExprCall(ExprSelect(msgvar, '->', 'WriteSentinel'),
                               args=[ExprLiteral.Int(hashfunc(sentinelKey))]))
         ])
         return block
 
     @classmethod
     def checkedRead(cls, ipdltype, var,
@@ -1953,17 +1964,17 @@ class _ParamTraits():
                     '==',
                     ExprCall(ExprSelect(cls.var, '->', 'GetIPCChannel'))
                 ),
                 ExprLiteral.String("Actor must be from the same channel as the"
                                    " actor it's being sent over"),
             ]))
         )
 
-        # IPC::WriteParam(..)
+        # IPC::WriteIPDLParam(..)
         write += [ifnull,
                   StmtExpr(cls.write(idvar, cls.msgvar, cls.actor))]
 
         # bool Read(..) impl
         actorvar = ExprVar('actor')
         read = [
             StmtDecl(Decl(Type('mozilla::Maybe', T=Type('mozilla::ipc::IProtocol',
                                                         ptr=True)), actorvar.name),