ui/android: Fix Resource meta-data sharing with ResourceManager.

ui/android: Fix Resource meta-data sharing with ResourceManager.

The android native resource management system provides a
ui::ResourceManager::Resource, which pulls data from the java Resource,
to provide meta-data to the code building cc::Layers for drawing this
resource.

The resource exposes padding and aperture, which are well defined
concepts for a nine-patch bitmap, but are arbitrarily set in all other
cases. In static bitmaps they are set to the bitmap size, while for
dynamic resources they could represent anything.

The change pulls out a native Resource class which is a container for
the UIResource built from the corresponding java resource. The java
Resource API now has a method which lets it build its native
representation that can hold the meta-data to be used for drawing it
using a cc::Layer. Currently we still build a NinePatchResource for
dynamic resources, follow up changes will fix that to add correct
native Resource subclasses.

BUG=700454
Review-Url: https://codereview.chromium.org/2746483003
Cr-Commit-Position: refs/heads/master@{#457655}
Committed: https://chromium.googlesource.com/chromium/src/+/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e

[Khushal, 2017-03-09 23:05:37]

khushalsagar@chromium.org changed reviewers:
+ dtrainor@chromium.org, mdjones@chromium.org

[Khushal, 2017-03-09 23:05:38]

It should be a completely mechanical change, I still have to test it out to make
sure it didn't break anything. Does this look reasonable?

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource_manager_impl.cc (left):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource_manager_impl.cc:202: if (item ==
resources_[res_type].end()) {
I was worried about making repeated dynamic allocations here now. But I am
assuming this was only avoiding it for the case where a dynamic resource is
updated, which should be rare?

[mdjones, 2017-03-10 18:12:00]

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/nine_p...
File ui/android/resources/nine_patch_resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/nine_p...
ui/android/resources/nine_patch_resource.cc:13:
DCHECK_EQ(Type::NINE_PATCH_BITMAP, resource->type());
Where is type_ set for this class?

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.cc:14: Resource::Resource(Type type) : type_(type)
{}
Shouldn't a resource class know what type it is?

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.h (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.h:17: enum class Type { BITMAP, NINE_PATCH_BITMAP,
UNKNOWN };
What is UNKNOWN used for?

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource_manager_impl.cc (left):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource_manager_impl.cc:202: if (item ==
resources_[res_type].end()) {
On 2017/03/09 23:05:37, Khushal wrote:
> I was worried about making repeated dynamic allocations here now. But I am
> assuming this was only avoiding it for the case where a dynamic resource is
> updated, which should be rare?

Dynamic resources use the same id when they are updated which is pretty often.
Tab titles are a good example. However, the bulk of the allocation happens in
the bitmap creation logic below which happens regardless of this.

[Khushal, 2017-03-10 19:19:24]

Okay, I tested this with the tab switcher and overlay and the world does not
burn. PTAL.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/nine_p...
File ui/android/resources/nine_patch_resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/nine_p...
ui/android/resources/nine_patch_resource.cc:13:
DCHECK_EQ(Type::NINE_PATCH_BITMAP, resource->type());
On 2017/03/10 18:12:00, mdjones wrote:
> Where is type_ set for this class?

Woops. I didn't realize the code here was using the default Resource ctor.
Note to self: next time actually run the code before sending for review. :P

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.cc:14: Resource::Resource(Type type) : type_(type)
{}
On 2017/03/10 18:12:00, mdjones wrote:
> Shouldn't a resource class know what type it is?

Is this about the UNKNOWN case? The type is stored on Resource so a user can
safely downcast it.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.h (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.h:17: enum class Type { BITMAP, NINE_PATCH_BITMAP,
UNKNOWN };
On 2017/03/10 18:12:00, mdjones wrote:
> What is UNKNOWN used for?

I was planning on using it for sub-classes outside on ui/. Like for the Toolbar
resource. But may be we can still have these types be declared in the enum here,
even if the sub-class exists in chrome/?

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource_manager_impl.cc (left):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource_manager_impl.cc:202: if (item ==
resources_[res_type].end()) {
On 2017/03/10 18:12:00, mdjones wrote:
> On 2017/03/09 23:05:37, Khushal wrote:
> > I was worried about making repeated dynamic allocations here now. But I am
> > assuming this was only avoiding it for the case where a dynamic resource is
> > updated, which should be rare?
> 
> Dynamic resources use the same id when they are updated which is pretty often.

True, and I assumed these updates are quite rare. Like the toolbar will be
invalidated and updated only when the text changes, right?

> Tab titles are a good example. However, the bulk of the allocation happens in
> the bitmap creation logic below which happens regardless of this.

[Khushal, 2017-03-10 19:19:49]

Description was changed from

==========
ui/android: Fix Resource meta-data sharing with ResourceManager.

The android native resource management system provides a
ui::ResourceManager::Resource, which pulls data from the java Resource,
to provide meta-data to the code building cc::Layers for drawing this
resource.

The resource exposes padding and aperture, which are well defined
concepts for a nine-patch bitmap, but are arbitrarily set in all other
cases. In static bitmaps they are set to the bitmap size, while for
dynamic resources they could represent anything.

The change pulls out a native Resource class which is a container for
the UIResource built from the corresponding java resource. The java
Resource API now has a method which lets it build its native
representation that can hold the meta-data to be used for drawing it
using a cc::Layer. Currently we still build a NinePatchResource for
dynamic resources, follow up changes will fix that to add correct
native Resource subclasses.

BUG=
==========

to

==========
ui/android: Fix Resource meta-data sharing with ResourceManager.

The android native resource management system provides a
ui::ResourceManager::Resource, which pulls data from the java Resource,
to provide meta-data to the code building cc::Layers for drawing this
resource.

The resource exposes padding and aperture, which are well defined
concepts for a nine-patch bitmap, but are arbitrarily set in all other
cases. In static bitmaps they are set to the bitmap size, while for
dynamic resources they could represent anything.

The change pulls out a native Resource class which is a container for
the UIResource built from the corresponding java resource. The java
Resource API now has a method which lets it build its native
representation that can hold the meta-data to be used for drawing it
using a cc::Layer. Currently we still build a NinePatchResource for
dynamic resources, follow up changes will fix that to add correct
native Resource subclasses.

BUG=700454
==========

[mdjones, 2017-03-13 16:47:06]

lgtm

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.cc:14: Resource::Resource(Type type) : type_(type)
{}
On 2017/03/10 19:19:24, Khushal wrote:
> On 2017/03/10 18:12:00, mdjones wrote:
> > Shouldn't a resource class know what type it is?
> 
> Is this about the UNKNOWN case? The type is stored on Resource so a user can
> safely downcast it.

I meant that each class should have it's resource type hard-coded. An argument
can be made either way though.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.h (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.h:17: enum class Type { BITMAP, NINE_PATCH_BITMAP,
UNKNOWN };
On 2017/03/10 19:19:24, Khushal wrote:
> On 2017/03/10 18:12:00, mdjones wrote:
> > What is UNKNOWN used for?
> 
> I was planning on using it for sub-classes outside on ui/. Like for the
Toolbar
> resource. But may be we can still have these types be declared in the enum
here,
> even if the sub-class exists in chrome/?

I don't think we should have chrome specific things here. I think resources
should be clearly defined; I believe there are few reasons to need to sub-class
this. If there needs to be extra logic, I think that should live in the objects
that use these resources rather than in the resources themselves.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource_manager_impl.cc (left):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource_manager_impl.cc:202: if (item ==
resources_[res_type].end()) {
On 2017/03/10 19:19:24, Khushal wrote:
> On 2017/03/10 18:12:00, mdjones wrote:
> > On 2017/03/09 23:05:37, Khushal wrote:
> > > I was worried about making repeated dynamic allocations here now. But I am
> > > assuming this was only avoiding it for the case where a dynamic resource
is
> > > updated, which should be rare?
> > 
> > Dynamic resources use the same id when they are updated which is pretty
often.
> 
> True, and I assumed these updates are quite rare. Like the toolbar will be
> invalidated and updated only when the text changes, right?
> 
> > Tab titles are a good example. However, the bulk of the allocation happens
in
> > the bitmap creation logic below which happens regardless of this.
> 

Right, we shouldn't have to worry about this.

[Khushal, 2017-03-13 21:03:25]

khushalsagar@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Khushal, 2017-03-13 21:03:25]

+tedchoc since looks like David is OOO today.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.cc (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.cc:14: Resource::Resource(Type type) : type_(type)
{}
On 2017/03/13 16:47:06, mdjones wrote:
> On 2017/03/10 19:19:24, Khushal wrote:
> > On 2017/03/10 18:12:00, mdjones wrote:
> > > Shouldn't a resource class know what type it is?
> > 
> > Is this about the UNKNOWN case? The type is stored on Resource so a user can
> > safely downcast it.
> 
> I meant that each class should have it's resource type hard-coded. An argument
> can be made either way though.

Discussed offline. We are setting the type of a resource in the sub-class.

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
File ui/android/resources/resource.h (right):

https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
ui/android/resources/resource.h:17: enum class Type { BITMAP, NINE_PATCH_BITMAP,
UNKNOWN };
On 2017/03/13 16:47:06, mdjones wrote:
> On 2017/03/10 19:19:24, Khushal wrote:
> > On 2017/03/10 18:12:00, mdjones wrote:
> > > What is UNKNOWN used for?
> > 
> > I was planning on using it for sub-classes outside on ui/. Like for the
> Toolbar
> > resource. But may be we can still have these types be declared in the enum
> here,
> > even if the sub-class exists in chrome/?
> 
> I don't think we should have chrome specific things here. I think resources
> should be clearly defined; I believe there are few reasons to need to
sub-class
> this. If there needs to be extra logic, I think that should live in the
objects
> that use these resources rather than in the resources themselves.

My plan for getting rid of padding/aperture coming from ViewResourceAdapter was
to add a Resource sub-class in chrome/ for the Toolbar and plumb the values from
there more clearly. Specifically I think what we end up needing is
tab_strip_height, the height of the toolbar_with_no_shadow, whether
shadow_is_at_bottom and the position of the location bar to build the
ToolbarLayer. We could add TOOLBAR type to the enum here but have the sub-class
live in chrome/? I was inclined on tagging this data with the resource since it
is necessary to draw it correctly using cc. Any other ideas to structure this
better?

[Ted C, 2017-03-13 23:08:33]

On 2017/03/13 21:03:25, Khushal wrote:
> +tedchoc since looks like David is OOO today.

I can look over the code for sanity things, but dtrainor@ is the best
person to review this as he wrote the initial code.  Unless there is a
pressing need to land this today, I'd prefer to defer to him.

> 
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> File ui/android/resources/resource.cc (right):
> 
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> ui/android/resources/resource.cc:14: Resource::Resource(Type type) :
type_(type)
> {}
> On 2017/03/13 16:47:06, mdjones wrote:
> > On 2017/03/10 19:19:24, Khushal wrote:
> > > On 2017/03/10 18:12:00, mdjones wrote:
> > > > Shouldn't a resource class know what type it is?
> > > 
> > > Is this about the UNKNOWN case? The type is stored on Resource so a user
can
> > > safely downcast it.
> > 
> > I meant that each class should have it's resource type hard-coded. An
argument
> > can be made either way though.
> 
> Discussed offline. We are setting the type of a resource in the sub-class.
> 
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> File ui/android/resources/resource.h (right):
> 
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> ui/android/resources/resource.h:17: enum class Type { BITMAP,
NINE_PATCH_BITMAP,
> UNKNOWN };
> On 2017/03/13 16:47:06, mdjones wrote:
> > On 2017/03/10 19:19:24, Khushal wrote:
> > > On 2017/03/10 18:12:00, mdjones wrote:
> > > > What is UNKNOWN used for?
> > > 
> > > I was planning on using it for sub-classes outside on ui/. Like for the
> > Toolbar
> > > resource. But may be we can still have these types be declared in the enum
> > here,
> > > even if the sub-class exists in chrome/?
> > 
> > I don't think we should have chrome specific things here. I think resources
> > should be clearly defined; I believe there are few reasons to need to
> sub-class
> > this. If there needs to be extra logic, I think that should live in the
> objects
> > that use these resources rather than in the resources themselves.
> 
> My plan for getting rid of padding/aperture coming from ViewResourceAdapter
was
> to add a Resource sub-class in chrome/ for the Toolbar and plumb the values
from
> there more clearly. Specifically I think what we end up needing is
> tab_strip_height, the height of the toolbar_with_no_shadow, whether
> shadow_is_at_bottom and the position of the location bar to build the
> ToolbarLayer. We could add TOOLBAR type to the enum here but have the
sub-class
> live in chrome/? I was inclined on tagging this data with the resource since
it
> is necessary to draw it correctly using cc. Any other ideas to structure this
> better?

[Khushal, 2017-03-13 23:38:30]

On 2017/03/13 23:08:33, Ted C wrote:
> On 2017/03/13 21:03:25, Khushal wrote:
> > +tedchoc since looks like David is OOO today.
> 
> I can look over the code for sanity things, but dtrainor@ is the best
> person to review this as he wrote the initial code.  Unless there is a
> pressing need to land this today, I'd prefer to defer to him.

No worries, this is nothing urgent. I can wait for dtrainor@ to take a look.

> 
> > 
> >
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> > File ui/android/resources/resource.cc (right):
> > 
> >
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> > ui/android/resources/resource.cc:14: Resource::Resource(Type type) :
> type_(type)
> > {}
> > On 2017/03/13 16:47:06, mdjones wrote:
> > > On 2017/03/10 19:19:24, Khushal wrote:
> > > > On 2017/03/10 18:12:00, mdjones wrote:
> > > > > Shouldn't a resource class know what type it is?
> > > > 
> > > > Is this about the UNKNOWN case? The type is stored on Resource so a user
> can
> > > > safely downcast it.
> > > 
> > > I meant that each class should have it's resource type hard-coded. An
> argument
> > > can be made either way though.
> > 
> > Discussed offline. We are setting the type of a resource in the sub-class.
> > 
> >
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> > File ui/android/resources/resource.h (right):
> > 
> >
>
https://codereview.chromium.org/2746483003/diff/1/ui/android/resources/resour...
> > ui/android/resources/resource.h:17: enum class Type { BITMAP,
> NINE_PATCH_BITMAP,
> > UNKNOWN };
> > On 2017/03/13 16:47:06, mdjones wrote:
> > > On 2017/03/10 19:19:24, Khushal wrote:
> > > > On 2017/03/10 18:12:00, mdjones wrote:
> > > > > What is UNKNOWN used for?
> > > > 
> > > > I was planning on using it for sub-classes outside on ui/. Like for the
> > > Toolbar
> > > > resource. But may be we can still have these types be declared in the
enum
> > > here,
> > > > even if the sub-class exists in chrome/?
> > > 
> > > I don't think we should have chrome specific things here. I think
resources
> > > should be clearly defined; I believe there are few reasons to need to
> > sub-class
> > > this. If there needs to be extra logic, I think that should live in the
> > objects
> > > that use these resources rather than in the resources themselves.
> > 
> > My plan for getting rid of padding/aperture coming from ViewResourceAdapter
> was
> > to add a Resource sub-class in chrome/ for the Toolbar and plumb the values
> from
> > there more clearly. Specifically I think what we end up needing is
> > tab_strip_height, the height of the toolbar_with_no_shadow, whether
> > shadow_is_at_bottom and the position of the location bar to build the
> > ToolbarLayer. We could add TOOLBAR type to the enum here but have the
> sub-class
> > live in chrome/? I was inclined on tagging this data with the resource since
> it
> > is necessary to draw it correctly using cc. Any other ideas to structure
this
> > better?

[David Trainor- moved to gerrit, 2017-03-14 18:31:52]

lgtm % nits!

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/LayoutResource.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/LayoutResource.java:20: public
LayoutResource(float pxToDp, Resource resource) {
Can we add a resource.buildLayoutResource(float pxToDp)?

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/ResourceManager.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/ResourceManager.java:89: public
static long createBitmapResource() {
Maybe these should live on the resources.  I admit it adds more ugly JNI bridges
though :(.  Maybe one factory class?

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File
ui/android/java/src/org/chromium/ui/resources/sprites/CrushedSpriteResource.java
(right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/sprites/CrushedSpriteResource.java:86:
return 0;
Add a TODO to figure this out.

[Khushal, 2017-03-15 01:50:06]

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/LayoutResource.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/LayoutResource.java:20: public
LayoutResource(float pxToDp, Resource resource) {
On 2017/03/14 18:31:52, David Trainor-ping if over 24h wrote:
> Can we add a resource.buildLayoutResource(float pxToDp)?

Here as well, how about a final NinePatchData getNinePatchData() method on
Resource instead?

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/ResourceManager.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/ResourceManager.java:89: public
static long createBitmapResource() {
On 2017/03/14 18:31:52, David Trainor-ping if over 24h wrote:
> Maybe these should live on the resources.  I admit it adds more ugly JNI
bridges
> though :(.  Maybe one factory class?

Actually, how about just making NinePatchData public? You only have one public
method then, createBitmapResource(NinePatchData) and if you pass a NinePatchData
then it will create the NinePatchResource.

In this patch I'll have to keep the createNinePatchBitmap public, for the
DynamicResource, but going forward we can remove that when DynamicResources
build their own Resource.

And if its just one public helper method, might as well just keep it on this
class?

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File
ui/android/java/src/org/chromium/ui/resources/sprites/CrushedSpriteResource.java
(right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/sprites/CrushedSpriteResource.java:86:
return 0;
On 2017/03/14 18:31:52, David Trainor-ping if over 24h wrote:
> Add a TODO to figure this out.

Done.

[Khushal, 2017-03-16 19:01:44]

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/LayoutResource.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/LayoutResource.java:20: public
LayoutResource(float pxToDp, Resource resource) {
On 2017/03/15 01:50:06, Khushal wrote:
> On 2017/03/14 18:31:52, David Trainor-ping if over 24h wrote:
> > Can we add a resource.buildLayoutResource(float pxToDp)?
> 
> Here as well, how about a final NinePatchData getNinePatchData() method on
> Resource instead?

Done.

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/resources/ResourceManager.java (right):

https://codereview.chromium.org/2746483003/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/resources/ResourceManager.java:89: public
static long createBitmapResource() {
On 2017/03/15 01:50:06, Khushal wrote:
> On 2017/03/14 18:31:52, David Trainor-ping if over 24h wrote:
> > Maybe these should live on the resources.  I admit it adds more ugly JNI
> bridges
> > though :(.  Maybe one factory class?
> 
> Actually, how about just making NinePatchData public? You only have one public
> method then, createBitmapResource(NinePatchData) and if you pass a
NinePatchData
> then it will create the NinePatchResource.
> 
> In this patch I'll have to keep the createNinePatchBitmap public, for the
> DynamicResource, but going forward we can remove that when DynamicResources
> build their own Resource.
> 
> And if its just one public helper method, might as well just keep it on this
> class?

I moved it to a separate class. Cleaner.

[Khushal, 2017-03-16 19:01:48]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-16 19:01:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
mdjones@chromium.org, dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2746483003/#ps80001
(title: "..")

[Khushal, 2017-03-16 19:02:03]

The CQ bit was unchecked by khushalsagar@chromium.org

[Khushal, 2017-03-16 19:03:14]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-16 19:03:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
mdjones@chromium.org, dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2746483003/#ps100001
(title: "actually upload files")

[commit-bot: I haz the power, 2017-03-16 19:04:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 19:33:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 19:33:27]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Khushal, 2017-03-16 20:11:19]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-16 20:11:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
mdjones@chromium.org, dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2746483003/#ps120001
(title: "test")

[commit-bot: I haz the power, 2017-03-16 20:12:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 21:52:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 21:52:38]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Khushal, 2017-03-16 22:08:10]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-03-16 22:08:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 23:45:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 23:45:08]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Khushal, 2017-03-17 00:46:00]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-17 00:46:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
mdjones@chromium.org, dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2746483003/#ps140001
(title: "jni")

[commit-bot: I haz the power, 2017-03-17 00:46:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 01:57:49]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489711560878050,
"parent_rev": "0f0c1893bd9b05b51a6e5df58495137fb1c1938b", "commit_rev":
"39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e"}

[commit-bot: I haz the power, 2017-03-17 01:58:56]

Description was changed from

==========
ui/android: Fix Resource meta-data sharing with ResourceManager.

The android native resource management system provides a
ui::ResourceManager::Resource, which pulls data from the java Resource,
to provide meta-data to the code building cc::Layers for drawing this
resource.

The resource exposes padding and aperture, which are well defined
concepts for a nine-patch bitmap, but are arbitrarily set in all other
cases. In static bitmaps they are set to the bitmap size, while for
dynamic resources they could represent anything.

The change pulls out a native Resource class which is a container for
the UIResource built from the corresponding java resource. The java
Resource API now has a method which lets it build its native
representation that can hold the meta-data to be used for drawing it
using a cc::Layer. Currently we still build a NinePatchResource for
dynamic resources, follow up changes will fix that to add correct
native Resource subclasses.

BUG=700454
==========

to

==========
ui/android: Fix Resource meta-data sharing with ResourceManager.

The android native resource management system provides a
ui::ResourceManager::Resource, which pulls data from the java Resource,
to provide meta-data to the code building cc::Layers for drawing this
resource.

The resource exposes padding and aperture, which are well defined
concepts for a nine-patch bitmap, but are arbitrarily set in all other
cases. In static bitmaps they are set to the bitmap size, while for
dynamic resources they could represent anything.

The change pulls out a native Resource class which is a container for
the UIResource built from the corresponding java resource. The java
Resource API now has a method which lets it build its native
representation that can hold the meta-data to be used for drawing it
using a cc::Layer. Currently we still build a NinePatchResource for
dynamic resources, follow up changes will fix that to add correct
native Resource subclasses.

BUG=700454

Review-Url: https://codereview.chromium.org/2746483003
Cr-Commit-Position: refs/heads/master@{#457655}
Committed:
https://chromium.googlesource.com/chromium/src/+/39c53dc5b763b6ac8e60c78ea32b...
==========

[commit-bot: I haz the power, 2017-03-17 01:58:57]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/39c53dc5b763b6ac8e60c78ea32b...