Revert "Revert 239763 "Add DiscardableMemoryAllocator to minimize use of...""

base/memory/discardable_memory_unittest.cc

Index: base/memory/discardable_memory_unittest.cc
diff --git a/base/memory/discardable_memory_unittest.cc b/base/memory/discardable_memory_unittest.cc
index eb730f19f18e8482fde972598c99378f2c30447a..96f5a89e0e747f92757619540702487550c5870c 100644
--- a/base/memory/discardable_memory_unittest.cc
+++ b/base/memory/discardable_memory_unittest.cc
@@ -3,12 +3,32 @@
 // found in the LICENSE file.
 
 #include "base/memory/discardable_memory.h"
+
+#include <limits>

[reveman, 2013/12/10 16:25:46]

#if defined(OS_ANDROID) to only include what you need

+
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
 const size_t kSize = 1024;
 
+#if defined(OS_ANDROID)
+// TODO(pliard): enable this on all platforms. This currently fails when

[Philippe, 2013/12/10 15:18:38]

FYI, I'm not proud of doing this but I want to get the allocator in :) I wish
there was a compile time flag letting us know that the file is being built with
ASAN on but there isn't. It wasn't clear to me either how to disable this test
with ASAN. There was some TSAN, LSAN... suppression files but I didn't see
anything for ASAN. I believe David added his own test anyway to check this in
the provider unit tests. The test doesn't cover the code path where malloc()
returns NULL though (but the production code does correctly check the return
value of malloc()) which is why it didn't fail with ASAN.

[reveman, 2013/12/10 15:56:32]

I don't think we should have a test that pass a huge size to malloc and expect
it to fail. Can this be an allocator unit test instead of a general DM test?

[Philippe, 2013/12/10 16:00:28]

Yeah, agreed. I need to keep it here ideally on Android though since the code
dealing with this is not in the allocator. The allocator does indirectly check
that it's safe to do page alignment though. I can remove the TODO/comment and
just keep the ANDROID ifdef. How would you feel about that?

[Philippe, 2013/12/10 16:15:07]

I would be ok with moving it to the allocator unit tests if you feel strongly
though (which would not be the right place IMO since this test doesn't exerce
the allocator code).

[reveman, 2013/12/10 16:25:46]

I'm fine with an ANDROID ifdef for now.

I think the the DiscardableMemory framework is in need of a general refactor to
be more modular and less ifdef-y. I'm looking at this as part of using ashmem on
ChromeOS.

[Philippe, 2013/12/10 16:38:43]

Done, thanks.

+// malloc() is used (by discardable memory emulation) with ASAN due to ASAN
+// asserting that malloc() doesn't return NULL.
+TEST(DiscardableMemoryTest, TooLargeAllocationFails) {
+  const size_t kPageSize = 4096;
+  const size_t max_allowed_allocation_size =
+      std::numeric_limits<size_t>::max() - kPageSize + 1;
+  scoped_ptr<DiscardableMemory> memory(
+      DiscardableMemory::CreateLockedMemory(max_allowed_allocation_size + 1));
+  // On certain platforms (e.g. Android), page-alignment would have caused an
+  // overflow resulting in a small allocation if the input size wasn't checked
+  // correctly.
+  ASSERT_FALSE(memory);
+}
+#endif
+
 TEST(DiscardableMemoryTest, SupportedNatively) {
 #if defined(DISCARDABLE_MEMORY_ALWAYS_SUPPORTED_NATIVELY)
   ASSERT_TRUE(DiscardableMemory::SupportedNatively());