Switch the android overscroll glow to use UIResources.

Switch the android overscroll glow to use UIResources.

This change migrates the android overscroll glow effect from using the
PictureLayer to UIResources and UIResourceLayers

BUG=

Subsumed by: https://codereview.chromium.org/82553015/

[powei, 2013-11-26 00:10:39]

PTAL.  Note that loading of the overscroll resources is now done at compositor
constructor.  Let me know what you think.

Thanks!

[jdduke (slow), 2013-11-26 01:39:53]

Thanks powei@, this will be a nice payment on the technical debt I've incurred
with the overscroll effect =/

https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
File content/browser/android/overscroll_glow.cc (right):

https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
content/browser/android/overscroll_glow.cc:57: make_scoped_ptr(new
EdgeEffect(edge_layer_, glow_layer_));
There can potentially be >1 EdgeEffects active at any given time (e.g., left +
top), in which case having but one set of layers will result in the properties
(size/opacity/orientation) being overwritten. I could optimize this a bit so
that only one of the {top,bottom} and {left,right} pairs are active at any given
time, but certainly we should allow for, say, left+top to overscroll
simultaneously.

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
content/browser/renderer_host/compositor_impl_android.cc:159:
TRACE_EVENT0("compositor", "CompositorImpl::LoadOverscrollResources");
We did some optimizations to improve overscroll resource load time, but it was
still fairly slow and so we kept it asynchronous.  Is there any way we can keep
the SkBitmap loading asynchronous here? Would that be too complicated?  Even a
PostDelayedTask()/PostTaskAndReply() would be an improvement here, deferring
overscroll resource id creation until such time as the bitmaps have loaded.

[no sievers, 2013-11-26 20:10:16]

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
content/browser/renderer_host/compositor_impl_android.cc:147:
LoadOverscrollResources();
I feel like all this is some very specific functionality that can maybe live in
a better place than the compositor itself (the functionality in here is generic
otherwise).

Maybe WindowAndroid? wdyt?

And then maybe (now or later), it could also make sense to move the overscroll
related code from RWHV into ViewAndroid. (I had also been wondering if
ViewAndroid would be a better place to hold the tab's layer node rather than
ContentViewCore doing it now.)

[powei, 2013-11-26 22:17:40]

+tedchoc@, dtrainor@ for WindowAndroid discussion.  

Question at hand is where to place resources for the overscroll glow.  This then
involves whether WindowAndroid should be able to talk to the compositor or not.

Suggestions are much appreciated.

https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
File content/browser/android/overscroll_glow.cc (right):

https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
content/browser/android/overscroll_glow.cc:57: make_scoped_ptr(new
EdgeEffect(edge_layer_, glow_layer_));
On 2013/11/26 01:39:54, jdduke wrote:
> There can potentially be >1 EdgeEffects active at any given time (e.g., left +
> top), in which case having but one set of layers will result in the properties
> (size/opacity/orientation) being overwritten. I could optimize this a bit so
> that only one of the {top,bottom} and {left,right} pairs are active at any
given
> time, but certainly we should allow for, say, left+top to overscroll
> simultaneously.

Done.  My bad for not noticing what's actually going on here.  I switched it
back to 2 layers per effect as before.

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
content/browser/renderer_host/compositor_impl_android.cc:147:
LoadOverscrollResources();
On 2013/11/26 20:10:17, sievers wrote:
> I feel like all this is some very specific functionality that can maybe live
in
> a better place than the compositor itself (the functionality in here is
generic
> otherwise).
> 
> Maybe WindowAndroid? wdyt?
> 
> And then maybe (now or later), it could also make sense to move the overscroll
> related code from RWHV into ViewAndroid. (I had also been wondering if
> ViewAndroid would be a better place to hold the tab's layer node rather than
> ContentViewCore doing it now.)

Placing it outside of the compositor sounds good to me, but then WindowAndroid
would have to talk to the compositor (to request the resources), which is not
possible currently (since ui does not depend on content).

Maybe move WindowAndroid and ViewAndroid?

https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
content/browser/renderer_host/compositor_impl_android.cc:159:
TRACE_EVENT0("compositor", "CompositorImpl::LoadOverscrollResources");
On 2013/11/26 01:39:54, jdduke wrote:
> We did some optimizations to improve overscroll resource load time, but it was
> still fairly slow and so we kept it asynchronous.  Is there any way we can
keep
> the SkBitmap loading asynchronous here? Would that be too complicated?  Even a
> PostDelayedTask()/PostTaskAndReply() would be an improvement here, deferring
> overscroll resource id creation until such time as the bitmaps have loaded.

Good idea.  I'll give asynchronous load a try.

[no sievers, 2013-11-26 22:23:37]

On 2013/11/26 22:17:40, powei wrote:
> +tedchoc@, dtrainor@ for WindowAndroid discussion.  
> 
> Question at hand is where to place resources for the overscroll glow.  This
then
> involves whether WindowAndroid should be able to talk to the compositor or
not.
> 

To satisfy DEPS you'd have to add another interface that the compositor
implements anyway.
Maybe that's not a bad thing. The window only needs a small set of functions for
managing UI resources right?


> Suggestions are much appreciated.
> 
>
https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
> File content/browser/android/overscroll_glow.cc (right):
> 
>
https://codereview.chromium.org/82553015/diff/80001/content/browser/android/o...
> content/browser/android/overscroll_glow.cc:57: make_scoped_ptr(new
> EdgeEffect(edge_layer_, glow_layer_));
> On 2013/11/26 01:39:54, jdduke wrote:
> > There can potentially be >1 EdgeEffects active at any given time (e.g., left
+
> > top), in which case having but one set of layers will result in the
properties
> > (size/opacity/orientation) being overwritten. I could optimize this a bit so
> > that only one of the {top,bottom} and {left,right} pairs are active at any
> given
> > time, but certainly we should allow for, say, left+top to overscroll
> > simultaneously.
> 
> Done.  My bad for not noticing what's actually going on here.  I switched it
> back to 2 layers per effect as before.
> 
>
https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
> File content/browser/renderer_host/compositor_impl_android.cc (right):
> 
>
https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
> content/browser/renderer_host/compositor_impl_android.cc:147:
> LoadOverscrollResources();
> On 2013/11/26 20:10:17, sievers wrote:
> > I feel like all this is some very specific functionality that can maybe live
> in
> > a better place than the compositor itself (the functionality in here is
> generic
> > otherwise).
> > 
> > Maybe WindowAndroid? wdyt?
> > 
> > And then maybe (now or later), it could also make sense to move the
overscroll
> > related code from RWHV into ViewAndroid. (I had also been wondering if
> > ViewAndroid would be a better place to hold the tab's layer node rather than
> > ContentViewCore doing it now.)
> 
> Placing it outside of the compositor sounds good to me, but then WindowAndroid
> would have to talk to the compositor (to request the resources), which is not
> possible currently (since ui does not depend on content).
> 
> Maybe move WindowAndroid and ViewAndroid?
> 
>
https://codereview.chromium.org/82553015/diff/80001/content/browser/renderer_...
> content/browser/renderer_host/compositor_impl_android.cc:159:
> TRACE_EVENT0("compositor", "CompositorImpl::LoadOverscrollResources");
> On 2013/11/26 01:39:54, jdduke wrote:
> > We did some optimizations to improve overscroll resource load time, but it
was
> > still fairly slow and so we kept it asynchronous.  Is there any way we can
> keep
> > the SkBitmap loading asynchronous here? Would that be too complicated?  Even
a
> > PostDelayedTask()/PostTaskAndReply() would be an improvement here, deferring
> > overscroll resource id creation until such time as the bitmaps have loaded.
> 
> Good idea.  I'll give asynchronous load a try.

[no sievers, 2013-11-26 22:26:13]

On 2013/11/26 22:23:37, sievers wrote:
> On 2013/11/26 22:17:40, powei wrote:
> > +tedchoc@, dtrainor@ for WindowAndroid discussion.  
> > 
> > Question at hand is where to place resources for the overscroll glow.  This
> then
> > involves whether WindowAndroid should be able to talk to the compositor or
> not.
> > 
> 
> To satisfy DEPS you'd have to add another interface that the compositor
> implements anyway.
> Maybe that's not a bad thing. The window only needs a small set of functions
for
> managing UI resources right?
> 
> 

It has another advantage: With your previous patch you explicitly avoided
exposing a raw compositor pointer to RWHV.
Instead you just had it talk to View/WindowAndroid where the lifetime between
the objects was already better defined.

If you keep abstracting things a bit at that level, you can avoid having to do
expose it now.

[powei, 2013-11-27 18:40:54]

On 2013/11/26 22:26:13, sievers wrote:
> On 2013/11/26 22:23:37, sievers wrote:
> > On 2013/11/26 22:17:40, powei wrote:
> > > +tedchoc@, dtrainor@ for WindowAndroid discussion.  
> > > 
> > > Question at hand is where to place resources for the overscroll glow. 
This
> > then
> > > involves whether WindowAndroid should be able to talk to the compositor or
> > not.
> > > 
> > 
> > To satisfy DEPS you'd have to add another interface that the compositor
> > implements anyway.
> > Maybe that's not a bad thing. The window only needs a small set of functions
> for
> > managing UI resources right?
> > 
> > 
> 
> It has another advantage: With your previous patch you explicitly avoided
> exposing a raw compositor pointer to RWHV.
> Instead you just had it talk to View/WindowAndroid where the lifetime between
> the objects was already better defined.
> 
> If you keep abstracting things a bit at that level, you can avoid having to do
> expose it now.

yeah, not having to expose the compositor pointer sounds good.  i'll move
forward in that direction (with the interface).  thanks.

[jdduke (slow), 2014-03-01 02:13:34]

On 2013/11/27 18:40:54, powei wrote:
> On 2013/11/26 22:26:13, sievers wrote:
> > On 2013/11/26 22:23:37, sievers wrote:
> > > On 2013/11/26 22:17:40, powei wrote:
> > > > +tedchoc@, dtrainor@ for WindowAndroid discussion.  
> > > > 
> > > > Question at hand is where to place resources for the overscroll glow. 
> This
> > > then
> > > > involves whether WindowAndroid should be able to talk to the compositor
or
> > > not.
> > > > 
> > > 
> > > To satisfy DEPS you'd have to add another interface that the compositor
> > > implements anyway.
> > > Maybe that's not a bad thing. The window only needs a small set of
functions
> > for
> > > managing UI resources right?
> > > 
> > > 
> > 
> > It has another advantage: With your previous patch you explicitly avoided
> > exposing a raw compositor pointer to RWHV.
> > Instead you just had it talk to View/WindowAndroid where the lifetime
between
> > the objects was already better defined.
> > 
> > If you keep abstracting things a bit at that level, you can avoid having to
do
> > expose it now.
> 
> yeah, not having to expose the compositor pointer sounds good.  i'll move
> forward in that direction (with the interface).  thanks.

Are we still planning on making this (or a similar) change?

[aelias_OOO_until_Jul13, 2014-03-01 02:50:05]

Yes, I think so, although this particular one wasn't blocking ubercomp so we
haven't prioritized it.

[powei, 2014-03-03 18:35:42]

Yes, I think this is still a good idea.  We had a meeting (a few months ago)
about having a general purpose UIResourceProvider interface to be tied to
WindowAndroid.  The plan was that should be done first, then this overscroll
move would naturally follow.

[powei, 2014-07-08 19:10:30]

Closed since I can't figure out how to upload to this patch with the new
changes.  'git cl upload' after 'git cl patch' did not work.

continuation is here: https://codereview.chromium.org/377013002/