chrome/android: Update toolbar drawing in native.

chrome/android: Update toolbar drawing in native.

Add a ToolbarResource to push relevant info for drawing the toolbar to
native. This replaces the previous use of NinePatchResource for this.
And some cleanup in ToolbarLayer.

BUG=700454
Review-Url: https://codereview.chromium.org/2752693003
Cr-Commit-Position: refs/heads/master@{#458831}
Committed: https://chromium.googlesource.com/chromium/src/+/51b27db46a139e5b7ae668cf22b23f5f434d05f7

[Khushal, 2017-03-14 23:59:51]

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

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

On 2017/03/14 23:59:51, Khushal wrote:

pingy.

[mdjones, 2017-03-16 21:01:48]

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java:16:
public static long createToolbarContainerResource(
Are we going to do something similar for the crushed sprite stuff?

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java:169:
private int mToolbarHeightPx;
This can be pulled directly from toolbar now.

https://codereview.chromium.org/2752693003/diff/40001/chrome/browser/android/...
File chrome/browser/android/compositor/resources/toolbar_resource.h (right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/browser/android/...
chrome/browser/android/compositor/resources/toolbar_resource.h:12: class
ToolbarResource : public ui::Resource {
Make this a final class.

https://codereview.chromium.org/2752693003/diff/40001/ui/android/resources/ni...
File ui/android/resources/nine_patch_resource.cc (right):

https://codereview.chromium.org/2752693003/diff/40001/ui/android/resources/ni...
ui/android/resources/nine_patch_resource.cc:26: void
NinePatchResource::UpdateNinePatchLayer(
This feels like something that should either be static or on the NinePatchLayer
itself. It looks like you would just need to add a getter for padding on this
class.

[David Trainor- moved to gerrit, 2017-03-17 07:41:22]

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java:16:
public static long createToolbarContainerResource(
On 2017/03/16 21:01:47, mdjones wrote:
> Are we going to do something similar for the crushed sprite stuff?

I think that (crushed sprite) could go in ui/ since it doesn't require something
chrome/ specific?

[mdjones, 2017-03-17 15:38:10]

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java:16:
public static long createToolbarContainerResource(
On 2017/03/17 07:41:22, David Trainor-ping if over 24h wrote:
> On 2017/03/16 21:01:47, mdjones wrote:
> > Are we going to do something similar for the crushed sprite stuff?
> 
> I think that (crushed sprite) could go in ui/ since it doesn't require
something
> chrome/ specific?

That makes sense, I was thinking about it as part of contextual search, but
crushed sprites in general aren't chrome specific.

[Khushal, 2017-03-17 23:02:58]

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java:16:
public static long createToolbarContainerResource(
On 2017/03/17 15:38:10, mdjones wrote:
> On 2017/03/17 07:41:22, David Trainor-ping if over 24h wrote:
> > On 2017/03/16 21:01:47, mdjones wrote:
> > > Are we going to do something similar for the crushed sprite stuff?
> > 
> > I think that (crushed sprite) could go in ui/ since it doesn't require
> something
> > chrome/ specific?
> 
> That makes sense, I was thinking about it as part of contextual search, but
> crushed sprites in general aren't chrome specific.

Yeah. CrushedSprite in next patch. That should be all in ui/

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java:169:
private int mToolbarHeightPx;
On 2017/03/16 21:01:47, mdjones wrote:
> This can be pulled directly from toolbar now.

Done.

https://codereview.chromium.org/2752693003/diff/40001/chrome/browser/android/...
File chrome/browser/android/compositor/resources/toolbar_resource.h (right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/browser/android/...
chrome/browser/android/compositor/resources/toolbar_resource.h:12: class
ToolbarResource : public ui::Resource {
On 2017/03/16 21:01:48, mdjones wrote:
> Make this a final class.

Done.

https://codereview.chromium.org/2752693003/diff/40001/ui/android/resources/ni...
File ui/android/resources/nine_patch_resource.cc (right):

https://codereview.chromium.org/2752693003/diff/40001/ui/android/resources/ni...
ui/android/resources/nine_patch_resource.cc:26: void
NinePatchResource::UpdateNinePatchLayer(
On 2017/03/16 21:01:48, mdjones wrote:
> This feels like something that should either be static or on the
NinePatchLayer
> itself. It looks like you would just need to add a getter for padding on this
> class.

The getter is already public. This was supposed to be more of a helper method
for not redoing the math for size/position all over the place. I think I'll just
add 2 methods for getting the draw size and position instead, to mirror Border.
The rest of it can stay where it was.

[Khushal, 2017-03-17 23:49:45]

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
(right):

https://codereview.chromium.org/2752693003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java:16:
public static long createToolbarContainerResource(
On 2017/03/17 23:02:58, Khushal wrote:
> On 2017/03/17 15:38:10, mdjones wrote:
> > On 2017/03/17 07:41:22, David Trainor-ping if over 24h wrote:
> > > On 2017/03/16 21:01:47, mdjones wrote:
> > > > Are we going to do something similar for the crushed sprite stuff?
> > > 
> > > I think that (crushed sprite) could go in ui/ since it doesn't require
> > something
> > > chrome/ specific?
> > 
> > That makes sense, I was thinking about it as part of contextual search, but
> > crushed sprites in general aren't chrome specific.
> 
> Yeah. CrushedSprite in next patch. That should be all in ui/

I looked at CrushedSprite and they are funkier than I thought. With the regular
Resource we register the bitmap with the cc::UIResourceManager since that is the
bitmap we want to draw with, and we only expose the UIResource. We don't want to
expose the raw bitmap and have callers use it directly, since that would mean
multiple UIResources could be generated for the same bitmap.

The CrushedSpriteResource exposes the raw bitmap because the bitmap to use is
generated for each frame using this bitmap, and then it is set on the layer
directly. I don't think we want to have a base interface/class that exposes the
bitmap just for this use-case. The Crushed sprite stuff is already separate from
the rest of resource management. Its probably easier to let it stay that way.
+dtrainor@, thoughts?

[mdjones, 2017-03-17 23:50:29]

lgtm

[David Trainor- moved to gerrit, 2017-03-20 15:30:37]

lgtm

[Khushal, 2017-03-20 19:00:35]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-20 19:00:35]

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/2752693003/#ps100001
(title: "make API private")

[commit-bot: I haz the power, 2017-03-20 19:01:45]

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-20 19:19:56]

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

[commit-bot: I haz the power, 2017-03-20 19:19:57]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Khushal, 2017-03-20 19:46:28]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-03-20 19:47:16]

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-20 23:14:00]

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

[commit-bot: I haz the power, 2017-03-20 23:14:01]

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-21 02:05:09]

The CQ bit was checked by khushalsagar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 02:05:44]

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

[Khushal, 2017-03-21 02:08:57]

The CQ bit was checked by khushalsagar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 02:09:34]

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

[Khushal, 2017-03-21 02:09:38]

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

[Khushal, 2017-03-21 02:09:39]

+dfalcantara, do you want to take a glance at webapps. The change should be a
no-op.

[commit-bot: I haz the power, 2017-03-21 03:35:24]

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

[commit-bot: I haz the power, 2017-03-21 03:35:25]

Dry run: This issue passed the CQ dry run.

[gone, 2017-03-21 18:02:54]

dfalcantara@chromium.org changed reviewers:
+ yusufo@chromium.org

[gone, 2017-03-21 18:02:55]

Yusuf has worked on the control container stuff more than I have.  Redirecting.

[Yusuf, 2017-03-22 17:25:03]

lgtm

[Khushal, 2017-03-22 17:35:50]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-03-22 17:35:51]

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/2752693003/#ps140001
(title: "..")

[commit-bot: I haz the power, 2017-03-22 17:36:18]

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-22 19:18:16]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1490204150674380,
"parent_rev": "7371329ead225cd39a356999abcb12430556944e", "commit_rev":
"51b27db46a139e5b7ae668cf22b23f5f434d05f7"}

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

Description was changed from

==========
chrome/android: Update toolbar drawing in native.

Add a ToolbarResource to push relevant info for drawing the toolbar to
native. This replaces the previous use of NinePatchResource for this.
And some cleanup in ToolbarLayer.

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

to

==========
chrome/android: Update toolbar drawing in native.

Add a ToolbarResource to push relevant info for drawing the toolbar to
native. This replaces the previous use of NinePatchResource for this.
And some cleanup in ToolbarLayer.

BUG=700454

Review-Url: https://codereview.chromium.org/2752693003
Cr-Commit-Position: refs/heads/master@{#458831}
Committed:
https://chromium.googlesource.com/chromium/src/+/51b27db46a139e5b7ae668cf22b2...
==========

[commit-bot: I haz the power, 2017-03-22 19:19:05]

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