Bug 1297981 - Delete BufferList::FlattenBytes and Pickle::FlattenBytes. r=billm draft
authorKan-Ru Chen <kanru@kanru.info>
Thu, 25 Aug 2016 17:15:38 +0800
changeset 405295 93c124a8a7de0b7d7547307a4d70b2caf0949f45
parent 404988 01748a2b1a463f24efd9cd8abad9ccfd76b037b8
child 529418 465831bc2e091b5415378f7d419898c3bbb90a63
push id27467
push userbmo:kchen@mozilla.com
push dateThu, 25 Aug 2016 09:16:49 +0000
reviewersbillm
bugs1297981
milestone51.0a1
Bug 1297981 - Delete BufferList::FlattenBytes and Pickle::FlattenBytes. r=billm MozReview-Commit-ID: G3a4DN4Lovi
ipc/chromium/src/base/pickle.cc
ipc/chromium/src/base/pickle.h
mfbt/BufferList.h
mfbt/tests/TestBufferList.cpp
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -391,41 +391,16 @@ bool Pickle::ReadWString(PickleIterator*
   if (!ReadBytesInto(iter, chars.get(), len * sizeof(wchar_t))) {
     return false;
   }
   result->assign(chars.get(), len);
 
   return true;
 }
 
-bool Pickle::FlattenBytes(PickleIterator* iter, const char** data, uint32_t length,
-                          uint32_t alignment) {
-  DCHECK(iter);
-  DCHECK(data);
-  DCHECK(alignment == 4 || alignment == 8);
-  DCHECK(intptr_t(header_) % alignment == 0);
-
-  if (AlignInt(length) < length) {
-    return false;
-  }
-
-  uint32_t padding_len = intptr_t(iter->iter_.Data()) % alignment;
-  if (!iter->iter_.AdvanceAcrossSegments(buffers_, padding_len)) {
-    return false;
-  }
-
-  if (!buffers_.FlattenBytes(iter->iter_, data, length)) {
-    return false;
-  }
-
-  header_ = reinterpret_cast<Header*>(buffers_.Start());
-
-  return iter->iter_.AdvanceAcrossSegments(buffers_, AlignInt(length) - length);
-}
-
 bool Pickle::ExtractBuffers(PickleIterator* iter, size_t length, BufferList* buffers,
                             uint32_t alignment) const
 {
   DCHECK(iter);
   DCHECK(buffers);
   DCHECK(alignment == 4 || alignment == 8);
   DCHECK(intptr_t(header_) % alignment == 0);
 
--- a/ipc/chromium/src/base/pickle.h
+++ b/ipc/chromium/src/base/pickle.h
@@ -106,18 +106,16 @@ class Pickle {
   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 ReadBytesInto(PickleIterator* iter, void* data, uint32_t length) const;
-  MOZ_MUST_USE bool FlattenBytes(PickleIterator* iter, const char** data, uint32_t length,
-                                 uint32_t alignment = sizeof(memberAlignmentType));
   MOZ_MUST_USE bool ExtractBuffers(PickleIterator* iter, size_t length, BufferList* buffers,
                                    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_MUST_USE bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const
--- a/mfbt/BufferList.h
+++ b/mfbt/BufferList.h
@@ -56,20 +56,16 @@ class BufferList : private AllocPolicy
   friend class BufferList;
 
  public:
   // For the convenience of callers, all segments are required to be a multiple
   // of 8 bytes in capacity. Also, every buffer except the last one is required
   // to be full (i.e., size == capacity). Therefore, a byte at offset N within
   // the BufferList and stored in memory at an address A will satisfy
   // (N % Align == A % Align) if Align == 2, 4, or 8.
-  //
-  // NB: FlattenBytes can create non-full segments in the middle of the
-  // list. However, it ensures that these buffers are 8-byte aligned, so the
-  // offset invariant is not violated.
   static const size_t kSegmentAlignment = 8;
 
   // Allocate a BufferList. The BufferList will free all its buffers when it is
   // destroyed. An initial buffer of size aInitialSize and capacity
   // aInitialCapacity is allocated automatically. This data will be contiguous
   // an can be accessed via |Start()|. Subsequent buffers will be allocated with
   // capacity aStandardCapacity.
   BufferList(size_t aInitialSize,
@@ -239,24 +235,16 @@ class BufferList : private AllocPolicy
   // bytes may be split across multiple buffers. Size() is increased by aSize.
   inline bool WriteBytes(const char* aData, size_t aSize);
 
   // Copies possibly non-contiguous byte range starting at aIter into
   // aData. aIter is advanced by aSize bytes. Returns false if it runs out of
   // data before aSize.
   inline bool ReadBytes(IterImpl& aIter, char* aData, size_t aSize) const;
 
-  // FlattenBytes reconfigures the BufferList so that data in the range
-  // [aIter, aIter + aSize) is stored contiguously. A pointer to this data is
-  // returned in aOutData. Returns false if not enough data is available. All
-  // other iterators are invalidated by this method.
-  //
-  // This method requires aIter and aSize to be 8-byte aligned.
-  inline bool FlattenBytes(IterImpl& aIter, const char** aOutData, size_t aSize);
-
   // Return a new BufferList that shares storage with this BufferList. The new
   // BufferList is read-only. It allows iteration over aSize bytes starting at
   // aIter. Borrow can fail, in which case *aSuccess will be false upon
   // return. The borrowed BufferList can use a different AllocPolicy than the
   // original one. However, it is not responsible for freeing buffers, so the
   // AllocPolicy is only used for the buffer vector.
   template<typename BorrowingAllocPolicy>
   BufferList<BorrowingAllocPolicy> Borrow(IterImpl& aIter, size_t aSize, bool* aSuccess,
@@ -367,74 +355,16 @@ BufferList<AllocPolicy>::ReadBytes(IterI
     remaining -= toCopy;
 
     aIter.Advance(*this, toCopy);
   }
 
   return true;
 }
 
-template<typename AllocPolicy>
-bool
-BufferList<AllocPolicy>::FlattenBytes(IterImpl& aIter, const char** aOutData, size_t aSize)
-{
-  MOZ_RELEASE_ASSERT(aSize);
-  MOZ_RELEASE_ASSERT(mOwning);
-
-  if (aIter.HasRoomFor(aSize)) {
-    // If the data is already contiguous, just return a pointer.
-    *aOutData = aIter.Data();
-    aIter.Advance(*this, aSize);
-    return true;
-  }
-
-  // This buffer will become the new contiguous segment.
-  char* buffer = this->template pod_malloc<char>(Size());
-  if (!buffer) {
-    return false;
-  }
-
-  size_t copied = 0;
-  size_t offset;
-  bool found = false;
-  for (size_t i = 0; i < mSegments.length(); i++) {
-    Segment& segment = mSegments[i];
-    memcpy(buffer + copied, segment.Start(), segment.mSize);
-
-    if (i == aIter.mSegment) {
-      offset = copied + (aIter.mData - segment.Start());
-
-      // Do we have aSize bytes after aIter?
-      if (Size() - offset >= aSize) {
-        found = true;
-        *aOutData = buffer + offset;
-
-        aIter.mSegment = 0;
-        aIter.mData = buffer + offset + aSize;
-        aIter.mDataEnd = buffer + Size();
-      }
-    }
-
-    this->free_(segment.mData);
-
-    copied += segment.mSize;
-  }
-
-  mSegments.clear();
-  mSegments.infallibleAppend(Segment(buffer, Size(), Size()));
-
-  if (!found) {
-    aIter.mSegment = 0;
-    aIter.mData = Start();
-    aIter.mDataEnd = Start() + Size();
-  }
-
-  return found;
-}
-
 template<typename AllocPolicy> template<typename BorrowingAllocPolicy>
 BufferList<BorrowingAllocPolicy>
 BufferList<AllocPolicy>::Borrow(IterImpl& aIter, size_t aSize, bool* aSuccess,
                                 BorrowingAllocPolicy aAP) const
 {
   BufferList<BorrowingAllocPolicy> result(aAP);
 
   size_t size = aSize;
--- a/mfbt/tests/TestBufferList.cpp
+++ b/mfbt/tests/TestBufferList.cpp
@@ -162,39 +162,16 @@ int main(void)
 
   iter = bl.Iter();
   MOZ_RELEASE_ASSERT(iter.AdvanceAcrossSegments(bl, kTotalSize - kLastSegmentSize - kStandardCapacity));
   MOZ_RELEASE_ASSERT(iter.RemainingInSegment() == kStandardCapacity);
   iter.Advance(bl, kStandardCapacity);
   MOZ_RELEASE_ASSERT(iter.RemainingInSegment() == kLastSegmentSize);
   MOZ_RELEASE_ASSERT(unsigned(*iter.Data()) == (kTotalSize - kLastSegmentSize - kInitialSize - kSmallWrite) % 37);
 
-  // Flattening.
-
-  const size_t kFlattenSize = 1000;
-
-  const char* flat;
-  iter = bl.Iter();
-  MOZ_RELEASE_ASSERT(iter.AdvanceAcrossSegments(bl, kInitialSize));
-  MOZ_RELEASE_ASSERT(bl.FlattenBytes(iter, &flat, kFlattenSize));
-  MOZ_RELEASE_ASSERT(flat[0] == 0x0a);
-  MOZ_RELEASE_ASSERT(flat[kSmallWrite / 2] == 0x0a);
-  for (size_t i = kSmallWrite; i < kFlattenSize; i++) {
-    MOZ_RELEASE_ASSERT(unsigned(flat[i]) == (i - kSmallWrite) % 37);
-  }
-  MOZ_RELEASE_ASSERT(unsigned(*iter.Data()) == (kFlattenSize - kSmallWrite) % 37);
-
-  const size_t kSecondFlattenSize = 40;
-
-  MOZ_RELEASE_ASSERT(bl.FlattenBytes(iter, &flat, kSecondFlattenSize));
-  for (size_t i = 0; i < kSecondFlattenSize; i++) {
-    MOZ_RELEASE_ASSERT(unsigned(flat[i]) == (i + kFlattenSize - kInitialSize) % 37);
-  }
-  MOZ_RELEASE_ASSERT(iter.Done());
-
   // Clear.
 
   bl.Clear();
   MOZ_RELEASE_ASSERT(bl.Size() == 0);
   MOZ_RELEASE_ASSERT(bl.Iter().Done());
 
   // Move assignment.