WIP - loop in LayoutManager for preferred size changes

WIP - loop in LayoutManager for preferred size changes

BUG=

[Evan Stade, 2013-05-08 22:31:42]

Scott, what do you think of this change? It seems like the responsibility for
knowing what to do when a child's size or visibility changes lies with the
layout manager when there is one.

I would like to do something similar for ChildVisibilityChanged as well.

https://chromiumcodereview.appspot.com/15071002/diff/1/ui/views/layout/layout...
File ui/views/layout/layout_manager.h (right):

https://chromiumcodereview.appspot.com/15071002/diff/1/ui/views/layout/layout...
ui/views/layout/layout_manager.h:61: void HostPreferredSizeShouldChange(View*
host);
this makes it so that it's no longer just an interface. I could change this by
friending every LayoutManager class that needs access to PreferredSizeChanged()
and all that, or just making the necessary methods public. Let me know which you
prefer.

[sky, 2013-05-09 14:54:15]

Do you have a use case as to what you're trying to accomplish?

[Evan Stade, 2013-05-09 18:14:49]

On 2013/05/09 14:54:15, sky wrote:
> Do you have a use case as to what you're trying to accomplish?

yes, I want SetVisible and PreferredSizeChanged to cause new layouts when
necessary. I think the fact this doesn't happen is a cause of a lot of the
painting issues we're seeing in the requestAutocomplete dialog.

The default View implementation is to do nothing, but for layouts that depend on
visibility or preferred size, it seems like the correct implementation is to
either propagate upwards (if the child's preferred size affects the host view's
preferred size) or to Layout() (if it does not). Otherwise, you have to do
things like add a view class just for propagation, as in
https://chromiumcodereview.appspot.com/14895004/diff/8001/chrome/browser/ui/v...

But since the layout manager is where the knowledge of how preferred size,
visibility, and layout interact, I thought it best to place the responsibility
for correctly handling this there.

[sky, 2013-05-09 19:29:18]

On Thu, May 9, 2013 at 11:14 AM,  <estade@chromium.org> wrote:
> On 2013/05/09 14:54:15, sky wrote:
>>
>> Do you have a use case as to what you're trying to accomplish?
>
>
> yes, I want SetVisible and PreferredSizeChanged to cause new layouts when
> necessary. I think the fact this doesn't happen is a cause of a lot of the
> painting issues we're seeing in the requestAutocomplete dialog.
>
> The default View implementation is to do nothing, but for layouts that
> depend on
> visibility or preferred size, it seems like the correct implementation is to
> either propagate upwards (if the child's preferred size affects the host
> view's
> preferred size) or to Layout() (if it does not). Otherwise, you have to do
> things like add a view class just for propagation, as in
>
https://chromiumcodereview.appspot.com/14895004/diff/8001/chrome/browser/ui/v...
>
> But since the layout manager is where the knowledge of how preferred size,
> visibility, and layout interact, I thought it best to place the
> responsibility
> for correctly handling this there.
>
> https://codereview.chromium.org/15071002/

I tend to think it's a combination of the view and layout manager that
know how to deal. In nearly all cases a visibility/preferred-size
change is going to effect the preferred size of the parent. And since
the change effects the parent, it likely effects all ancestors up the
view tree. The interesting thing is how far up the tree you need to go
before you layout. Some views effectively have a fixed side, so layout
should occur from them, othewise you likely need to start at the
RootView.

It seems like ChildPreferredSize should really continue up the tree.
We could even have a validate_root, that if true means
ChildVisibilityChanged doesn't continue up from such a view, but
instead does a Layout().

We initially shied away from any sort of automatic layout. It's too
easy to get wrong and end up doing lots of unnecessary layouts,
hurting performance. ChildPreferredSize is a step in the automatic
layout direction, and adding validate_root_ even more so. I tend to
think if we do add the validate_root then we would also need something
like schedulepaint. In so far as we don't layout immediately, but say
after a delay like SchedulePaint does.

Doing any of this has lots of subtle implications. I know Ben was
interested in removing InvalidateLayout() at some point, which is
tightly related to this. I'm adding him for his thoughts.

  -Scott

[Evan Stade, 2013-05-09 22:08:39]

On 2013/05/09 19:29:18, sky wrote:
> On Thu, May 9, 2013 at 11:14 AM,  <mailto:estade@chromium.org> wrote:
> > On 2013/05/09 14:54:15, sky wrote:
> >>
> >> Do you have a use case as to what you're trying to accomplish?
> >
> >
> > yes, I want SetVisible and PreferredSizeChanged to cause new layouts when
> > necessary. I think the fact this doesn't happen is a cause of a lot of the
> > painting issues we're seeing in the requestAutocomplete dialog.
> >
> > The default View implementation is to do nothing, but for layouts that
> > depend on
> > visibility or preferred size, it seems like the correct implementation is to
> > either propagate upwards (if the child's preferred size affects the host
> > view's
> > preferred size) or to Layout() (if it does not). Otherwise, you have to do
> > things like add a view class just for propagation, as in
> >
>
https://chromiumcodereview.appspot.com/14895004/diff/8001/chrome/browser/ui/v...
> >
> > But since the layout manager is where the knowledge of how preferred size,
> > visibility, and layout interact, I thought it best to place the
> > responsibility
> > for correctly handling this there.
> >
> > https://codereview.chromium.org/15071002/
> 
> I tend to think it's a combination of the view and layout manager that
> know how to deal. In nearly all cases a visibility/preferred-size
> change is going to effect the preferred size of the parent. And since
> the change effects the parent, it likely effects all ancestors up the
> view tree. The interesting thing is how far up the tree you need to go
> before you layout. Some views effectively have a fixed side, so layout
> should occur from them, othewise you likely need to start at the
> RootView.
> 
> It seems like ChildPreferredSize should really continue up the tree.
> We could even have a validate_root, that if true means
> ChildVisibilityChanged doesn't continue up from such a view, but
> instead does a Layout().
> 
> We initially shied away from any sort of automatic layout. It's too
> easy to get wrong and end up doing lots of unnecessary layouts,
> hurting performance. ChildPreferredSize is a step in the automatic
> layout direction, and adding validate_root_ even more so. I tend to
> think if we do add the validate_root then we would also need something
> like schedulepaint. In so far as we don't layout immediately, but say
> after a delay like SchedulePaint does.

The alternative to automatic is that the child has to know how far up the tree
the next fixed-size view is. This seems bug-prone.

> 
> Doing any of this has lots of subtle implications. I know Ben was
> interested in removing InvalidateLayout() at some point, which is
> tightly related to this. I'm adding him for his thoughts.
> 
>   -Scott

[sky, 2013-05-09 22:29:23]

On Thu, May 9, 2013 at 3:08 PM,  <estade@chromium.org> wrote:
> On 2013/05/09 19:29:18, sky wrote:
>
>> On Thu, May 9, 2013 at 11:14 AM,  <mailto:estade@chromium.org> wrote:
>> > On 2013/05/09 14:54:15, sky wrote:
>> >>
>> >> Do you have a use case as to what you're trying to accomplish?
>> >
>> >
>> > yes, I want SetVisible and PreferredSizeChanged to cause new layouts
>> > when
>> > necessary. I think the fact this doesn't happen is a cause of a lot of
>> > the
>> > painting issues we're seeing in the requestAutocomplete dialog.
>> >
>> > The default View implementation is to do nothing, but for layouts that
>> > depend on
>> > visibility or preferred size, it seems like the correct implementation
>> > is to
>> > either propagate upwards (if the child's preferred size affects the host
>> > view's
>> > preferred size) or to Layout() (if it does not). Otherwise, you have to
>> > do
>> > things like add a view class just for propagation, as in
>> >
>
>
>
https://chromiumcodereview.appspot.com/14895004/diff/8001/chrome/browser/ui/v...
>>
>> >
>> > But since the layout manager is where the knowledge of how preferred
>> > size,
>> > visibility, and layout interact, I thought it best to place the
>> > responsibility
>> > for correctly handling this there.
>> >
>> > https://codereview.chromium.org/15071002/
>
>
>> I tend to think it's a combination of the view and layout manager that
>> know how to deal. In nearly all cases a visibility/preferred-size
>> change is going to effect the preferred size of the parent. And since
>> the change effects the parent, it likely effects all ancestors up the
>> view tree. The interesting thing is how far up the tree you need to go
>> before you layout. Some views effectively have a fixed side, so layout
>> should occur from them, othewise you likely need to start at the
>> RootView.
>
>
>> It seems like ChildPreferredSize should really continue up the tree.
>> We could even have a validate_root, that if true means
>> ChildVisibilityChanged doesn't continue up from such a view, but
>> instead does a Layout().
>
>
>> We initially shied away from any sort of automatic layout. It's too
>> easy to get wrong and end up doing lots of unnecessary layouts,
>> hurting performance. ChildPreferredSize is a step in the automatic
>> layout direction, and adding validate_root_ even more so. I tend to
>> think if we do add the validate_root then we would also need something
>> like schedulepaint. In so far as we don't layout immediately, but say
>> after a delay like SchedulePaint does.
>
>
> The alternative to automatic is that the child has to know how far up the
> tree
> the next fixed-size view is. This seems bug-prone.

I'm ok with ChildPreferredSizeChanged going all the way up the tree.
You'll have to make sure that doesn't cause problems. I tend to think
it wouldn't.

  -Scott

>
>
>> Doing any of this has lots of subtle implications. I know Ben was
>> interested in removing InvalidateLayout() at some point, which is
>> tightly related to this. I'm adding him for his thoughts.
>
>
>>    -Scott
>
>
>
>
> https://codereview.chromium.org/15071002/

[Evan Stade, 2013-05-10 02:01:45]

I think I will have to come back to this after io.

[Evan Stade, 2013-05-23 21:26:43]

ok, thought about this some more. This CL is still a WIP but it does a few
things

1. as before, the layout manager can react to ChildPreferredSizeChanged.

2. PreferredSizeChanged additionally calls Layout() (but avoids calling it if
the size changed propagation has already caused a layout).

3. I created a scoping object to use in place of InvalidateLayout(), but then I
decided it was less useful than just doing a deferred layout. This CL has the
scoping object approach, https://codereview.chromium.org/15912007/ has the
deferred layouts.

This patch passes unit and manual tests, although I should probably add more
unit tests. WDYT?