Bug 1262671 - IPC ReadData/ReadBytes elimination (r=froydnj)
authorBill McCloskey <billm@mozilla.com>
Mon, 16 May 2016 14:32:37 -0700
changeset 340379 c3b21c100d396a74f2ef871a0e05fc3120052ab9
parent 340378 4dc70010565c32f332c7f37eb34706571d690e18
child 340380 010987e0dc81f5021cf3b200ceab78d742783d21
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1262671
milestone49.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 1262671 - IPC ReadData/ReadBytes elimination (r=froydnj)
dom/ipc/StructuredCloneData.cpp
dom/ipc/StructuredCloneData.h
dom/plugins/ipc/NPEventAndroid.h
dom/plugins/ipc/NPEventUnix.h
dom/plugins/ipc/NPEventWindows.h
dom/plugins/ipc/PluginMessageUtils.h
ipc/chromium/src/base/histogram.cc
ipc/chromium/src/base/histogram.h
ipc/chromium/src/base/pickle.cc
ipc/chromium/src/base/pickle.h
ipc/chromium/src/chrome/common/ipc_message_utils.h
ipc/glue/IPCMessageUtils.h
netwerk/ipc/NeckoMessageUtils.h
widget/nsGUIEventIPC.h
--- a/dom/ipc/StructuredCloneData.cpp
+++ b/dom/ipc/StructuredCloneData.cpp
@@ -93,18 +93,17 @@ StructuredCloneData::Write(JSContext* aC
 }
 
 void
 StructuredCloneData::WriteIPCParams(IPC::Message* aMsg) const
 {
   WriteParam(aMsg, DataLength());
 
   if (DataLength()) {
-    // Structured clone data must be 64-bit aligned.
-    aMsg->WriteBytes(Data(), DataLength(), sizeof(uint64_t));
+    aMsg->WriteBytes(Data(), DataLength());
   }
 }
 
 bool
 StructuredCloneData::ReadIPCParams(const IPC::Message* aMsg,
                                    PickleIterator* aIter)
 {
   MOZ_ASSERT(!Data());
@@ -113,28 +112,24 @@ StructuredCloneData::ReadIPCParams(const
   if (!ReadParam(aMsg, aIter, &dataLength)) {
     return false;
   }
 
   if (!dataLength) {
     return true;
   }
 
-  uint64_t* dataBuffer = nullptr;
-  const char** buffer =
-    const_cast<const char**>(reinterpret_cast<char**>(&dataBuffer));
-  // Structured clone data must be 64-bit aligned.
-  if (!aMsg->ReadBytes(aIter, buffer, dataLength, sizeof(uint64_t))) {
+  mSharedData = SharedJSAllocatedData::AllocateForExternalData(dataLength);
+  NS_ENSURE_TRUE(mSharedData, false);
+
+  if (!aMsg->ReadBytesInto(aIter, mSharedData->Data(), dataLength)) {
+    mSharedData = nullptr;
     return false;
   }
 
-  mSharedData = SharedJSAllocatedData::CreateFromExternalData(dataBuffer,
-                                                              dataLength);
-  NS_ENSURE_TRUE(mSharedData, false);
-
   return true;
 }
 
 bool
 StructuredCloneData::CopyExternalData(const void* aData,
                                       size_t aDataLength)
 {
   MOZ_ASSERT(!Data());
--- a/dom/ipc/StructuredCloneData.h
+++ b/dom/ipc/StructuredCloneData.h
@@ -26,29 +26,37 @@ class SharedJSAllocatedData final
 public:
   SharedJSAllocatedData(uint64_t* aData, size_t aDataLength)
   : mData(aData), mDataLength(aDataLength)
   {
     MOZ_ASSERT(mData);
   }
 
   static already_AddRefed<SharedJSAllocatedData>
-  CreateFromExternalData(const void* aData, size_t aDataLength)
+  AllocateForExternalData(size_t aDataLength)
   {
     uint64_t* data = Allocate64bitSafely(aDataLength);
     if (!data) {
       return nullptr;
     }
 
-    memcpy(data, aData, aDataLength);
     RefPtr<SharedJSAllocatedData> sharedData =
       new SharedJSAllocatedData(data, aDataLength);
     return sharedData.forget();
   }
 
+  static already_AddRefed<SharedJSAllocatedData>
+  CreateFromExternalData(const void* aData, size_t aDataLength)
+  {
+    RefPtr<SharedJSAllocatedData> sharedData =
+      AllocateForExternalData(aDataLength);
+    memcpy(sharedData->Data(), aData, aDataLength);
+    return sharedData.forget();
+  }
+
   NS_INLINE_DECL_REFCOUNTING(SharedJSAllocatedData)
 
   uint64_t* Data() const { return mData; }
   size_t DataLength() const { return mDataLength; }
 
 private:
   ~SharedJSAllocatedData()
   {
--- a/dom/plugins/ipc/NPEventAndroid.h
+++ b/dom/plugins/ipc/NPEventAndroid.h
@@ -34,24 +34,17 @@ struct ParamTraits<mozilla::plugins::NPR
 
     static void Write(Message* aMsg, const paramType& aParam)
     {
         aMsg->WriteBytes(&aParam, sizeof(paramType));
     }
 
     static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
     {
-        const char* bytes = 0;
-
-        if (!aMsg->ReadBytes(aIter, &bytes, sizeof(paramType))) {
-            return false;
-        }
-
-        memcpy(aResult, bytes, sizeof(paramType));
-        return true;
+        return aMsg->ReadBytesInto(aIter, aResult, sizeof(paramType));
     }
 
     static void Log(const paramType& aParam, std::wstring* aLog)
     {
         // TODO
         aLog->append(L"(AndroidEvent)");
     }
 };
--- a/dom/plugins/ipc/NPEventUnix.h
+++ b/dom/plugins/ipc/NPEventUnix.h
@@ -52,24 +52,20 @@ struct ParamTraits<mozilla::plugins::NPR
 
     static void Write(Message* aMsg, const paramType& aParam)
     {
         aMsg->WriteBytes(&aParam, sizeof(paramType));
     }
 
     static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
     {
-        const char* bytes = 0;
-
-        if (!aMsg->ReadBytes(aIter, &bytes, sizeof(paramType))) {
+        if (!aMsg->ReadBytesInto(aIter, aResult, sizeof(paramType))) {
             return false;
         }
 
-        memcpy(aResult, bytes, sizeof(paramType));
-
 #ifdef MOZ_X11
         SetXDisplay(aResult->event);
 #endif
         return true;
     }
 
     static void Log(const paramType& aParam, std::wstring* aLog)
     {
--- a/dom/plugins/ipc/NPEventWindows.h
+++ b/dom/plugins/ipc/NPEventWindows.h
@@ -127,22 +127,19 @@ struct ParamTraits<mozilla::plugins::NPR
                 break;
         }
 
         aMsg->WriteBytes(&paramCopy, sizeof(paramType));
     }
 
     static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
     {
-        const char* bytes = 0;
-
-        if (!aMsg->ReadBytes(aIter, &bytes, sizeof(paramType))) {
+        if (!aMsg->ReadBytesInto(aIter, aResult, sizeof(paramType))) {
             return false;
         }
-        memcpy(aResult, bytes, sizeof(paramType));
 
         if (aResult->event.event == WM_PAINT) {
             // restore the lParam to point at the RECT
             aResult->event.lParam = reinterpret_cast<LPARAM>(&aResult->lParamData.rect);
         } else if (aResult->event.event == WM_WINDOWPOSCHANGED) {
             // restore the lParam to point at the WINDOWPOS
             aResult->event.lParam = reinterpret_cast<LPARAM>(&aResult->lParamData.windowpos);
         }
--- a/dom/plugins/ipc/PluginMessageUtils.h
+++ b/dom/plugins/ipc/PluginMessageUtils.h
@@ -454,25 +454,29 @@ struct ParamTraits<NPNSString*>
       return true;
     }
 
     long length;
     if (!ReadParam(aMsg, aIter, &length)) {
       return false;
     }
 
-    UniChar* buffer = nullptr;
+    // Avoid integer multiplication overflow.
+    if (length > INT_MAX / static_cast<long>(sizeof(UniChar))) {
+      return false;
+    }
+
+    auto chars = mozilla::MakeUnique<UniChar[]>(length);
     if (length != 0) {
-      if (!aMsg->ReadBytes(aIter, (const char**)&buffer, length * sizeof(UniChar)) ||
-          !buffer) {
+      if (!aMsg->ReadBytesInto(aIter, chars.get(), length * sizeof(UniChar))) {
         return false;
       }
     }
 
-    *aResult = (NPNSString*)::CFStringCreateWithBytes(kCFAllocatorDefault, (UInt8*)buffer,
+    *aResult = (NPNSString*)::CFStringCreateWithBytes(kCFAllocatorDefault, (UInt8*)chars.get(),
                                                       length * sizeof(UniChar),
                                                       kCFStringEncodingUTF16, false);
     if (!*aResult) {
       return false;
     }
 
     return true;
   }
@@ -520,26 +524,26 @@ struct ParamTraits<NSCursorInfo>
       return false;
     }
 
     uint32_t dataLength;
     if (!ReadParam(aMsg, aIter, &dataLength)) {
       return false;
     }
 
-    uint8_t* data = nullptr;
+    auto data = mozilla::MakeUnique<uint8_t[]>(dataLength);
     if (dataLength != 0) {
-      if (!aMsg->ReadBytes(aIter, (const char**)&data, dataLength) || !data) {
+      if (!aMsg->ReadBytesInto(aIter, data.get(), dataLength)) {
         return false;
       }
     }
 
     aResult->SetType(type);
     aResult->SetHotSpot(nsPoint(hotSpotX, hotSpotY));
-    aResult->SetCustomImageData(data, dataLength);
+    aResult->SetCustomImageData(data.get(), dataLength);
 
     return true;
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     const char* typeName = aParam.GetTypeName();
     nsPoint hotSpot = aParam.GetHotSpot();
--- a/ipc/chromium/src/base/histogram.cc
+++ b/ipc/chromium/src/base/histogram.cc
@@ -235,109 +235,16 @@ void Histogram::WriteAscii(bool graph_it
       WriteAsciiBucketGraph(current_size, max_size, output);
     WriteAsciiBucketContext(past, current, remaining, i, output);
     output->append(newline);
     past += current;
   }
   DCHECK_EQ(sample_count, past);
 }
 
-// static
-std::string Histogram::SerializeHistogramInfo(const Histogram& histogram,
-                                              const SampleSet& snapshot) {
-  DCHECK_NE(NOT_VALID_IN_RENDERER, histogram.histogram_type());
-
-  Pickle pickle;
-  pickle.WriteString(histogram.histogram_name());
-  pickle.WriteInt(histogram.declared_min());
-  pickle.WriteInt(histogram.declared_max());
-  pickle.WriteSize(histogram.bucket_count());
-  pickle.WriteUInt32(histogram.range_checksum());
-  pickle.WriteInt(histogram.histogram_type());
-  pickle.WriteInt(histogram.flags());
-
-  snapshot.Serialize(&pickle);
-  return std::string(static_cast<const char*>(pickle.data()), pickle.size());
-}
-
-// static
-bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
-  if (histogram_info.empty()) {
-      return false;
-  }
-
-  Pickle pickle(histogram_info.data(),
-                static_cast<int>(histogram_info.size()));
-  std::string histogram_name;
-  int declared_min;
-  int declared_max;
-  size_t bucket_count;
-  uint32_t range_checksum;
-  int histogram_type;
-  int pickle_flags;
-  SampleSet sample;
-
-  PickleIterator iter(pickle);
-  if (!pickle.ReadString(&iter, &histogram_name) ||
-      !pickle.ReadInt(&iter, &declared_min) ||
-      !pickle.ReadInt(&iter, &declared_max) ||
-      !pickle.ReadSize(&iter, &bucket_count) ||
-      !pickle.ReadUInt32(&iter, &range_checksum) ||
-      !pickle.ReadInt(&iter, &histogram_type) ||
-      !pickle.ReadInt(&iter, &pickle_flags) ||
-      !sample.Histogram::SampleSet::Deserialize(&iter, pickle)) {
-    CHROMIUM_LOG(ERROR) << "Pickle error decoding Histogram: " << histogram_name;
-    return false;
-  }
-  DCHECK(pickle_flags & kIPCSerializationSourceFlag);
-  // Since these fields may have come from an untrusted renderer, do additional
-  // checks above and beyond those in Histogram::Initialize()
-  if (declared_max <= 0 || declared_min <= 0 || declared_max < declared_min ||
-      INT_MAX / sizeof(Count) <= bucket_count || bucket_count < 2) {
-    CHROMIUM_LOG(ERROR) << "Values error decoding Histogram: " << histogram_name;
-    return false;
-  }
-
-  Flags flags = static_cast<Flags>(pickle_flags & ~kIPCSerializationSourceFlag);
-
-  DCHECK_NE(NOT_VALID_IN_RENDERER, histogram_type);
-
-  Histogram* render_histogram(NULL);
-
-  if (histogram_type == HISTOGRAM) {
-    render_histogram = Histogram::FactoryGet(
-        histogram_name, declared_min, declared_max, bucket_count, flags);
-  } else if (histogram_type == LINEAR_HISTOGRAM) {
-    render_histogram = LinearHistogram::FactoryGet(
-        histogram_name, declared_min, declared_max, bucket_count, flags);
-  } else if (histogram_type == BOOLEAN_HISTOGRAM) {
-    render_histogram = BooleanHistogram::FactoryGet(histogram_name, flags);
-  } else {
-    CHROMIUM_LOG(ERROR) << "Error Deserializing Histogram Unknown histogram_type: "
-                        << histogram_type;
-    return false;
-  }
-
-  DCHECK_EQ(render_histogram->declared_min(), declared_min);
-  DCHECK_EQ(render_histogram->declared_max(), declared_max);
-  DCHECK_EQ(render_histogram->bucket_count(), bucket_count);
-  DCHECK_EQ(render_histogram->range_checksum(), range_checksum);
-  DCHECK_EQ(render_histogram->histogram_type(), histogram_type);
-
-  if (render_histogram->flags() & kIPCSerializationSourceFlag) {
-    DVLOG(1) << "Single process mode, histogram observed and not copied: "
-             << histogram_name;
-  } else {
-    DCHECK_EQ(flags & render_histogram->flags(), flags);
-    render_histogram->AddSampleSet(sample);
-  }
-
-  return true;
-}
-
 //------------------------------------------------------------------------------
 // Methods for the validating a sample and a related histogram.
 //------------------------------------------------------------------------------
 
 Histogram::Inconsistencies Histogram::FindCorruption(
     const SampleSet& snapshot,
     const OffTheBooksMutexAutoLock& snapshotLockEvidence) const {
   int inconsistencies = NO_INCONSISTENCIES;
@@ -768,58 +675,16 @@ void Histogram::SampleSet::Add(const Sam
   OffTheBooksMutexAutoLock locker(mutex_);
   DCHECK_EQ(counts_.size(), other.counts_.size());
   sum_ += other.sum_;
   redundant_count_ += other.redundant_count_;
   for (size_t index = 0; index < counts_.size(); ++index)
     counts_[index] += other.counts_[index];
 }
 
-bool Histogram::SampleSet::Serialize(Pickle* pickle) const {
-  OffTheBooksMutexAutoLock locker(mutex_);
-  pickle->WriteInt64(sum_);
-  pickle->WriteInt64(redundant_count_);
-  pickle->WriteSize(counts_.size());
-
-  for (size_t index = 0; index < counts_.size(); ++index) {
-    pickle->WriteInt(counts_[index]);
-  }
-
-  return true;
-}
-
-bool Histogram::SampleSet::Deserialize(PickleIterator* iter, const Pickle& pickle) {
-  OffTheBooksMutexAutoLock locker(mutex_);
-  DCHECK_EQ(counts_.size(), 0u);
-  DCHECK_EQ(sum_, 0);
-  DCHECK_EQ(redundant_count_, 0);
-
-  size_t counts_size;
-
-  if (!pickle.ReadInt64(iter, &sum_) ||
-      !pickle.ReadInt64(iter, &redundant_count_) ||
-      !pickle.ReadSize(iter, &counts_size)) {
-    return false;
-  }
-
-  if (counts_size == 0)
-    return false;
-
-  int count = 0;
-  for (size_t index = 0; index < counts_size; ++index) {
-    int i;
-    if (!pickle.ReadInt(iter, &i))
-      return false;
-    counts_.push_back(i);
-    count += i;
-  }
-
-  return true;
-}
-
 //------------------------------------------------------------------------------
 // LinearHistogram: This histogram uses a traditional set of evenly spaced
 // buckets.
 //------------------------------------------------------------------------------
 
 LinearHistogram::~LinearHistogram() {
 }
 
--- a/ipc/chromium/src/base/histogram.h
+++ b/ipc/chromium/src/base/histogram.h
@@ -49,19 +49,16 @@
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/time.h"
 #include "base/lock.h"
 
-class Pickle;
-class PickleIterator;
-
 namespace base {
 
 using mozilla::OffTheBooksMutex;
 using mozilla::OffTheBooksMutexAutoLock;
 
 //------------------------------------------------------------------------------
 // Provide easy general purpose histogram in a macro, just like stats counters.
 // The first four macros use 50 buckets.
@@ -297,23 +294,16 @@ class Histogram {
     LINEAR,
     CUSTOM
   };
 
   enum Flags {
     kNoFlags = 0,
     kUmaTargetedHistogramFlag = 0x1,  // Histogram should be UMA uploaded.
 
-    // Indicate that the histogram was pickled to be sent across an IPC Channel.
-    // If we observe this flag on a histogram being aggregated into after IPC,
-    // then we are running in a single process mode, and the aggregation should
-    // not take place (as we would be aggregating back into the source
-    // histogram!).
-    kIPCSerializationSourceFlag = 0x10,
-
     kHexRangePrintingFlag = 0x8000  // Fancy bucket-naming supported.
   };
 
   enum Inconsistencies {
     NO_INCONSISTENCIES = 0x0,
     RANGE_CHECKSUM_ERROR = 0x1,
     BUCKET_ORDER_ERROR = 0x2,
     COUNT_HIGH_ERROR = 0x4,
@@ -356,19 +346,16 @@ class Histogram {
     void Resize(const Histogram& histogram);
 
     // Accessor for histogram to make routine additions.
     void Accumulate(Sample value, Count count, size_t index);
 
     // Arithmetic manipulation of corresponding elements of the set.
     void Add(const SampleSet& other);
 
-    bool Serialize(Pickle* pickle) const;
-    bool Deserialize(PickleIterator* iter, const Pickle& pickle);
-
     size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf);
 
     //---------------- THREAD UNSAFE METHODS ----------------//
     //
     // The caller must hold |this.mutex_|, and must supply evidence by passing
     // a const reference to the relevant OffTheBooksMutexAutoLock used.
 
     Count counts(const OffTheBooksMutexAutoLock& ev, size_t i) const {
@@ -469,31 +456,16 @@ class Histogram {
 
   // Support generic flagging of Histograms.
   // 0x1 Currently used to mark this histogram to be recorded by UMA..
   // 0x8000 means print ranges in hex.
   void SetFlags(Flags flags) { flags_ = static_cast<Flags> (flags_ | flags); }
   void ClearFlags(Flags flags) { flags_ = static_cast<Flags>(flags_ & ~flags); }
   int flags() const { return flags_; }
 
-  // Convenience methods for serializing/deserializing the histograms.
-  // Histograms from Renderer process are serialized and sent to the browser.
-  // Browser process reconstructs the histogram from the pickled version
-  // accumulates the browser-side shadow copy of histograms (that mirror
-  // histograms created in the renderer).
-
-  // Serialize the given snapshot of a Histogram into a String. Uses
-  // Pickle class to flatten the object.
-  static std::string SerializeHistogramInfo(const Histogram& histogram,
-                                            const SampleSet& snapshot);
-  // The following method accepts a list of pickled histograms and
-  // builds a histogram and updates shadow copy of histogram data in the
-  // browser process.
-  static bool DeserializeHistogramInfo(const std::string& histogram_info);
-
   // Check to see if bucket ranges, counts and tallies in the snapshot are
   // consistent with the bucket ranges and checksums in our histogram.  This can
   // produce a false-alarm if a race occurred in the reading of the data during
   // a SnapShot process, but should otherwise be false at all times (unless we
   // have memory over-writes, or DRAM failures).
   virtual Inconsistencies FindCorruption(const SampleSet& snapshot,
                                          const OffTheBooksMutexAutoLock&
                                                snapshotLockEvidence) const;
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -430,25 +430,28 @@ bool Pickle::ReadBytes(PickleIterator* i
 
   *data = static_cast<const char*>(iter->iter_) + paddingLen;
   DCHECK(intptr_t(*data) % alignment == 0);
 
   UpdateIter(iter, length);
   return true;
 }
 
-bool Pickle::ReadData(PickleIterator* iter, const char** data, int* length) const {
-  DCHECK(iter);
-  DCHECK(data);
-  DCHECK(length);
+bool Pickle::FlattenBytes(PickleIterator* iter, const char** data, int length,
+                           uint32_t alignment) const {
+  return ReadBytes(iter, data, length, alignment);
+}
 
-  if (!ReadLength(iter, length))
+bool Pickle::ReadBytesInto(PickleIterator* iter, void* data, int length) const {
+  const char* outp;
+  if (!ReadBytes(iter, &outp, length)) {
     return false;
-
-  return ReadBytes(iter, data, *length);
+  }
+  memcpy(data, outp, length);
+  return true;
 }
 
 bool Pickle::ReadSentinel(PickleIterator* iter, uint32_t sentinel) const {
 #ifdef SENTINEL_CHECKING
   uint32_t found;
   if (!ReadUInt32(iter, &found)) {
     return false;
   }
--- a/ipc/chromium/src/base/pickle.h
+++ b/ipc/chromium/src/base/pickle.h
@@ -109,25 +109,25 @@ class Pickle {
   MOZ_MUST_USE bool ReadUInt32(PickleIterator* iter, uint32_t* result) const;
   MOZ_MUST_USE bool ReadInt64(PickleIterator* iter, int64_t* result) const;
   MOZ_MUST_USE bool ReadUInt64(PickleIterator* iter, uint64_t* result) const;
   MOZ_MUST_USE bool ReadDouble(PickleIterator* iter, double* result) const;
   MOZ_MUST_USE bool ReadIntPtr(PickleIterator* iter, intptr_t* result) const;
   MOZ_MUST_USE bool ReadUnsignedChar(PickleIterator* iter, unsigned char* result) const;
   MOZ_MUST_USE bool ReadString(PickleIterator* iter, std::string* result) const;
   MOZ_MUST_USE bool ReadWString(PickleIterator* iter, std::wstring* result) const;
-  MOZ_MUST_USE bool ReadData(PickleIterator* iter, const char** data, int* length) const;
-  MOZ_MUST_USE bool ReadBytes(PickleIterator* iter, const char** data, int length,
-                              uint32_t alignment = sizeof(memberAlignmentType)) const;
+  MOZ_MUST_USE bool ReadBytesInto(PickleIterator* iter, void* data, int length) const;
+  MOZ_MUST_USE bool FlattenBytes(PickleIterator* iter, const char** data, int length,
+                                 uint32_t alignment = sizeof(memberAlignmentType)) const;
 
   // Safer version of ReadInt() checks for the result not being negative.
   // Use it for reading the object sizes.
   MOZ_MUST_USE bool ReadLength(PickleIterator* iter, int* result) const;
 
-  MOZ_WARN_UNUSED_RESULT bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const;
+  MOZ_MUST_USE bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const;
 
   void EndRead(PickleIterator& iter) const {
     DCHECK(iter.iter_ == end_of_payload());
   }
 
   // Methods for adding to the payload of the Pickle.  These values are
   // appended to the end of the Pickle's payload.  When reading values from a
   // Pickle, it is important to read them in the order in which they were added
@@ -228,16 +228,19 @@ class Pickle {
     const char* end_of_region = reinterpret_cast<const char*>(iter.iter_) + len;
     // Watch out for overflow in pointer calculation, which wraps.
     return (iter.iter_ <= end_of_region) && (end_of_region <= end_of_payload());
   }
 
   typedef uint32_t memberAlignmentType;
 
  protected:
+  MOZ_MUST_USE bool ReadBytes(PickleIterator* iter, const char** data, int length,
+                              uint32_t alignment = sizeof(memberAlignmentType)) const;
+
   uint32_t payload_size() const { return header_->payload_size; }
 
   char* payload() {
     return reinterpret_cast<char*>(header_) + header_size_;
   }
   const char* payload() const {
     return reinterpret_cast<const char*>(header_) + header_size_;
   }
--- a/ipc/chromium/src/chrome/common/ipc_message_utils.h
+++ b/ipc/chromium/src/chrome/common/ipc_message_utils.h
@@ -50,21 +50,16 @@ class MessageIterator {
     return val;
   }
   const std::wstring NextWString() const {
     std::wstring val;
     if (!msg_.ReadWString(&iter_, &val))
       NOTREACHED();
     return val;
   }
-  void NextData(const char** data, int* length) const {
-    if (!msg_.ReadData(&iter_, data, length)) {
-      NOTREACHED();
-    }
-  }
  private:
   const Message& msg_;
   mutable PickleIterator iter_;
 };
 
 //-----------------------------------------------------------------------------
 // ParamTraits specializations, etc.
 //
@@ -187,76 +182,48 @@ struct ParamTraitsFundamental<unsigned l
     l->append(StringPrintf(L"%ul", p));
   }
 };
 
 template <>
 struct ParamTraitsFundamental<long long> {
   typedef long long param_type;
   static void Write(Message* m, const param_type& p) {
-    m->WriteData(reinterpret_cast<const char*>(&p), sizeof(param_type));
- }
+    m->WriteBytes(&p, sizeof(param_type));
+  }
   static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
-    const char *data;
-    int data_size = 0;
-    bool result = m->ReadData(iter, &data, &data_size);
-    if (result && data_size == sizeof(param_type)) {
-      memcpy(r, data, sizeof(param_type));
-    } else {
-      result = false;
-      NOTREACHED();
-    }
-    return result;
+    return m->ReadBytesInto(iter, r, sizeof(*r));
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(StringPrintf(L"%ll", p));
   }
 };
 
 template <>
 struct ParamTraitsFundamental<unsigned long long> {
   typedef unsigned long long param_type;
   static void Write(Message* m, const param_type& p) {
-    m->WriteData(reinterpret_cast<const char*>(&p), sizeof(param_type));
- }
+    m->WriteBytes(&p, sizeof(param_type));
+  }
   static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
-    const char *data;
-    int data_size = 0;
-    bool result = m->ReadData(iter, &data, &data_size);
-    if (result && data_size == sizeof(param_type)) {
-      memcpy(r, data, sizeof(param_type));
-    } else {
-      result = false;
-      NOTREACHED();
-    }
-    return result;
+    return m->ReadBytesInto(iter, r, sizeof(*r));
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(StringPrintf(L"%ull", p));
   }
 };
 
 template <>
 struct ParamTraitsFundamental<double> {
   typedef double param_type;
   static void Write(Message* m, const param_type& p) {
-    m->WriteData(reinterpret_cast<const char*>(&p), sizeof(param_type));
+    m->WriteDouble(p);
   }
   static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
-    const char *data;
-    int data_size = 0;
-    bool result = m->ReadData(iter, &data, &data_size);
-    if (result && data_size == sizeof(param_type)) {
-      memcpy(r, data, sizeof(param_type));
-    } else {
-      result = false;
-      NOTREACHED();
-    }
-
-    return result;
+    return m->ReadDouble(iter, r);
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(StringPrintf(L"e", p));
   }
 };
 
 // Fixed-size <stdint.h> types.
 
--- a/ipc/glue/IPCMessageUtils.h
+++ b/ipc/glue/IPCMessageUtils.h
@@ -227,43 +227,33 @@ struct ParamTraits<int8_t>
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     aMsg->WriteBytes(&aParam, sizeof(aParam));
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
-    const char* outp;
-    if (!aMsg->ReadBytes(aIter, &outp, sizeof(*aResult)))
-      return false;
-
-    *aResult = *reinterpret_cast<const paramType*>(outp);
-    return true;
+    return aMsg->ReadBytesInto(aIter, aResult, sizeof(*aResult));
   }
 };
 
 template<>
 struct ParamTraits<uint8_t>
 {
   typedef uint8_t paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     aMsg->WriteBytes(&aParam, sizeof(aParam));
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
-    const char* outp;
-    if (!aMsg->ReadBytes(aIter, &outp, sizeof(*aResult)))
-      return false;
-
-    *aResult = *reinterpret_cast<const paramType*>(outp);
-    return true;
+    return aMsg->ReadBytesInto(aIter, aResult, sizeof(*aResult));
   }
 };
 
 #if !defined(OS_POSIX)
 // See above re: keeping definitions in sync
 template<>
 struct ParamTraits<base::FileDescriptor>
 {
@@ -304,24 +294,22 @@ struct ParamTraits<nsACString>
       return false;
 
     if (isVoid) {
       aResult->SetIsVoid(true);
       return true;
     }
 
     uint32_t length;
-    if (ReadParam(aMsg, aIter, &length)) {
-      const char* buf;
-      if (aMsg->ReadBytes(aIter, &buf, length)) {
-        aResult->Assign(buf, length);
-        return true;
-      }
+    if (!ReadParam(aMsg, aIter, &length)) {
+      return false;
     }
-    return false;
+    aResult->SetLength(length);
+
+    return aMsg->ReadBytesInto(aIter, aResult->BeginWriting(), length);
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     if (aParam.IsVoid())
       aLog->append(L"(NULL)");
     else
       aLog->append(UTF8ToWide(aParam.BeginReading()));
@@ -354,25 +342,22 @@ struct ParamTraits<nsAString>
       return false;
 
     if (isVoid) {
       aResult->SetIsVoid(true);
       return true;
     }
 
     uint32_t length;
-    if (ReadParam(aMsg, aIter, &length)) {
-      const char16_t* buf;
-      if (aMsg->ReadBytes(aIter, reinterpret_cast<const char**>(&buf),
-                       length * sizeof(char16_t))) {
-        aResult->Assign(buf, length);
-        return true;
-      }
+    if (!ReadParam(aMsg, aIter, &length)) {
+      return false;
     }
-    return false;
+    aResult->SetLength(length);
+
+    return aMsg->ReadBytesInto(aIter, aResult->BeginWriting(), length * sizeof(char16_t));
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     if (aParam.IsVoid())
       aLog->append(L"(NULL)");
     else {
 #ifdef WCHAR_T_IS_UTF16
@@ -469,36 +454,29 @@ struct ParamTraits<nsTArray<E>>
     }
 
     if (sUseWriteBytes) {
       int pickledLength = 0;
       if (!ByteLengthIsValid(length, sizeof(E), &pickledLength)) {
         return false;
       }
 
-      const char* outdata;
-      if (!aMsg->ReadBytes(aIter, &outdata, pickledLength)) {
-        return false;
-      }
-
       E* elements = aResult->AppendElements(length);
-
-      memcpy(elements, outdata, pickledLength);
+      return aMsg->ReadBytesInto(aIter, elements, pickledLength);
     } else {
       aResult->SetCapacity(length);
 
       for (uint32_t index = 0; index < length; index++) {
         E* element = aResult->AppendElement();
         if (!ReadParam(aMsg, aIter, element)) {
           return false;
         }
       }
+      return true;
     }
-
-    return true;
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     for (uint32_t index = 0; index < aParam.Length(); index++) {
       if (index) {
         aLog->append(L" ");
       }
@@ -547,21 +525,17 @@ struct ParamTraits<float>
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     aMsg->WriteBytes(&aParam, sizeof(paramType));
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
-    const char* outFloat;
-    if (!aMsg->ReadBytes(aIter, &outFloat, sizeof(float)))
-      return false;
-    *aResult = *reinterpret_cast<const float*>(outFloat);
-    return true;
+    return aMsg->ReadBytesInto(aIter, aResult, sizeof(*aResult));
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     aLog->append(StringPrintf(L"%g", aParam));
   }
 };
 
@@ -744,26 +718,27 @@ struct ParamTraits<mozilla::SerializedSt
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
     if (!ReadParam(aMsg, aIter, &aResult->dataLength)) {
       return false;
     }
 
-    if (aResult->dataLength) {
-      const char** buffer =
-        const_cast<const char**>(reinterpret_cast<char**>(&aResult->data));
-      // Structured clone data must be 64-bit aligned.
-      if (!aMsg->ReadBytes(aIter, buffer, aResult->dataLength,
-                           sizeof(uint64_t))) {
-        return false;
-      }
-    } else {
+    if (!aResult->dataLength) {
       aResult->data = nullptr;
+      return true;
+    }
+
+    const char** buffer =
+      const_cast<const char**>(reinterpret_cast<char**>(&aResult->data));
+    // Structured clone data must be 64-bit aligned.
+    if (!aMsg->FlattenBytes(aIter, buffer, aResult->dataLength,
+                            sizeof(uint64_t))) {
+      return false;
     }
 
     return true;
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
     LogParam(aParam.dataLength, aLog);
--- a/netwerk/ipc/NeckoMessageUtils.h
+++ b/netwerk/ipc/NeckoMessageUtils.h
@@ -113,39 +113,29 @@ struct ParamTraits<mozilla::net::NetAddr
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, mozilla::net::NetAddr* aResult)
   {
     if (!ReadParam(aMsg, aIter, &aResult->raw.family))
       return false;
 
     if (aResult->raw.family == AF_UNSPEC) {
-      const char *tmp;
-      if (aMsg->ReadBytes(aIter, &tmp, sizeof(aResult->raw.data))) {
-        memcpy(&(aResult->raw.data), tmp, sizeof(aResult->raw.data));
-        return true;
-      }
-      return false;
+      return aMsg->ReadBytesInto(aIter, &aResult->raw.data, sizeof(aResult->raw.data));
     } else if (aResult->raw.family == AF_INET) {
       return ReadParam(aMsg, aIter, &aResult->inet.port) &&
              ReadParam(aMsg, aIter, &aResult->inet.ip);
     } else if (aResult->raw.family == AF_INET6) {
       return ReadParam(aMsg, aIter, &aResult->inet6.port) &&
              ReadParam(aMsg, aIter, &aResult->inet6.flowinfo) &&
              ReadParam(aMsg, aIter, &aResult->inet6.ip.u64[0]) &&
              ReadParam(aMsg, aIter, &aResult->inet6.ip.u64[1]) &&
              ReadParam(aMsg, aIter, &aResult->inet6.scope_id);
 #if defined(XP_UNIX)
     } else if (aResult->raw.family == AF_LOCAL) {
-      const char *tmp;
-      if (aMsg->ReadBytes(aIter, &tmp, sizeof(aResult->local.path))) {
-        memcpy(&(aResult->local.path), tmp, sizeof(aResult->local.path));
-        return true;
-      }
-      return false;
+      return aMsg->ReadBytesInto(aIter, &aResult->local.path, sizeof(aResult->local.path));
 #endif
     }
 
     /* We've been tricked by some socket family we don't know about! */
     return false;
   }
 };
 
--- a/widget/nsGUIEventIPC.h
+++ b/widget/nsGUIEventIPC.h
@@ -44,22 +44,17 @@ struct ParamTraits<mozilla::BaseEventFla
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     aMsg->WriteBytes(&aParam, sizeof(aParam));
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
-    const char* outp;
-    if (!aMsg->ReadBytes(aIter, &outp, sizeof(*aResult))) {
-      return false;
-    }
-    *aResult = *reinterpret_cast<const paramType*>(outp);
-    return true;
+    return aMsg->ReadBytesInto(aIter, aResult, sizeof(*aResult));
   }
 };
 
 template<>
 struct ParamTraits<mozilla::WidgetEvent>
 {
   typedef mozilla::WidgetEvent paramType;