base: Refactor DiscardableMemoryProvider to build and be tested on all platforms.

base/memory/discardable_memory_emulated.h

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_MEMORY_DISCARDABLE_MEMORY_EMULATED_H_
+#define BASE_MEMORY_DISCARDABLE_MEMORY_EMULATED_H_
+
+#include "base/memory/discardable_memory.h"
+
+namespace base {
+
+class DiscardableMemoryEmulated : public DiscardableMemory {

[Philippe, 2013/12/11 08:56:27]

Should we have this in base::internal?

[reveman, 2013/12/12 06:54:23]

Done.

+ public:
+  explicit DiscardableMemoryEmulated(size_t size);
+  virtual ~DiscardableMemoryEmulated();
+
+  static void PurgeForTesting();
+
+  bool Initialize();

[Philippe, 2013/12/11 08:56:27]

Just a suggestion, feel free to ignore :)

I see that we have this pattern now in a few files:
scoped_ptr<DiscardableMemoryEmulated> memory(
    new DiscardableMemoryEmulated(size));
if (!memory->Initialize())
  return scoped_ptr<DiscardableMemoryEmulated>();
return memory.PassAs<DiscardableMemory>();

If you make this Initialize() method and the constructor private and add a
Create() public static factory method returning a scoped_ptr then we could
decrease slightly the boilerplate on the client site.

What do you think?

[reveman, 2013/12/12 06:54:23]

I prefer to avoid static Create functions when possible as it forces us to write
more reusable code. ie. the functionality in DiscardableMemoryEmulated can
easily be reused by creating a derived class or moving some of the functionality
to a separate base class that can be inherited from.

We're using the same pattern for some of the types in ui/gl and
GpuMemoryBufferTypeImpl found in content/common/gpu/client/.

[Philippe, 2013/12/12 08:53:35]

Fair enough, I would tend to avoid such inheritance though and prefer
composition. But then the static factory method doesn't play nicely either with
composition as it comes with its own heap allocation. So I guess you are
planning to reuse/extend DiscardableMemoryEmulated?

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

Composition can definitely make more sense in some cases and this doesn't
prevent us from using that. This is more about putting some pattern in place
that makes the code easier to extend and maintain. This pattern works well in my
experience for code that requires platform specific implementations.

DiscardableMemoryEmulated might not be reused specifically but future ashmem
code that needs to be used on both android and chromeos will be.

+
+  // Overridden from DiscardableMemory:
+  virtual LockDiscardableMemoryStatus Lock() OVERRIDE;
+  virtual void Unlock() OVERRIDE;
+  virtual void* Memory() const OVERRIDE;
+
+ private:
+  scoped_ptr<uint8, FreeDeleter> memory_;
+  bool is_locked_;
+
+  DISALLOW_COPY_AND_ASSIGN(DiscardableMemoryEmulated);
+};
+
+}  // namespace base
+
+#endif  // BASE_MEMORY_DISCARDABLE_MEMORY_EMULATED_H_