Revert "[PartitionAlloc] Add a flag to allow callers to zero-fill allocations."

This reverts commit b60e5cc0.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 589389 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2I2MGU1Y2MwYmQ0YjM1MWM1ODk4ZGMzNjM3ZjUzN2MzNTU1NDZjMTEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/20069

Sample Failed Step: base_unittests

Original change's description:
> [PartitionAlloc] Add a flag to allow callers to zero-fill allocations.
> 
> This enables callers to avoid doing it themselves (centralizing the `memset` in
> 1 place, rather than duplicating it at every call site that wants it), and to
> allow the PartitionAlloc implementation to avoid doing it itself if the
> operating system has already zero'd the new page(s).
> 
> Bug: 864462,680657
> Change-Id: Ibeae0bf7b9f20238334bf9bc022d0f243a77ef11
> Reviewed-on: https://chromium-review.googlesource.com/1162825
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589389}

Change-Id: Ifd54700a2acb141c458df69304d7ccdce2719cc5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 864462,680657
Reviewed-on: https://chromium-review.googlesource.com/1212346
Cr-Commit-Position: refs/heads/master@{#589404}

base/allocator/partition_allocator/partition_alloc.cc

@@ -235,7 +235,8 @@ bool PartitionReallocDirectMappedInPlace(PartitionRootGeneric* root,
     root->RecommitSystemPages(char_ptr + current_size, recommit_size);
 
 #if DCHECK_IS_ON()
-    memset(char_ptr + current_size, kUninitializedByte, recommit_size);
+    memset(char_ptr + current_size, internal::kUninitializedByte,
+           recommit_size);
 #endif
   } else {
     // We can't perform the realloc in-place.

base/allocator/partition_allocator/partition_alloc.h

@@ -313,6 +313,12 @@ ALWAYS_INLINE void PartitionFree(void* ptr) {
   internal::PartitionPage* page = internal::PartitionPage::FromPointer(ptr);
   // TODO(palmer): See if we can afford to make this a CHECK.
   DCHECK(internal::PartitionRootBase::IsValidPage(page));
+
+  // This is somewhat redundant with |PartitionPage::Free|.
+  // TODO(crbug.com/680657): Doing this here might? make it OK to not do it
+  // there.
+  memset(original_ptr, 0xCD, PartitionAllocGetSize(original_ptr));
+
   page->Free(ptr);
 #endif
 }
@@ -339,10 +345,9 @@ ALWAYS_INLINE void* PartitionAllocGenericFlags(PartitionRootGeneric* root,
                                                size_t size,
                                                const char* type_name) {
   DCHECK_LT(flags, PartitionAllocLastFlag << 1);
-  const bool zero_fill = flags & PartitionAllocZeroFill;
 
 #if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
-  void* result = zero_fill ? calloc(1, size) : malloc(size);
+  void* result = malloc(size);
   CHECK(result || flags & PartitionAllocReturnNull);
   return result;
 #else
@@ -356,14 +361,6 @@ ALWAYS_INLINE void* PartitionAllocGenericFlags(PartitionRootGeneric* root,
     ret = root->AllocFromBucket(bucket, flags, size);
   }
   PartitionAllocHooks::AllocationHookIfEnabled(ret, requested_size, type_name);
-
-  // TODO(crbug.com/864462): This is suboptimal. Change `AllocFromBucket` such
-  // that it tells callers if the allocation was satisfied with a fresh mapping
-  // from the OS, so that we can skip this step and save some time.
-  if (zero_fill) {
-    memset(ret, 0, requested_size);
-  }
-
   return ret;
 #endif
 }

base/allocator/partition_allocator/partition_alloc_constants.h

@@ -151,16 +151,11 @@ static const size_t kMaxFreeableSpans = 16;
 // "out of physical memory" in crash reports.
 static const size_t kReasonableSizeOfUnusedPages = 1024 * 1024 * 1024;  // 1GB
 
-// These two byte values match tcmalloc.
-static const unsigned char kUninitializedByte = 0xAB;
-static const unsigned char kFreedByte = 0xCD;
-
 // Flags for PartitionAllocGenericFlags.
 enum PartitionAllocFlags {
   PartitionAllocReturnNull = 1 << 0,
-  PartitionAllocZeroFill = 1 << 1,
 
-  PartitionAllocLastFlag = PartitionAllocZeroFill
+  PartitionAllocLastFlag = PartitionAllocReturnNull
 };
 
 }  // namespace base

base/allocator/partition_allocator/partition_alloc_unittest.cc

@@ -2158,36 +2158,6 @@ TEST_F(PartitionAllocTest, SmallReallocDoesNotMoveTrailingCookie) {
   generic_allocator.root()->Free(ptr);
 }
 
-TEST_F(PartitionAllocTest, ZeroFill) {
-  const size_t test_sizes[] = {
-      1,
-      17,
-      100,
-      kSystemPageSize,
-      kSystemPageSize + 1,
-      internal::PartitionBucket::get_direct_map_size(100),
-      1 << 20,
-      1 << 21,
-  };
-
-  constexpr static size_t kAllZerosSentinel =
-      std::numeric_limits<size_t>::max();
-  for (size_t size : test_sizes) {
-    char* p = static_cast<char*>(PartitionAllocGenericFlags(
-        generic_allocator.root(), PartitionAllocZeroFill, size, nullptr));
-    size_t non_zero_position = kAllZerosSentinel;
-    for (size_t i = 0; i < size; ++i) {
-      if (0 != p[i]) {
-        non_zero_position = i;
-        break;
-      }
-    }
-    EXPECT_EQ(kAllZerosSentinel, non_zero_position)
-        << "test allocation size: " << size;
-    PartitionFree(p);
-  }
-}
-
 }  // namespace internal
 }  // namespace base
 

base/allocator/partition_allocator/partition_bucket.cc

@@ -37,6 +37,8 @@ ALWAYS_INLINE PartitionPage* PartitionDirectMap(PartitionRootBase* root,
   map_size += kPageAllocationGranularityOffsetMask;
   map_size &= kPageAllocationGranularityBaseMask;
 
+  // TODO: these pages will be zero-filled. Consider internalizing an
+  // AllocZeroed() API so we can avoid a memset() entirely in this case.
   char* ptr = reinterpret_cast<char*>(
       AllocPages(nullptr, map_size, kSuperPageSize, PageReadWrite));
   if (UNLIKELY(!ptr))

base/allocator/partition_allocator/partition_cookie.h

@@ -12,9 +12,11 @@ namespace base {
 namespace internal {
 
 #if DCHECK_IS_ON()
-// Handles alignment up to XMM instructions on Intel.
-static const size_t kCookieSize = 16;
-
+// These two byte values match tcmalloc.
+static const unsigned char kUninitializedByte = 0xAB;
+static const unsigned char kFreedByte = 0xCD;
+static const size_t kCookieSize =
+    16;  // Handles alignment up to XMM instructions on Intel.
 static const unsigned char kCookieValue[kCookieSize] = {
     0xDE, 0xAD, 0xBE, 0xEF, 0xCA, 0xFE, 0xD0, 0x0D,
     0x13, 0x37, 0xF0, 0x05, 0xBA, 0x11, 0xAB, 0x1E};

base/allocator/partition_allocator/partition_page.h

@@ -198,25 +198,17 @@ ALWAYS_INLINE size_t PartitionPage::get_raw_size() const {
 }
 
 ALWAYS_INLINE void PartitionPage::Free(void* ptr) {
+// If these asserts fire, you probably corrupted memory.
+#if DCHECK_IS_ON()
   size_t slot_size = this->bucket->slot_size;
-  const size_t raw_size = get_raw_size();
-  if (raw_size) {
+  size_t raw_size = get_raw_size();
+  if (raw_size)
     slot_size = raw_size;
-  }
-
-#if DCHECK_IS_ON()
-  // If these asserts fire, you probably corrupted memory.
   PartitionCookieCheckValue(ptr);
   PartitionCookieCheckValue(reinterpret_cast<char*>(ptr) + slot_size -
                             kCookieSize);
-#endif
-
-  // Perhaps surprisingly, this does not measurably regress performance. See
-  // https://crbug.com/680657 for history. We formerly did this in
-  // |PartitionFree|, and in `DCHECK_IS_ON()` builds we redundantly did it
-  // here, too. Now we only do it here, unconditionally.
   memset(ptr, kFreedByte, slot_size);
-
+#endif
   DCHECK(this->num_allocated_slots);
   // TODO(palmer): See if we can afford to make this a CHECK.
   // FIX FIX FIX