New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
Fix resource meta-data sharing in native android code.
Project Member Reported by khushals...@chromium.org, Mar 10 2017 Back to list
The primary purpose of this change is to remove padding and aperture from base Resource, both java and native code. These make sense for a nine-patch bitmap but end up having arbitray values everywhere else. And are used in an ad-hoc way to send meta-data from java to native to build cc::Layers for drawing UI elements backed by the resource.

On the java side this will remove getPadding and getAperture from the interface. On the native side we'll have a Resource base class, which is a container for a UIResource. Subclasses can inherit from this to attach any additional meta-data, NinePatchResource being one of them. We can have an enum for each sub-class to safely downcast since the ResourceManager will still return the base Resource.
 
Labels: -Pri-3 Pri-2
Project Member Comment 2 by bugdroid1@chromium.org, Mar 17 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e

commit 39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e
Author: khushalsagar <khushalsagar@chromium.org>
Date: Fri Mar 17 01:57:45 2017

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}

[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/decoration_title.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/contextual_search_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/overlay_panel_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/tab_handle_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/tab_handle_layer.h
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/tab_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/layer/toolbar_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/chrome/browser/android/compositor/scene_layer/tab_strip_scene_layer.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/BUILD.gn
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/LayoutResource.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/Resource.java
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/ResourceFactory.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/ResourceManager.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/dynamics/BitmapDynamicResource.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/dynamics/ViewResourceAdapter.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/sprites/CrushedSpriteResource.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/statics/NinePatchData.java
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/java/src/org/chromium/ui/resources/statics/StaticResource.java
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/nine_patch_resource.cc
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/nine_patch_resource.h
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource.cc
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource.h
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_factory.cc
[add] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_factory.h
[delete] https://crrev.com/0f0c1893bd9b05b51a6e5df58495137fb1c1938b/ui/android/resources/resource_manager.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_manager.h
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_manager_impl.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_manager_impl.h
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/resources/resource_manager_impl_unittest.cc
[modify] https://crrev.com/39c53dc5b763b6ac8e60c78ea32bb4e7ce7f009e/ui/android/ui_android_jni_registrar.cc

Cc: twelling...@chromium.org
I was looking at having CrushedSpriteResource inherit from Resource as well to keep things consistent. But from what I could see on a first pass, CrushedSprite resources are fundamentally different from Resource in the following way.

With the regular Resource we register the bitmap with the cc::UIResourceManager since that is the bitmap we want the compositor 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 creates a possibility of multiple UIResources generated for the same bitmap.

The CrushedSpriteResource exposes the raw bitmap in native because the bitmap to use is generated for each frame using this bitmap, and then it is set on the layer directly to let cc upload it as a UIResource for that frame. I don't think we want to have the Resource class expose the bitmap just for this use-case. Unless someone else feels strongly about it, I'll let the CrushedSpriteResource be as it is.
Project Member Comment 4 by bugdroid1@chromium.org, Mar 22 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51b27db46a139e5b7ae668cf22b23f5f434d05f7

commit 51b27db46a139e5b7ae668cf22b23f5f434d05f7
Author: khushalsagar <khushalsagar@chromium.org>
Date: Wed Mar 22 19:18:12 2017

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}

[add] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/android/java/src/org/chromium/chrome/browser/compositor/resources/ResourceFactory.java
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappControlContainer.java
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/android/java_sources.gni
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/BUILD.gn
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/compositor/layer/toolbar_layer.cc
[add] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/compositor/resources/resource_factory.cc
[add] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/compositor/resources/resource_factory.h
[add] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/compositor/resources/toolbar_resource.cc
[add] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/chrome/browser/android/compositor/resources/toolbar_resource.h
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/ui/android/java/src/org/chromium/ui/resources/ResourceFactory.java
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/ui/android/java/src/org/chromium/ui/resources/dynamics/ViewResourceAdapter.java
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/ui/android/resources/nine_patch_resource.cc
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/ui/android/resources/nine_patch_resource.h
[modify] https://crrev.com/51b27db46a139e5b7ae668cf22b23f5f434d05f7/ui/android/resources/resource.h

Status: Fixed
Sign in to add a comment