cc: Fix incomplete changes to add ETC1 support

cc: Fix incomplete changes to add ETC1 support

ETC1 resources was recently added (https://codereview.chromium.org/27973002),
but the change did not cover the LayerTreeHostImpl path, which cause crashes
when one tries to use ETC1 UIResourceBitmap.  This update adds in the missing
changes.

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

[powei, 2013-10-25 18:28:49]

Missed one necessary change in last patch.  PTAL.  Thanks.

[aelias_OOO_until_Jul13, 2013-10-25 22:56:49]

OK, could you add a LayerTreeHostImpl test case for this?

[powei, 2013-10-29 18:10:07]

Added a test and a pixel-ref class.  PTAL.  Thanks.

[aelias_OOO_until_Jul13, 2013-10-29 18:29:37]

https://codereview.chromium.org/41903003/diff/210001/cc/resources/etc1_pixel_...
File cc/resources/etc1_pixel_ref.h (right):

https://codereview.chromium.org/41903003/diff/210001/cc/resources/etc1_pixel_...
cc/resources/etc1_pixel_ref.h:15: class ETC1PixelRef : public SkPixelRef {
I think we can just use SkMallocPixelRef instead?  It provides the same
functionality as this (holds the size).

[aelias_OOO_until_Jul13, 2013-10-31 02:10:55]

As discussed offline, SkMallocPixelRef doesn't have SK_API so we can't use it,
and on second thought, I don't think Skia will agree to expose it because it
would defeat their attempt to hide the difference between software and hardware.

I don't think we care about the allocator used for the bitmap, just the Skia
refcounting behavior.  How about we make the UIResourceBitmap constructor take a
Chromium scoped_array_ptr<void*>, and hide the ETC1 SkPixelRef subclass in
ui_resource_bitmap.cc and just have it hold onto that scoped_ptr without the
intermediate SkData thing.

[powei, 2013-10-31 02:27:21]

On 2013/10/31 02:10:55, aelias wrote:
> As discussed offline, SkMallocPixelRef doesn't have SK_API so we can't use it,
> and on second thought, I don't think Skia will agree to expose it because it
> would defeat their attempt to hide the difference between software and
hardware.
> 
> I don't think we care about the allocator used for the bitmap, just the Skia
> refcounting behavior.  How about we make the UIResourceBitmap constructor take
a
> Chromium scoped_array_ptr<void*>, and hide the ETC1 SkPixelRef subclass in
> ui_resource_bitmap.cc and just have it hold onto that scoped_ptr without the
> intermediate SkData thing.

Sounds good.  I'll do that.

[powei, 2013-10-31 21:47:26]

PTAL.

https://codereview.chromium.org/41903003/diff/210001/cc/resources/etc1_pixel_...
File cc/resources/etc1_pixel_ref.h (right):

https://codereview.chromium.org/41903003/diff/210001/cc/resources/etc1_pixel_...
cc/resources/etc1_pixel_ref.h:15: class ETC1PixelRef : public SkPixelRef {
On 2013/10/29 18:29:38, aelias wrote:
> I think we can just use SkMallocPixelRef instead?  It provides the same
> functionality as this (holds the size).

Done.  SkMallocPixelRef is not exposed.  Moved implementation to inside
UIResourceBitmap.

[aelias_OOO_until_Jul13, 2013-10-31 21:59:09]

lgtm!

[commit-bot: I haz the power, 2013-11-04 18:09:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/powei@chromium.org/41903003/540001

[commit-bot: I haz the power, 2013-11-04 18:43:57]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[powei, 2013-11-04 18:47:57]

+yfriedman@ for compositor.h OWNER

PTAL, Yaron.  Thank you.

[Yaron, 2013-11-04 18:51:25]

lgtm

This needs a tracking bug and a BUG= line in the cl description.

[powei, 2013-11-04 19:05:16]

On 2013/11/04 18:51:25, Yaron wrote:
> lgtm
> 
> This needs a tracking bug and a BUG= line in the cl description.

Done and thanks.

[commit-bot: I haz the power, 2013-11-04 19:12:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/powei@chromium.org/41903003/540001

[commit-bot: I haz the power, 2013-11-04 21:50:32]

Retried try job too often on win_rel for step(s) telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[powei, 2013-11-07 19:05:08]

PTAL.  Thanks.  After offline discussion, we decided to expose the pixel ref
class.    

And sorry for the delay; I was tracking down a related crash.  Resolved now.

[aelias_OOO_until_Jul13, 2013-11-07 19:30:26]

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/etc1_pixel...
File cc/resources/etc1_pixel_ref.h (right):

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/etc1_pixel...
cc/resources/etc1_pixel_ref.h:18: explicit ETC1PixelRef(scoped_ptr<uint8_t[]>
pixels);
Could you also make this take a gfx::Size in the constructor and store it?

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/ui_resourc...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:49: gfx::Size size);
Then this doesn't need a gfx::Size either.

https://codereview.chromium.org/41903003/diff/1170001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/41903003/diff/1170001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.h:64: virtual
cc::UIResourceId GenerateCompressedUIResource(gfx::Size& size,
Can we delete this method?  It steals ownership of the memory in a way that
doesn't fit the use case for thumbnails, and seems redundant with
GenerateUIResource.

[powei, 2013-11-07 19:45:31]

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/etc1_pixel...
File cc/resources/etc1_pixel_ref.h (right):

https://codereview.chromium.org/41903003/diff/1170001/cc/resources/etc1_pixel...
cc/resources/etc1_pixel_ref.h:18: explicit ETC1PixelRef(scoped_ptr<uint8_t[]>
pixels);
On 2013/11/07 19:30:27, aelias wrote:
> Could you also make this take a gfx::Size in the constructor and store it?

Then both UIResourceBitmap and this class will hold a gfx::Size?  Also, seems
like Skia subclasses of SkPixelRef typically do not store the size.  Maybe we
need another wrapper class (ETC1Bitmap) like how SkBitmap wraps around
SkPixelRef?

https://codereview.chromium.org/41903003/diff/1170001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/41903003/diff/1170001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.h:64: virtual
cc::UIResourceId GenerateCompressedUIResource(gfx::Size& size,
On 2013/11/07 19:30:27, aelias wrote:
> Can we delete this method?  It steals ownership of the memory in a way that
> doesn't fit the use case for thumbnails, and seems redundant with
> GenerateUIResource.

Done.

[aelias_OOO_until_Jul13, 2013-11-07 22:28:22]

OK, the size thing doesn't really matter, let's get this landed as is.  lgtm

[commit-bot: I haz the power, 2013-11-08 00:39:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/powei@chromium.org/41903003/1270001

[commit-bot: I haz the power, 2013-11-08 01:31:58]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-08 09:40:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/powei@chromium.org/41903003/1630001

[commit-bot: I haz the power, 2013-11-08 12:04:18]

Change committed as 233874