Bug 1383260: Fix problems with unmarshaling handler-wrapped object when in its original apartment; r=jimm
authorAaron Klotz <aklotz@mozilla.com>
Fri, 21 Jul 2017 16:16:16 -0600
changeset 422431 add7f83758561c8d534b47af5e98379b4f2f19dd
parent 422430 a7f939d3f4a462c0b372917ffe21cc2702c0a877
child 422432 95c0feba19eca15b4d7084e5eb0d5f08b4858b88
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1383260
milestone56.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 1383260: Fix problems with unmarshaling handler-wrapped object when in its original apartment; r=jimm
ipc/mscom/Objref.cpp
ipc/mscom/oop/Handler.cpp
--- a/ipc/mscom/Objref.cpp
+++ b/ipc/mscom/Objref.cpp
@@ -237,13 +237,21 @@ StripHandlerFromOBJREF(NotNull<IStream*>
     return false;
   }
 
   hr = aStream->Write(resAddr, resAddrSize, &bytesWritten);
   if (FAILED(hr) || bytesWritten != resAddrSize) {
     return false;
   }
 
+  // The difference between a OBJREF_STANDARD and an OBJREF_HANDLER is
+  // sizeof(CLSID), so we'll zero out the remaining bytes.
+  CLSID zeroClsid = {0};
+  hr = aStream->Write(&zeroClsid, sizeof(CLSID), &bytesWritten);
+  if (FAILED(hr) || bytesWritten != sizeof(CLSID)) {
+    return false;
+  }
+
   return true;
 }
 
 } // namespace mscom
 } // namespace mozilla
--- a/ipc/mscom/oop/Handler.cpp
+++ b/ipc/mscom/oop/Handler.cpp
@@ -136,32 +136,37 @@ Handler::GetMarshalSizeMax(REFIID riid, 
   }
 
   // We do not necessarily want to use the pv that COM is giving us; we may want
   // to marshal a different proxy that is more appropriate to what we're
   // wrapping...
   hr = mUnmarshal->GetMarshalSizeMax(marshalAs, unkToMarshal.get(),
                                      dwDestContext, pvDestContext,
                                      mshlflags, pSize);
+
+#if defined(MOZ_MSCOM_REMARSHAL_NO_HANDLER)
+  return hr;
+#else
   if (FAILED(hr)) {
     return hr;
   }
 
   if (!HasPayload()) {
     return S_OK;
   }
 
   DWORD payloadSize = 0;
   hr = GetHandlerPayloadSize(marshalAs, &payloadSize);
   if (FAILED(hr)) {
     return hr;
   }
 
   *pSize += payloadSize;
   return S_OK;
+#endif // defined(MOZ_MSCOM_REMARSHAL_NO_HANDLER)
 }
 
 HRESULT
 Handler::MarshalInterface(IStream* pStm, REFIID riid, void* pv,
                           DWORD dwDestContext, void* pvDestContext,
                           DWORD mshlflags)
 {
   // We do not necessarily want to use the pv that COM is giving us; we may want
@@ -230,27 +235,24 @@ HRESULT
 Handler::UnmarshalInterface(IStream* pStm, REFIID riid, void** ppv)
 {
   REFIID unmarshalAs = MarshalAs(riid);
   HRESULT hr = mUnmarshal->UnmarshalInterface(pStm, unmarshalAs, ppv);
   if (FAILED(hr)) {
     return hr;
   }
 
-  hr = ReadHandlerPayload(pStm, unmarshalAs);
-
   // This method may be called on the same object multiple times (as new
   // interfaces are queried off the proxy). Not all interfaces will necessarily
   // refresh the payload, so we set mHasPayload using OR to reflect that fact.
   // (Otherwise mHasPayload could be cleared and the handler would think that
   // it doesn't have a payload even though it actually does).
-  mHasPayload |= (hr == S_OK);
+  mHasPayload |= (ReadHandlerPayload(pStm, unmarshalAs) == S_OK);
 
-  // hr may be S_FALSE, but we don't want to return that
-  return SUCCEEDED(hr) ? S_OK : hr;
+  return hr;
 }
 
 HRESULT
 Handler::ReleaseMarshalData(IStream* pStm)
 {
   return mUnmarshal->ReleaseMarshalData(pStm);
 }