Make GrScratchKey memory buffer correct size on copy

Make GrScratchKey memory buffer correct size on copy

Scratch key memory buffer of a copy of a key was too big. The (new) copy
was N times uint32_t bytes instead of N bytes.

Adds few tests to resource cache. These tests would not catch the too
big buffer. This is just a precaution for too small buffers. The main
idea of the test change is that the scratch key should contain some
information, so that lookup with a scratch key can also return no
match. Otherwise testing of scratch lookup result is not indicative of
correct code (eg. no-information scratch key will always match).

Committed: https://skia.googlesource.com/skia/+/711ef4831363fb8cbdf061dc2c36c65b13c0ccf2

[Kimmo Kinnunen, 2015-01-21 10:26:59]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com

[Kimmo Kinnunen, 2015-01-21 10:46:02]

Personal preference would be to make Builder generic to the size, since the size
defines the type of the data.. 

However, maybe this would be easier to review and get in.

[bsalomon, 2015-01-21 14:10:12]

lgtm, nice catch and thanks for the additional tests. What do you mean by making
the builder generic to the size? A template?

[Kimmo Kinnunen, 2015-01-21 14:15:17]

On 2015/01/21 14:10:12, bsalomon wrote:
> lgtm, nice catch and thanks for the additional tests. What do you mean by
making
> the builder generic to the size? A template?

Yeah. current, dynamic typing:
GrScratchKey::Builder builder(key, kType, 2);

Builder polymorphic to the size (static typing):
GrScratchKey::Builder<2> builder(key, kType);

Not a big deal..

[Kimmo Kinnunen, 2015-01-21 14:15:43]

The CQ bit was checked by kkinnunen@nvidia.com

[commit-bot: I haz the power, 2015-01-21 14:16:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/860333002/1

[commit-bot: I haz the power, 2015-01-21 14:39:18]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/711ef4831363fb8cbdf061dc2c36c65b13c0ccf2

[mtklein, 2015-01-21 16:04:51]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/864833003/ by mtklein@google.com.

The reason for reverting is: Our Valgrind bot just spewed out a weird error.  I
don't know if it's related, but it looks at least like one of the stacks was in
the right area, so I'm going to revert this precautionarily.  Sorry if this is a
false positive.

http://build.chromium.org/p/client.skia/builders/Test-Ubuntu12-ShuttleA-GTX55....

[mtklein, 2015-01-21 16:23:08]

On 2015/01/21 16:04:51, mtklein wrote:
> A revert of this CL (patchset #1 id:1) has been created in
> https://codereview.chromium.org/864833003/ by mailto:mtklein@google.com.
> 
> The reason for reverting is: Our Valgrind bot just spewed out a weird error. 
I
> don't know if it's related, but it looks at least like one of the stacks was
in
> the right area, so I'm going to revert this precautionarily.  Sorry if this is
a
> false positive.
> 
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu12-ShuttleA-GTX55....

The test was failing too:

Failures:
	../../tests/ResourceCacheTest.cpp:523	scratchKey1.size() <= sizeof(scratchKey1)
	../../tests/ResourceCacheTest.cpp:527	scratchKey.size() <= sizeof(scratchKey)
	../../tests/ResourceCacheTest.cpp:533	scratchKey.size() <= sizeof(scratchKey)

[bsalomon, 2015-01-21 16:25:04]

On 2015/01/21 16:04:51, mtklein wrote:
> A revert of this CL (patchset #1 id:1) has been created in
> https://codereview.chromium.org/864833003/ by mailto:mtklein@google.com.
> 
> The reason for reverting is: Our Valgrind bot just spewed out a weird error. 
I
> don't know if it's related, but it looks at least like one of the stacks was
in
> the right area, so I'm going to revert this precautionarily.  Sorry if this is
a
> false positive.
> 
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu12-ShuttleA-GTX55....

Attempting to reland here: https://codereview.chromium.org/863983003