views: Have a more accurate name for View parent_owned accessors.

views: Have a more accurate name for View parent_owned accessors.

Remove the getter accessor of parent_owned() because it's used only internally by View.
And change the setter accessor from set_parent_owned() to set_owned_by_client().

BUG=122384
R=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=136766

[tfarina, 2012-05-10 16:43:14]

Hi Ben, could you review this to me?

[Ben Goodger (Google), 2012-05-10 16:51:19]

http://codereview.chromium.org/10384068/diff/1/ui/views/view.h
File ui/views/view.h (right):

http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode121
ui/views/view.h:121: bool owned_by_parent() const { return owned_by_parent_; }
is this accessor used by anyone outside of View? seems like it could be
unnecessary.

http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode122
ui/views/view.h:122: void set_not_owned_by_parent() { owned_by_parent_ = false;
}
this function name feels awkward to me (though so did the old one).

there are really two modes of ownership:

- hierarchy (specifically, the parent views::View)
- client code (code that uses views)

Perhaps this could be set_owned_by_client().

[tfarina, 2012-05-10 16:59:04]

On Thu, May 10, 2012 at 1:51 PM,  <ben@chromium.org> wrote:
>
> http://codereview.chromium.org/10384068/diff/1/ui/views/view.h
> File ui/views/view.h (right):
>
> http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode121
> ui/views/view.h:121: bool owned_by_parent() const { return
> owned_by_parent_; }
> is this accessor used by anyone outside of View? seems like it could be
> unnecessary.
>
> http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode122
> ui/views/view.h:122: void set_not_owned_by_parent() { owned_by_parent_ =
> false; }
> this function name feels awkward to me (though so did the old one).
>
> there are really two modes of ownership:
>
> - hierarchy (specifically, the parent views::View)
> - client code (code that uses views)
>
> Perhaps this could be set_owned_by_client().
>
If I'm interpreting this right, the suggestion is to replace
parent_owned_/owned_by_parent_ by owned_by_client_.

And that means reverting the condition from:

parent_owned_ = true; // Default

to:

owned_by_client_ = false;  // Default

And thus:

void set_owned_by_client() { owned_by_client_ = true; }

?

-- 
Thiago

[Ben Goodger (Google), 2012-05-10 21:22:56]

sounds good.

-Ben

On Thu, May 10, 2012 at 9:59 AM, Thiago Farina <tfarina@chromium.org> wrote:

> On Thu, May 10, 2012 at 1:51 PM,  <ben@chromium.org> wrote:
> >
> > http://codereview.chromium.org/10384068/diff/1/ui/views/view.h
> > File ui/views/view.h (right):
> >
> >
> http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode121
> > ui/views/view.h:121: bool owned_by_parent() const { return
> > owned_by_parent_; }
> > is this accessor used by anyone outside of View? seems like it could be
> > unnecessary.
> >
> >
> http://codereview.chromium.org/10384068/diff/1/ui/views/view.h#newcode122
> > ui/views/view.h:122: void set_not_owned_by_parent() { owned_by_parent_ =
> > false; }
> > this function name feels awkward to me (though so did the old one).
> >
> > there are really two modes of ownership:
> >
> > - hierarchy (specifically, the parent views::View)
> > - client code (code that uses views)
> >
> > Perhaps this could be set_owned_by_client().
> >
> If I'm interpreting this right, the suggestion is to replace
> parent_owned_/owned_by_parent_ by owned_by_client_.
>
> And that means reverting the condition from:
>
> parent_owned_ = true; // Default
>
> to:
>
> owned_by_client_ = false;  // Default
>
> And thus:
>
> void set_owned_by_client() { owned_by_client_ = true; }
>
> ?
>
> --
> Thiago
>

[tfarina, 2012-05-10 22:02:27]

set_owned_by_client

[tfarina, 2012-05-10 22:04:21]

On 2012/05/10 21:22:56, Ben Goodger (Google) wrote:
> sounds good.
> 
Done.

[Ben Goodger (Google), 2012-05-10 22:05:53]

lgtm

[tfarina, 2012-05-11 17:10:56]

rebase