Introduce RootWindowProperty for RootWindow's properties

Introduce RootWindowProperty for RootWindow's properties

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

[James Cook, 2013-08-30 00:06:36]

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.cc
File ash/root_window_property.cc (right):

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.c...
ash/root_window_property.cc:21: display_id(gfx::Display::kInvalidDisplayID) {
Initialize controller to NULL?

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h
File ash/root_window_property.h (right):

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:20: struct RootWindowProperty {
Should this be RootWindowProperties because it contains multiple properties?  Or
maybe RootWindowConfiguration or RootWindowSettings?

I think the latter names might be better, since this doesn't actually store
window properties in the aura::WindowProperty sense.

Also, this struct should have a comment telling why it should be used instead of
general window properties.

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:36: // new instance of the |root| does not exist and
never returns NULL.
of -> if

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:39: // Returns the RootWindowProperty for const
|root|. This may return NULL.
Should this version just cast away the const and never return NULL?  It seems
like it might be hard for a caller to know which version they are calling and
hence whether or not to check for NULL.

[James Cook, 2013-08-30 00:06:54]

Oh, and I like the idea of consolidating all these settings in one place!

[oshima, 2013-08-30 05:38:32]

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.cc
File ash/root_window_property.cc (right):

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.c...
ash/root_window_property.cc:21: display_id(gfx::Display::kInvalidDisplayID) {
On 2013/08/30 00:06:36, James Cook wrote:
> Initialize controller to NULL?

Done.

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h
File ash/root_window_property.h (right):

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:20: struct RootWindowProperty {
On 2013/08/30 00:06:36, James Cook wrote:
> Should this be RootWindowProperties because it contains multiple properties? 
Or
> maybe RootWindowConfiguration or RootWindowSettings?
> 
> I think the latter names might be better, since this doesn't actually store
> window properties in the aura::WindowProperty sense.
> 
> Also, this struct should have a comment telling why it should be used instead
of
> general window properties.

I thought about RootWindowProperties, but I picked RootWindowProperty to see
what you think :) RootWindowSettings sounds a bit odd to me, but probably
because I'm not native speaker.

Here are all candidates including ones I thought of:

RootWindowAshInfo
RootWindowSettings
RootWindowExtraInfo
RootWindowProperties
RootWindowConfiguration

Let me know your pick.

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:36: // new instance of the |root| does not exist and
never returns NULL.
On 2013/08/30 00:06:36, James Cook wrote:
> of -> if

Done.

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:39: // Returns the RootWindowProperty for const
|root|. This may return NULL.
On 2013/08/30 00:06:36, James Cook wrote:
> Should this version just cast away the const and never return NULL?  It seems
> like it might be hard for a caller to know which version they are calling and
> hence whether or not to check for NULL.

I added InitRootWindowProperty instead to explicitly initialize the property.

[James Cook, 2013-08-30 16:26:25]

lgtm after naming is fixed

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h
File ash/root_window_property.h (right):

https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
ash/root_window_property.h:20: struct RootWindowProperty {
On 2013/08/30 05:38:33, oshima wrote:
> On 2013/08/30 00:06:36, James Cook wrote:
> > Should this be RootWindowProperties because it contains multiple properties?

> Or
> > maybe RootWindowConfiguration or RootWindowSettings?
> > 
> > I think the latter names might be better, since this doesn't actually store
> > window properties in the aura::WindowProperty sense.
> > 
> > Also, this struct should have a comment telling why it should be used
instead
> of
> > general window properties.
> 
> I thought about RootWindowProperties, but I picked RootWindowProperty to see
> what you think :) RootWindowSettings sounds a bit odd to me, but probably
> because I'm not native speaker.
> 
> Here are all candidates including ones I thought of:
> 
> RootWindowAshInfo
> RootWindowSettings
> RootWindowExtraInfo
> RootWindowProperties
> RootWindowConfiguration
> 
> Let me know your pick.

I like RootWindowSettings the best.  (Yeah, it's a little weird, but I really
want to avoid the word Properties.)

Also, this struct neeeds a comment.

[oshima, 2013-08-30 18:56:43]

On 2013/08/30 16:26:25, James Cook wrote:
> lgtm after naming is fixed
> 
> https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h
> File ash/root_window_property.h (right):
> 
>
https://codereview.chromium.org/23496024/diff/5001/ash/root_window_property.h...
> ash/root_window_property.h:20: struct RootWindowProperty {
> On 2013/08/30 05:38:33, oshima wrote:
> > On 2013/08/30 00:06:36, James Cook wrote:
> > > Should this be RootWindowProperties because it contains multiple
properties?
> 
> > Or
> > > maybe RootWindowConfiguration or RootWindowSettings?
> > > 
> > > I think the latter names might be better, since this doesn't actually
store
> > > window properties in the aura::WindowProperty sense.
> > > 
> > > Also, this struct should have a comment telling why it should be used
> instead
> > of
> > > general window properties.
> > 
> > I thought about RootWindowProperties, but I picked RootWindowProperty to see
> > what you think :) RootWindowSettings sounds a bit odd to me, but probably
> > because I'm not native speaker.
> > 
> > Here are all candidates including ones I thought of:
> > 
> > RootWindowAshInfo
> > RootWindowSettings
> > RootWindowExtraInfo
> > RootWindowProperties
> > RootWindowConfiguration
> > 
> > Let me know your pick.
> 
> I like RootWindowSettings the best.  (Yeah, it's a little weird, but I really
> want to avoid the word Properties.)
Ok, renamed to RootWindowSettings

> 
> Also, this struct neeeds a comment.

done

[commit-bot: I haz the power, 2013-08-30 18:56:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/23496024/23001

[commit-bot: I haz the power, 2013-08-30 21:49:12]

Change committed as 220667