Replace SystemUIResourceManager with ResourceManager

Replace SystemUIResourceManager with ResourceManager

And move UIResourceClientAndroid and UIResourceProvider under ui/android.

BUG=426939
Committed: https://crrev.com/f229b81b8d31706d3aa922294d1e9c8f567bd400
Cr-Commit-Position: refs/heads/master@{#307844}

[Jaekyun Seok (inactive), 2014-12-08 07:29:30]

jaekyun@chromium.org changed reviewers:
+ dtrainor@chromium.org, jdduke@chromium.org

[Jaekyun Seok (inactive), 2014-12-08 07:29:31]

Jared and David, could you please review this CL which replaces
SystemUIResourceManagerImpl with ResourceManager?

Thanks,

[jdduke (slow), 2014-12-08 16:23:13]

https://codereview.chromium.org/755643004/diff/1/content/browser/android/cont...
File content/browser/android/content_view_render_view.cc (right):

https://codereview.chromium.org/755643004/diff/1/content/browser/android/cont...
content/browser/android/content_view_render_view.cc:20: #include
"ui/android/resources/ui_resource_provider.h"
Do we even need this include?

https://codereview.chromium.org/755643004/diff/1/content/browser/android/syst...
File content/browser/android/system_ui_resource_manager_impl_unittest.cc (left):

https://codereview.chromium.org/755643004/diff/1/content/browser/android/syst...
content/browser/android/system_ui_resource_manager_impl_unittest.cc:140:
TEST_F(SystemUIResourceManagerImplTest, PreloadEnsureResource) {
Do we have any test coverage of the new UIResourceManager?

https://codereview.chromium.org/755643004/diff/1/ui/android/java/src/org/chro...
File
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java
(right):

https://codereview.chromium.org/755643004/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java:57:
return StaticResource.create(Resources.getSystem(), R.drawable.refresh_gray, 0,
0);
Should we use "context.getResources()" instead of "Resources.getSystem()"?
Technically the refresh resources aren't even system resources, so at some point
we might consider making them a difference type of resource.

https://codereview.chromium.org/755643004/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java:89:
Bitmap bitmap = Bitmap.createBitmap((int) arcWidth, (int) height,
Bitmap.Config.ARGB_8888);
dtrainor@: We used A8 for the original SkBitmap format, this will use 4x as much
memory which I don't think we want... We might need to add support for that
format to the JavaBitmap code.

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.cc (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.cc:77: if (!resource ||
!resource->ui_resource)
The result of GetResource should never be null, I think.

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.h:56: void
PreloadResource(ui::SystemUIResourceType type) override;
Can we just get rid of the SystemUIResourceManager interface entirely?

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.h:63: ResourceManager::Resource*
GetResource(AndroidResourceType res_type,
Where are these methods that take a string used? I would *much* prefer we don't
take a string argument anywhere here, and certainly not a string *by value* (see
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/EUqoIz2iFU4...).

If necessary, we could expose the |int
GetAndroidResourceIdFromString(std::string res_name) method|, so the caller can
cache the id just once. But I still would like to know where these methods are
called.

[Jaekyun Seok (inactive), 2014-12-09 11:42:36]

PTAL.

https://codereview.chromium.org/755643004/diff/1/content/browser/android/cont...
File content/browser/android/content_view_render_view.cc (right):

https://codereview.chromium.org/755643004/diff/1/content/browser/android/cont...
content/browser/android/content_view_render_view.cc:20: #include
"ui/android/resources/ui_resource_provider.h"
On 2014/12/08 16:23:13, jdduke wrote:
> Do we even need this include?

Done.

https://codereview.chromium.org/755643004/diff/1/content/browser/android/syst...
File content/browser/android/system_ui_resource_manager_impl_unittest.cc (left):

https://codereview.chromium.org/755643004/diff/1/content/browser/android/syst...
content/browser/android/system_ui_resource_manager_impl_unittest.cc:140:
TEST_F(SystemUIResourceManagerImplTest, PreloadEnsureResource) {
On 2014/12/08 16:23:13, jdduke wrote:
> Do we have any test coverage of the new UIResourceManager?

Moved these tests to resource_manager_unittest.cc

https://codereview.chromium.org/755643004/diff/1/ui/android/java/src/org/chro...
File
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java
(right):

https://codereview.chromium.org/755643004/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java:57:
return StaticResource.create(Resources.getSystem(), R.drawable.refresh_gray, 0,
0);
On 2014/12/08 16:23:13, jdduke wrote:
> Should we use "context.getResources()" instead of "Resources.getSystem()"?
> Technically the refresh resources aren't even system resources, so at some
point
> we might consider making them a difference type of resource.

Done.

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.cc (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.cc:77: if (!resource ||
!resource->ui_resource)
On 2014/12/08 16:23:13, jdduke wrote:
> The result of GetResource should never be null, I think.

Yes, the result of GetResource can't be null in a normal case. But
TestResourceManager in resource_manager_unittest.cc overrides
RequestResourceFromJava, and in that case GetResource can returns null.  So we
need to handle the case that the result of GetResource is null.

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.h:56: void
PreloadResource(ui::SystemUIResourceType type) override;
On 2014/12/08 16:23:13, jdduke wrote:
> Can we just get rid of the SystemUIResourceManager interface entirely?

I want to keep the SystemUIResourceManager interface because it is being exposed
in ui/base/android/window_android_compositor.h .

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.h:63: ResourceManager::Resource*
GetResource(AndroidResourceType res_type,
On 2014/12/08 16:23:13, jdduke wrote:
> Where are these methods that take a string used? I would *much* prefer we
don't
> take a string argument anywhere here, and certainly not a string *by value*
(see
>
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/EUqoIz2iFU4...).
> 
> If necessary, we could expose the |int
> GetAndroidResourceIdFromString(std::string res_name) method|, so the caller
can
> cache the id just once. But I still would like to know where these methods are
> called.

This was requested from David.

David, could you please answer to these comments?

[jdduke (slow), 2014-12-09 16:11:07]

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.cc (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.cc:77: if (!resource ||
!resource->ui_resource)
On 2014/12/09 11:42:35, Jaekyun Seok wrote:
> On 2014/12/08 16:23:13, jdduke wrote:
> > The result of GetResource should never be null, I think.
> 
> Yes, the result of GetResource can't be null in a normal case. But
> TestResourceManager in resource_manager_unittest.cc overrides
> RequestResourceFromJava, and in that case GetResource can returns null.  So we
> need to handle the case that the result of GetResource is null.

Hmm, can we avoid that? It's a shame to have to modify implementation
assumptions just to accommodate a test. Could we make it so the test allows this
to never return null?

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
content/browser/renderer_host/compositor_impl_android.cc:330:
ui::SystemUIResourceManager& CompositorImpl::GetSystemUIResourceManager() {
OK, but eventually I'd like this interface to go away entirely, as it's just a
convenient wrapper now. Maybe add a TODO to remove it in
system_ui_resource_manager.h?

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
content/browser/renderer_host/compositor_impl_android.h:100: virtual
ui::SystemUIResourceManager& GetSystemUIResourceManager() override;
It's a little unclear why GetSystemUIResourceManager hangs off the
WindowAndroidCompositor interface, but GetResourceManager() hangs off the
Compositor interface? When we remove SystemUIResourceManager can we have
GetResourceManager() hang off the WindowAndroidCompositor interface?

[Jaekyun Seok (inactive), 2014-12-09 23:35:22]

PTAL.

I removed SystemUIResourceManager, and modified unittest not to return null from
GetResource.

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
File ui/android/resources/resource_manager.cc (right):

https://codereview.chromium.org/755643004/diff/1/ui/android/resources/resourc...
ui/android/resources/resource_manager.cc:77: if (!resource ||
!resource->ui_resource)
On 2014/12/09 16:11:07, jdduke wrote:
> On 2014/12/09 11:42:35, Jaekyun Seok wrote:
> > On 2014/12/08 16:23:13, jdduke wrote:
> > > The result of GetResource should never be null, I think.
> > 
> > Yes, the result of GetResource can't be null in a normal case. But
> > TestResourceManager in resource_manager_unittest.cc overrides
> > RequestResourceFromJava, and in that case GetResource can returns null.  So
we
> > need to handle the case that the result of GetResource is null.
> 
> Hmm, can we avoid that? It's a shame to have to modify implementation
> assumptions just to accommodate a test. Could we make it so the test allows
this
> to never return null?

Done.

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/755643004/diff/20001/content/browser/renderer...
content/browser/renderer_host/compositor_impl_android.h:100: virtual
ui::SystemUIResourceManager& GetSystemUIResourceManager() override;
On 2014/12/09 16:11:07, jdduke wrote:
> It's a little unclear why GetSystemUIResourceManager hangs off the
> WindowAndroidCompositor interface, but GetResourceManager() hangs off the
> Compositor interface? When we remove SystemUIResourceManager can we have
> GetResourceManager() hang off the WindowAndroidCompositor interface? 

I will remove SystemUIResourceManager and add GetResourceManager() to
WindowAndroidCompositor interface.

[jdduke (slow), 2014-12-10 01:46:41]

Thanks, this is shaping up to be a really nice cleanup.

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
ui/android/resources/resource_manager.h:57: virtual void
PreloadResource(ui::SystemUIResourceType type);
Let's go ahead and remove these two methods, updating the callers to use the new
Preload/Get API.

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
ui/android/resources/resource_manager.h:62: ResourceManager::Resource*
GetResource(AndroidResourceType res_type,
I'm wary of adding these methods that take a string without having any existing
users. Could we add them later? 

In any case, please avoid passing std::string by value. Either pass by const
ref, or pass a const char* (or a StringPiece).

[Jaekyun Seok (inactive), 2014-12-10 02:25:41]

PTAL.

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
ui/android/resources/resource_manager.h:57: virtual void
PreloadResource(ui::SystemUIResourceType type);
On 2014/12/10 01:46:41, jdduke wrote:
> Let's go ahead and remove these two methods, updating the callers to use the
new
> Preload/Get API.

I believe that GetUIResourceId is useful and so I will keep it after changing it
to get res_type and res_id as parameters.

https://codereview.chromium.org/755643004/diff/40001/ui/android/resources/res...
ui/android/resources/resource_manager.h:62: ResourceManager::Resource*
GetResource(AndroidResourceType res_type,
On 2014/12/10 01:46:41, jdduke wrote:
> I'm wary of adding these methods that take a string without having any
existing
> users. Could we add them later? 
> 

I agree. Let's do this later when it is needed.

> In any case, please avoid passing std::string by value. Either pass by const
> ref, or pass a const char* (or a StringPiece).

[Jaekyun Seok (inactive), 2014-12-10 02:28:32]

I removed the log to grab resources by string.

[Jaekyun Seok (inactive), 2014-12-10 02:28:57]

On 2014/12/10 02:28:32, Jaekyun Seok wrote:
> I removed the log to grab resources by string.

Sorry for typos. I removed the logic to grab resources by string.

[jdduke (slow), 2014-12-10 19:19:27]

I'm not an owner for most of this, but in general this lgtm. I'll let dtrainor@
give it another run through. Thanks again!

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
File content/browser/android/edge_effect_l.cc (right):

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
content/browser/android/edge_effect_l.cc:238: ui::ANDROID_RESOURCE_TYPE_SYSTEM,
kResourceType));
Hmm, the |kResourceType| name is a little confusing now, maybe change it to
|kResourceId|?

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
File content/browser/android/overscroll_refresh.cc (right):

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
content/browser/android/overscroll_refresh.cc:329:
ui::ANDROID_RESOURCE_TYPE_SYSTEM, kIdleResourceType);
Same here, |kIdleResourceId| and |kActiveResourceId|.

https://codereview.chromium.org/755643004/diff/60001/ui/android/java/src/org/...
File
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java
(right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java:56:
case SystemUIResourceType.OVERSCROLL_REFRESH_IDLE:
Thanks. At some point we could probably shift these to be a different resource
type (they're not actually system resources), so maybe add a "TODO(jdduke): Make
refresh resources be of type ANDROID_RESOURCE_TYPE_STATIC".

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/res...
ui/android/resources/resource_manager.h:57: virtual cc::UIResourceId
GetUIResourceId(AndroidResourceType res_type,
Hmm, I don't think any of these public methods should be virtual.

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/ui_...
File ui/android/resources/ui_resource_provider.cc (right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/ui_...
ui/android/resources/ui_resource_provider.cc:29: UIResourceClientMap client_map
= ui_resource_client_map_;
Nit: This should really be (sorry, I know this wasn't your change):

UIResourceClientMap client_map;
client_map.swap(ui_resource_client_map_);

[Jaekyun Seok (inactive), 2014-12-10 21:42:38]

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
File content/browser/android/edge_effect_l.cc (right):

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
content/browser/android/edge_effect_l.cc:238: ui::ANDROID_RESOURCE_TYPE_SYSTEM,
kResourceType));
On 2014/12/10 19:19:27, jdduke wrote:
> Hmm, the |kResourceType| name is a little confusing now, maybe change it to
> |kResourceId|?

Done.

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
File content/browser/android/overscroll_refresh.cc (right):

https://codereview.chromium.org/755643004/diff/60001/content/browser/android/...
content/browser/android/overscroll_refresh.cc:329:
ui::ANDROID_RESOURCE_TYPE_SYSTEM, kIdleResourceType);
On 2014/12/10 19:19:27, jdduke wrote:
> Same here, |kIdleResourceId| and |kActiveResourceId|.

Done.

https://codereview.chromium.org/755643004/diff/60001/ui/android/java/src/org/...
File
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java
(right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/resources/system/SystemResourceLoader.java:56:
case SystemUIResourceType.OVERSCROLL_REFRESH_IDLE:
On 2014/12/10 19:19:27, jdduke wrote:
> Thanks. At some point we could probably shift these to be a different resource
> type (they're not actually system resources), so maybe add a "TODO(jdduke):
Make
> refresh resources be of type ANDROID_RESOURCE_TYPE_STATIC".

Done.

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/res...
ui/android/resources/resource_manager.h:57: virtual cc::UIResourceId
GetUIResourceId(AndroidResourceType res_type,
On 2014/12/10 19:19:27, jdduke wrote:
> Hmm, I don't think any of these public methods should be virtual.

I will remove virtual from GetResource. But virtual is needed for
GetUIResourceId and PreloadResource because OverscrollRefreshTest overrides
them.

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/ui_...
File ui/android/resources/ui_resource_provider.cc (right):

https://codereview.chromium.org/755643004/diff/60001/ui/android/resources/ui_...
ui/android/resources/ui_resource_provider.cc:29: UIResourceClientMap client_map
= ui_resource_client_map_;
On 2014/12/10 19:19:27, jdduke wrote:
> Nit: This should really be (sorry, I know this wasn't your change):
> 
> UIResourceClientMap client_map;
> client_map.swap(ui_resource_client_map_);

Done.

[Jaekyun Seok (inactive), 2014-12-10 22:01:05]

jaekyun@chromium.org changed reviewers:
+ avi@chromium.org, cjhopman@chromium.org, sievers@chromium.org

[Jaekyun Seok (inactive), 2014-12-10 22:01:06]

sievers@chromium.org: Please review changes in content/.

avi@chromium.org: Please review changes in content/ and ui/.

cjhopman@chromium.org: Please review changes in build/.

Thanks,

[jdduke (slow), 2014-12-10 22:08:08]

https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
File build/android/pylib/gtest/gtest_config.py (right):

https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
build/android/pylib/gtest/gtest_config.py:39: 'ui_base_unittests',
I believe there might also be a recipe that needs to change to get these tests
running on certain builders, but I'm not exactly sure...

[Avi (use Gerrit), 2014-12-10 22:13:07]

lgtm

stampity stamp

[cjhopman, 2014-12-10 22:13:38]

build/ lgtm

[jdduke (slow), 2014-12-10 22:19:47]

https://codereview.chromium.org/755643004/diff/80001/content/browser/android/...
File content/browser/android/overscroll_refresh_unittest.cc (right):

https://codereview.chromium.org/755643004/diff/80001/content/browser/android/...
content/browser/android/overscroll_refresh_unittest.cc:33: // ResoruceManager
implementation.
I missed this change, hmm... 

Either we should make ui::ResourceManager a pure virtual interface, with a
ui::ResourceManagerImpl implementation for production, or we should have just
one MockResourceManager for testing that only overrides those two private
virtual methods. Having both public and private virtual methods on
ResourceManager makes it difficult to reason about.

[jdduke (slow), 2014-12-10 22:33:26]

https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
ui/android/resources/resource_manager.h:35: class UI_ANDROID_EXPORT
ResourceManager {
Per discussion offline, this change is already quite large so I'll make the
interface refactoring in a follow-up patch. In the meantime, could you add:

"TODO(jdduke): Make ResourceManager a pure interface, crbug/426939."

here?

[Jaekyun Seok (inactive), 2014-12-10 22:49:12]

https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
File build/android/pylib/gtest/gtest_config.py (right):

https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
build/android/pylib/gtest/gtest_config.py:39: 'ui_base_unittests',
On 2014/12/10 22:08:08, jdduke wrote:
> I believe there might also be a recipe that needs to change to get these tests
> running on certain builders, but I'm not exactly sure...

I added 'ui_android_unittests' to all the places where
'android_webview_unittests' exists. So I believe that 'ui_android_unittests'
will be run on android trybots.

cjhopman@, could you please confirm that this is a right way to run
'ui_android_unittests' on android trybots?

https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
File ui/android/resources/resource_manager.h (right):

https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
ui/android/resources/resource_manager.h:35: class UI_ANDROID_EXPORT
ResourceManager {
On 2014/12/10 22:33:26, jdduke wrote:
> Per discussion offline, this change is already quite large so I'll make the
> interface refactoring in a follow-up patch. In the meantime, could you add:
> 
> "TODO(jdduke): Make ResourceManager a pure interface, crbug/426939."
> 
> here?

Done.

[cjhopman, 2014-12-10 22:56:23]

On 2014/12/10 22:49:12, Jaekyun Seok wrote:
>
https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
> File build/android/pylib/gtest/gtest_config.py (right):
> 
>
https://codereview.chromium.org/755643004/diff/80001/build/android/pylib/gtes...
> build/android/pylib/gtest/gtest_config.py:39: 'ui_base_unittests',
> On 2014/12/10 22:08:08, jdduke wrote:
> > I believe there might also be a recipe that needs to change to get these
tests
> > running on certain builders, but I'm not exactly sure...
> 
> I added 'ui_android_unittests' to all the places where
> 'android_webview_unittests' exists. So I believe that 'ui_android_unittests'
> will be run on android trybots.
> 
> cjhopman@, could you please confirm that this is a right way to run
> 'ui_android_unittests' on android trybots?

I expect you will still need to edit a recipe to actually have the bots run the
tests. The recipes are in a different repo, though. It might be best to file a
ticket against infra (if you want it done quickly you may be able to get some
direction in how to edit the recipe from them).


> 
>
https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
> File ui/android/resources/resource_manager.h (right):
> 
>
https://codereview.chromium.org/755643004/diff/80001/ui/android/resources/res...
> ui/android/resources/resource_manager.h:35: class UI_ANDROID_EXPORT
> ResourceManager {
> On 2014/12/10 22:33:26, jdduke wrote:
> > Per discussion offline, this change is already quite large so I'll make the
> > interface refactoring in a follow-up patch. In the meantime, could you add:
> > 
> > "TODO(jdduke): Make ResourceManager a pure interface, crbug/426939."
> > 
> > here?
> 
> Done.

[Jaekyun Seok (inactive), 2014-12-10 22:57:44]

jaekyun@chromium.org changed reviewers:
+ danakj@chromium.org

[Jaekyun Seok (inactive), 2014-12-10 22:57:45]

Dana, could you please review ui/android/DEPS as an owner of cc/?

[David Trainor- moved to gerrit, 2014-12-10 23:03:59]

I'm good with the overall structure.  As long as jdduke@ is okay with it as well
this lgtm.

[danakj, 2014-12-10 23:06:32]

On 2014/12/10 22:57:45, Jaekyun Seok wrote:
> Dana, could you please review ui/android/DEPS as an owner of cc/?

Can you explain why this moved to ui/ ?

[Jaekyun Seok (inactive), 2014-12-10 23:15:53]

On 2014/12/10 23:06:32, danakj wrote:
> On 2014/12/10 22:57:45, Jaekyun Seok wrote:
> > Dana, could you please review ui/android/DEPS as an owner of cc/?
> 
> Can you explain why this moved to ui/ ?

UIResourceProvider is moved to here because it is only related to android
resources and so ui/android/resources is a properer place for it.

[Jaekyun Seok (inactive), 2014-12-10 23:35:20]

On 2014/12/10 22:56:23, cjhopman wrote:
> I expect you will still need to edit a recipe to actually have the bots run
the
> tests. The recipes are in a different repo, though. It might be best to file a
> ticket against infra (if you want it done quickly you may be able to get some
> direction in how to edit the recipe from them).

I will request the infra team to add 'ui_android_unittests' to android trybots
after this CL is landed.

[danakj, 2014-12-10 23:38:50]

On 2014/12/10 22:57:45, Jaekyun Seok wrote:
> Dana, could you please review ui/android/DEPS as an owner of cc/?

LGTM

[Jaekyun Seok (inactive), 2014-12-10 23:43:18]

The CQ bit was checked by jaekyun@chromium.org

[commit-bot: I haz the power, 2014-12-10 23:44:32]

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

[commit-bot: I haz the power, 2014-12-11 00:24:48]

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

[commit-bot: I haz the power, 2014-12-11 00:24:50]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Jaekyun Seok (inactive), 2014-12-11 00:58:48]

The CQ bit was checked by jaekyun@chromium.org

[commit-bot: I haz the power, 2014-12-11 01:00:24]

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

[commit-bot: I haz the power, 2014-12-11 02:49:23]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-12-11 02:50:03]

Patchset 7 (id:??) landed as
https://crrev.com/f229b81b8d31706d3aa922294d1e9c8f567bd400
Cr-Commit-Position: refs/heads/master@{#307844}