Set ImageCacheSingleAllocationByteLimit to 64MB

Set ImageCacheSingleAllocationByteLimit to 64MB 

This variable is the maximum allocation size allowed for
(high-quality) image scaling filters that require pre-scaling (and
caching of the results). Skia will fallback to a filter that doesn't
require pre-scaling if the selected filter would require an allocation
that exceeds this limit.

Motivation: If a web page scales an image by a large factor, the
resulting bitmap may take up quite a bit of space - this will clobber
Skia's Scaled Image Cache and other caches that share a quota with the
Scaled Image Cache.

See SkGraphics.h for more documentation. 

Future work: Someone may want to tune this value based on device
memory size.

BUG=389439
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284793

[hal.canary, 2014-07-16 23:24:20]

As requested.

I'm not sure the best value to put in here.  The bug was triggered at 256 MB,
which is obviously too large.  64MB is a 4000x4000 image, which is still pretty
large, but could happen on a large high-dpi screen.

[reveman, 2014-07-17 15:33:21]

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:907:
SkGraphics::SetImageCacheSingleAllocationByteLimit(64 * 1024 * 1024);
Can you move "64 * 1024 * 1024" to a constant defined with other constants in
anonymous namespace and add a comment next to it explaining what it's for? I'm
thinking something along the lines:

"Maximum allocation size allowed for image scaling filters that require
pre-scaling. Skia will fallback to a filter that doesn't require pre-scaling if
the default filter would require an allocation that exceeds this limit."

[reveman, 2014-07-17 15:36:06]

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:907:
SkGraphics::SetImageCacheSingleAllocationByteLimit(64 * 1024 * 1024);
nit: move this call up next to the other SkGraphics call to keep these scaled
image cache related adjustments grouped together

[tomhudson, 2014-07-17 15:46:56]

On 2014/07/17 15:33:21, reveman wrote:
> "Maximum allocation size allowed for image scaling filters that require
> pre-scaling. Skia will fallback to a filter that doesn't require pre-scaling
if
> the default filter would require an allocation that exceeds this limit."

Is that explicit Skia behavior, or is it implicit somehow in the
Blink/Chrome/Skia interaction?
Either way, let's reference where in the codebase it happens, so at least people
can realize more easily when it becomes wrong.

[hal.canary, 2014-07-17 16:55:07]

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:907:
SkGraphics::SetImageCacheSingleAllocationByteLimit(64 * 1024 * 1024);
On 2014/07/17 15:33:21, reveman wrote:
> Can you move "64 * 1024 * 1024" to a constant defined with other constants in
> anonymous namespace and add a comment next to it explaining what it's for? I'm
> thinking something along the lines:
> 
> "Maximum allocation size allowed for image scaling filters that require
> pre-scaling. Skia will fallback to a filter that doesn't require pre-scaling
if
> the default filter would require an allocation that exceeds this limit."

Done.

https://codereview.chromium.org/397193002/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:907:
SkGraphics::SetImageCacheSingleAllocationByteLimit(64 * 1024 * 1024);
On 2014/07/17 15:36:06, reveman wrote:
> nit: move this call up next to the other SkGraphics call to keep these scaled
> image cache related adjustments grouped together

Done.

[reveman, 2014-07-17 17:08:05]

lgtm but you also need an owner review for content/renderer/

maybe update the patch description with something similar to the comment above
the constant instead of the current "Depends on Skia CL.." message

[hal.canary, 2014-07-18 14:28:49]

On 2014/07/17 17:08:05, reveman wrote:
> lgtm but you also need an owner review for content/renderer/
> 
> maybe update the patch description with something similar to the comment above
> the constant instead of the current "Depends on Skia CL.." message

Done and Done.

[reveman, 2014-07-22 16:24:04]

please add BUG=389439 to the description.

jamesr@, ping for review

[jamesr, 2014-07-22 18:07:45]

lgtm

[hal.canary, 2014-07-22 18:12:19]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-07-22 18:14:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/halcanary@google.com/397193002/20001

[commit-bot: I haz the power, 2014-07-22 21:31:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-22 23:14:07]

Change committed as 284793