Android should be able to discard CachedResources

Android should be able to discard CachedResources

This CL removes the Mac OS X specific implemenation of PurgeableBuffer in favor
of one built on top of Chromium's WebDiscardableMemory abstraction. Moving this
code to WebDiscardableMemory means that PurgeableBuffer now works on Android.
Specifically, that means that the Android kernel can now evict CachedResources
from the MemoryCache when the system is under memory pressure.

R=eseidel

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=151877

[abarth-chromium, 2013-06-05 19:33:37]

@jamesr: Would you be willing to review this CL?

[eseidel, 2013-06-05 19:39:47]

lgtm

I find it difficult to believe that this is worth the code complexity for this
win.  But this is definitely better than what we had before.

[commit-bot: I haz the power, 2013-06-05 19:40:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/16379004/7001

[commit-bot: I haz the power, 2013-06-05 19:40:08]

Failed to apply patch for Source/core/core.gyp/core.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         Source/core/core.gyp
  Created missing directory Source/core/core.gyp.
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: Source/core/core.gyp/core.gyp
  |diff --git a/Source/core/core.gyp/core.gyp b/Source/core/core.gyp/core.gyp
  |index
b46ece6b54d7c61ec945e68c26a379a6a8c0d421..c80e6207d9e980ec30aa671cb998e109fcf656a4
100644
  |--- a/Source/core/core.gyp/core.gyp
  |+++ b/Source/core/core.gyp/core.gyp
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:       Source/core/core.gyp/core.gyp
Index: Source/core/core.gyp/core.gyp
diff --git a/Source/core/core.gyp/core.gyp b/Source/core/core.gyp/core.gyp
index
b46ece6b54d7c61ec945e68c26a379a6a8c0d421..c80e6207d9e980ec30aa671cb998e109fcf656a4
100644
--- a/Source/core/core.gyp/core.gyp
+++ b/Source/core/core.gyp/core.gyp
@@ -713,7 +713,6 @@
             ['include', 'platform/mac/KillRingMac\\.mm$'],
             ['include', 'platform/mac/LocalCurrentGraphicsContext\\.mm$'],
             ['include', 'platform/mac/NSScrollerImpDetails\\.mm$'],
-            ['include', 'platform/mac/PurgeableBufferMac\\.cpp$'],
             ['include', 'platform/mac/ScrollbarThemeMac\\.mm$'],
             ['include', 'platform/mac/ScrollAnimatorMac\\.mm$'],
             ['include', 'platform/mac/ScrollElasticityController\\.mm$'],

[jamesr, 2013-06-05 19:41:25]

I can definitely review it.

This patch won't work as is, it'll DCHECK fail on all non-mac/android platforms.
 Hold off until I can take a careful look.

[jamesr, 2013-06-05 19:52:15]

Ah, I missed that the platform support implementation checks if the primitive is
supported before returning a WebDiscardableMemory.  This should work.

Implementation lgtm, but you should check with the folks using the discardable
memory on android for images first to make sure we understand how this'll react
with that.  We probably don't want to evict images to cache stylesheets, for
instance.  Adding hclam/qinmin as a first approximation.

[abarth-chromium, 2013-06-05 19:55:13]

On 2013/06/05 19:52:15, jamesr wrote:
> Ah, I missed that the platform support implementation checks if the primitive
is
> supported before returning a WebDiscardableMemory.  This should work.

Oh, good.  You had me worried there.  :)

> Implementation lgtm, but you should check with the folks using the discardable
> memory on android for images first to make sure we understand how this'll
react
> with that.  We probably don't want to evict images to cache stylesheets, for
> instance.  Adding hclam/qinmin as a first approximation.

Does Android have a notion of priority for discardable memory?  In any case,
this CL only makes it easier to retain images because we're moving from
never-evictable memory to evictable memory without increasing the size of the
cache.

[Avi (use Gerrit), 2013-06-05 19:59:41]

On 2013/06/05 19:55:13, abarth wrote:
> On 2013/06/05 19:52:15, jamesr wrote:
> > Ah, I missed that the platform support implementation checks if the
primitive
> is
> > supported before returning a WebDiscardableMemory.  This should work.
> 
> Oh, good.  You had me worried there.  :)
> 
> > Implementation lgtm, but you should check with the folks using the
discardable
> > memory on android for images first to make sure we understand how this'll
> react
> > with that.  We probably don't want to evict images to cache stylesheets, for
> > instance.  Adding hclam/qinmin as a first approximation.
> 
> Does Android have a notion of priority for discardable memory?  In any case,
> this CL only makes it easier to retain images because we're moving from
> never-evictable memory to evictable memory without increasing the size of the
> cache.

We designed base::DiscardableMemory as basically the lowest-common-denominator
of the Mac/Win32/Linux discardable memory systems. (The windows guys chose to
not ship a Win32 implementation because it was Win8-only which was apparently
too small a fraction to work well.) The Mac has priority, Win32 definitely
doesn't, and ashmem might but I don't believe so.

If priority would make base::DiscardableMemory let us know, but is it worth
adding it for just one platform?

[abarth-chromium, 2013-06-05 20:03:22]

On 2013/06/05 19:59:41, Avi wrote:
> If priority would make base::DiscardableMemory let us know, but is it worth
> adding it for just one platform?

My guess is that we won't need priorities.

[commit-bot: I haz the power, 2013-06-05 22:23:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/16379004/17001

[commit-bot: I haz the power, 2013-06-05 22:34:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/16379004/17001

[Alpha Left Google, 2013-06-05 22:35:01]

LGTM.

I believe this allows purging of SharedBuffer which contains raw data (encoded
data). Purging of decoded resources needs to be implemented separately.

[abarth-chromium, 2013-06-05 22:45:18]

On 2013/06/05 22:35:01, Alpha wrote:
> I believe this allows purging of SharedBuffer which contains raw data (encoded
> data). Purging of decoded resources needs to be implemented separately.

Correct.

[commit-bot: I haz the power, 2013-06-06 02:13:21]

Change committed as 151877