Disable setting incognito prefs for extensions for now.

Disable setting incognito prefs for extensions for now.


BUG=60099
TEST=no

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86666

[battre, 2011-05-24 10:58:45]

This CL disables setting incognito preferences for extensions, as discussed by
email.

Some details:

- I have left (rather than deleted) most code for handling incognito preferences
as this can be used for the final version in large parts.

- I have renamed preferences.get() to preferences.getEffective(). The reason is
that set() writes preferences for the current extension, therefore one would
expect get() to be symmetric and read these preferences. However, the current
implementation returns the effective values. In order to be able to implement a
get() method that is symmetric to set(), I needed to free the function-name.

- I want to add a static html document for the preferences API documentation. I
will do this in a separate CL to speed things up and separate code- and
documentation review.

@mnissler: Please do a pre-review
@bauerb, mpcomplete: Please do the main review.

Thanks,
Dominic

[Mattias Nissler (ping if slow), 2011-05-24 11:31:17]

What I see looks reasonable, but since I'm not at all familiar with extension
APIs, I have to defer to the rest of the gang.

[battre, 2011-05-24 12:26:13]

I have renamed the parameter of set and clear from
set( value: ..., {incognito: 'no'} );
to
set( value: ..., {scope: 'regular'} );

There are two reasons:

- for getEffective, we have boolean parameter 'incognito'. I think it is good to
give the boolean and the tri-state (formerly: no, session_only, persistent)
parameter different names.

- it struck me as strange to call set(..., incognito: 'no') and then
getEffective(..., incognito: true) returns returns the value we have just set.

[Bernhard Bauer, 2011-05-24 12:59:52]

On 2011/05/24 10:58:45, battre wrote:
> This CL disables setting incognito preferences for extensions, as discussed by
> email.
> 
> Some details:
> 
> - I have left (rather than deleted) most code for handling incognito
preferences
> as this can be used for the final version in large parts.
> 
> - I have renamed preferences.get() to preferences.getEffective(). The reason
is
> that set() writes preferences for the current extension, therefore one would
> expect get() to be symmetric and read these preferences. However, the current
> implementation returns the effective values. In order to be able to implement
a
> get() method that is symmetric to set(), I needed to free the function-name.

Do we want to do that? It seems that an extension could always just remember the
value it has set (and if that value gets cleared, get will return a different
levelOfControl).

> - I want to add a static html document for the preferences API documentation.
I
> will do this in a separate CL to speed things up and separate code- and
> documentation review.
> 
> @mnissler: Please do a pre-review
> @bauerb, mpcomplete: Please do the main review.
> 
> Thanks,
> Dominic

[battre, 2011-05-24 13:05:51]

On 2011/05/24 12:59:52, Bernhard Bauer wrote:
> On 2011/05/24 10:58:45, battre wrote:
> > This CL disables setting incognito preferences for extensions, as discussed
by
> > email.
> > 
> > Some details:
> > 
> > - I have left (rather than deleted) most code for handling incognito
> preferences
> > as this can be used for the final version in large parts.
> > 
> > - I have renamed preferences.get() to preferences.getEffective(). The reason
> is
> > that set() writes preferences for the current extension, therefore one would
> > expect get() to be symmetric and read these preferences. However, the
current
> > implementation returns the effective values. In order to be able to
implement
> a
> > get() method that is symmetric to set(), I needed to free the function-name.
> 
> Do we want to do that? It seems that an extension could always just remember
the
> value it has set (and if that value gets cleared, get will return a different
> levelOfControl).

If the value is overridden by a policy, you cannot see that it was cleared.

Even if we never implemented a get() method (which I think we should), I think
renaming the get() to getEffective() is the right thing to do because the
current get() is not symmetric to set().

[Bernhard Bauer, 2011-05-24 16:05:08]

On Tue, May 24, 2011 at 06:05,  <battre@chromium.org> wrote:
> On 2011/05/24 12:59:52, Bernhard Bauer wrote:
>>
>> On 2011/05/24 10:58:45, battre wrote:
>> > This CL disables setting incognito preferences for extensions, as
>> > discussed
>
> by
>>
>> > email.
>> >
>> > Some details:
>> >
>> > - I have left (rather than deleted) most code for handling incognito
>> preferences
>> > as this can be used for the final version in large parts.
>> >
>> > - I have renamed preferences.get() to preferences.getEffective(). The
>> > reason
>> is
>> > that set() writes preferences for the current extension, therefore one
>> > would
>> > expect get() to be symmetric and read these preferences. However, the
>
> current
>>
>> > implementation returns the effective values. In order to be able to
>
> implement
>>
>> a
>> > get() method that is symmetric to set(), I needed to free the
>> > function-name.
>
>> Do we want to do that? It seems that an extension could always just
>> remember
>
> the
>>
>> value it has set (and if that value gets cleared, get will return a
>> different
>> levelOfControl).
>
> If the value is overridden by a policy, you cannot see that it was cleared.

But in that case it wouldn't really matter, as it was never effective
in the first place.

> Even if we never implemented a get() method (which I think we should), I
> think
> renaming the get() to getEffective() is the right thing to do because the
> current get() is not symmetric to set().

I think what irks me most about that is that everywhere else the
concept of an "effective" preference value is implicit, i.e. in
PrefService::GetXXX. Conceptually, a preference just has a value
(which may come from one of multiple possible sources), and the last
value which was set by an extension is just that. The |get| and |set|
methods are symmetric in the sense that |get| reads the value and
|set| tries to change it, it's just that a |set| call might not be
*effective* if another higher-precedence source has already set a
value.

In any case, I think that's a change which should be discussed in a
different CL.

> http://codereview.chromium.org/6990054/
>

[battre, 2011-05-24 16:17:34]

On 2011/05/24 16:05:08, Bernhard Bauer wrote:
> On Tue, May 24, 2011 at 06:05,  <mailto:battre@chromium.org> wrote:
> > On 2011/05/24 12:59:52, Bernhard Bauer wrote:
> >>
> >> On 2011/05/24 10:58:45, battre wrote:
> >> > This CL disables setting incognito preferences for extensions, as
> >> > discussed by email.
> >> >
> >> > Some details:
> >> >
> >> > - I have left (rather than deleted) most code for handling incognito
> >> preferences
> >> > as this can be used for the final version in large parts.
> >> >
> >> > - I have renamed preferences.get() to preferences.getEffective(). The
> >> > reason is
> >> > that set() writes preferences for the current extension, therefore one
> >> > would
> >> > expect get() to be symmetric and read these preferences. However, the
> >> > current
> >> > implementation returns the effective values. In order to be able to
> >> > implement a
> >> > get() method that is symmetric to set(), I needed to free the
> >> > function-name.
> >
> >> Do we want to do that? It seems that an extension could always just
> >> remember the
> >> value it has set (and if that value gets cleared, get will return a
> >> different levelOfControl).
> >
> > If the value is overridden by a policy, you cannot see that it was cleared.
> 
> But in that case it wouldn't really matter, as it was never effective
> in the first place.

Well, the policy can be disabled at runtime. Either way, as there is no symmetry
between set and the old get, I consider the old approach unclean. Note that set
takes a tri-state (regular, incognito_persistent, incognito_transient - or
whatever we call it) and get (or getEffective) takes a boolean. That is another
reason to differentiate the name.

And I actually do see a very good purpose for a get() method that queries the
preferences of this extension (not the effective preferences):
Assume you have restarted your browser and want to populate the extension's UI
with the preferences set by this extension. By having a get() method, we do not
need to write everything to localstate and read it from there. The proposed
approach would be significantly cleaner.

> > Even if we never implemented a get() method (which I think we should), I
> > think
> > renaming the get() to getEffective() is the right thing to do because the
> > current get() is not symmetric to set().
> 
> I think what irks me most about that is that everywhere else the
> concept of an "effective" preference value is implicit, i.e. in
> PrefService::GetXXX. 

PrefService::GetXXX is not user facing.


> Conceptually, a preference just has a value
> (which may come from one of multiple possible sources), and the last
> value which was set by an extension is just that. The |get| and |set|
> methods are symmetric in the sense that |get| reads the value and
> |set| tries to change it, it's just that a |set| call might not be
> *effective* if another higher-precedence source has already set a
> value.
> 
> In any case, I think that's a change which should be discussed in a
> different CL.

Well, I would rather not move stuff from one CL to another at this time if it is
not necessary. Lets get another opinion and decide.

[Bernhard Bauer, 2011-05-24 16:25:02]

On Tue, May 24, 2011 at 09:17,  <battre@chromium.org> wrote:
> On 2011/05/24 16:05:08, Bernhard Bauer wrote:
>>
>> On Tue, May 24, 2011 at 06:05,  <mailto:battre@chromium.org> wrote:
>> > On 2011/05/24 12:59:52, Bernhard Bauer wrote:
>> >>
>> >> On 2011/05/24 10:58:45, battre wrote:
>> >> > This CL disables setting incognito preferences for extensions, as
>> >> > discussed by email.
>> >> >
>> >> > Some details:
>> >> >
>> >> > - I have left (rather than deleted) most code for handling incognito
>> >> preferences
>> >> > as this can be used for the final version in large parts.
>> >> >
>> >> > - I have renamed preferences.get() to preferences.getEffective(). The
>> >> > reason is
>> >> > that set() writes preferences for the current extension, therefore
>> >> > one
>> >> > would
>> >> > expect get() to be symmetric and read these preferences. However, the
>> >> > current
>> >> > implementation returns the effective values. In order to be able to
>> >> > implement a
>> >> > get() method that is symmetric to set(), I needed to free the
>> >> > function-name.
>> >
>> >> Do we want to do that? It seems that an extension could always just
>> >> remember the
>> >> value it has set (and if that value gets cleared, get will return a
>> >> different levelOfControl).
>> >
>> > If the value is overridden by a policy, you cannot see that it was
>> > cleared.
>
>> But in that case it wouldn't really matter, as it was never effective
>> in the first place.
>
> Well, the policy can be disabled at runtime. Either way, as there is no
> symmetry
> between set and the old get, I consider the old approach unclean. Note that
> set
> takes a tri-state (regular, incognito_persistent, incognito_transient - or
> whatever we call it) and get (or getEffective) takes a boolean. That is
> another
> reason to differentiate the name.
>
> And I actually do see a very good purpose for a get() method that queries
> the
> preferences of this extension (not the effective preferences):
> Assume you have restarted your browser and want to populate the extension's
> UI
> with the preferences set by this extension. By having a get() method, we do
> not
> need to write everything to localstate and read it from there. The proposed
> approach would be significantly cleaner.

That is exactly not what I want. An extension should just display the
preference value, and disable the UI elements for changing it if it
can't be effectively changed (like we do for managed preferences).
Displaying a value and noting that this value is not actually what is
in effect seems confusing to me.

>> > Even if we never implemented a get() method (which I think we should), I
>> > think
>> > renaming the get() to getEffective() is the right thing to do because
>> > the
>> > current get() is not symmetric to set().
>
>> I think what irks me most about that is that everywhere else the
>> concept of an "effective" preference value is implicit, i.e. in
>> PrefService::GetXXX.
>
> PrefService::GetXXX is not user facing.
>
>
>> Conceptually, a preference just has a value
>> (which may come from one of multiple possible sources), and the last
>> value which was set by an extension is just that. The |get| and |set|
>> methods are symmetric in the sense that |get| reads the value and
>> |set| tries to change it, it's just that a |set| call might not be
>> *effective* if another higher-precedence source has already set a
>> value.
>
>> In any case, I think that's a change which should be discussed in a
>> different CL.
>
> Well, I would rather not move stuff from one CL to another at this time if
> it is
> not necessary. Lets get another opinion and decide.
>
>
>
> http://codereview.chromium.org/6990054/
>

[battre, 2011-05-24 16:43:19]

On 2011/05/24 16:25:02, Bernhard Bauer wrote:
> On Tue, May 24, 2011 at 09:17,  <mailto:battre@chromium.org> wrote:
> > On 2011/05/24 16:05:08, Bernhard Bauer wrote:
> >>
> >> On Tue, May 24, 2011 at 06:05, &nbsp;<mailto:battre@chromium.org> wrote:
> >> > On 2011/05/24 12:59:52, Bernhard Bauer wrote:
> >> >>
> >> >> On 2011/05/24 10:58:45, battre wrote:
> >> >> > This CL disables setting incognito preferences for extensions, as
> >> >> > discussed by email.
> >> >> >
> >> >> > Some details:
> >> >> >
> >> >> > - I have left (rather than deleted) most code for handling incognito
> >> >> preferences
> >> >> > as this can be used for the final version in large parts.
> >> >> >
> >> >> > - I have renamed preferences.get() to preferences.getEffective(). The
> >> >> > reason is
> >> >> > that set() writes preferences for the current extension, therefore
> >> >> > one
> >> >> > would
> >> >> > expect get() to be symmetric and read these preferences. However, the
> >> >> > current
> >> >> > implementation returns the effective values. In order to be able to
> >> >> > implement a
> >> >> > get() method that is symmetric to set(), I needed to free the
> >> >> > function-name.
> >> >
> >> >> Do we want to do that? It seems that an extension could always just
> >> >> remember the
> >> >> value it has set (and if that value gets cleared, get will return a
> >> >> different levelOfControl).
> >> >
> >> > If the value is overridden by a policy, you cannot see that it was
> >> > cleared.
> >
> >> But in that case it wouldn't really matter, as it was never effective
> >> in the first place.
> >
> > Well, the policy can be disabled at runtime. Either way, as there is no
> > symmetry
> > between set and the old get, I consider the old approach unclean. Note that
> > set
> > takes a tri-state (regular, incognito_persistent, incognito_transient - or
> > whatever we call it) and get (or getEffective) takes a boolean. That is
> > another
> > reason to differentiate the name.
> >
> > And I actually do see a very good purpose for a get() method that queries
> > the
> > preferences of this extension (not the effective preferences):
> > Assume you have restarted your browser and want to populate the extension's
> > UI
> > with the preferences set by this extension. By having a get() method, we do
> > not
> > need to write everything to localstate and read it from there. The proposed
> > approach would be significantly cleaner.
> 
> That is exactly not what I want. An extension should just display the
> preference value, and disable the UI elements for changing it if it
> can't be effectively changed (like we do for managed preferences).
> Displaying a value and noting that this value is not actually what is
> in effect seems confusing to me.

But I want to allow both.

This is a sample protocol for the proxy settings API.

1) Chrome is started
2) User clicks on button of proxy settings extension
3) Extension uses get() to query the values it had configured in the previous
browser session (which are still stored for regular preferences and persistent
incognito preferences) and populates the GUI with these values.
4) Extension uses getEffective() to check whether it is the extension that owns
the effective preferences. If it is, everything is good. If it is not, it
disables the controls of the UI.

What I am saying is:
- having a get() function as described above is very useful (I don't need to
store everything to the pref system *and* local store; I am guaranteed that I
don't have two stores that can be out of sync).
- we probably want to have a get() function with this functionality in the
future and we do not want the name "get()" to be blocked by something else
- I consider "getEffective()" to be a more appropriate name for getting the
effective preferences.

We might need to rename some functions in PrefService::GetXXX in order to
implement the functionality outlined here, but this is not user facing and does
not require changing the API.

[Bernhard Bauer, 2011-05-24 17:01:43]

On Tue, May 24, 2011 at 09:43,  <battre@chromium.org> wrote:
> On 2011/05/24 16:25:02, Bernhard Bauer wrote:
>>
>> On Tue, May 24, 2011 at 09:17,  <mailto:battre@chromium.org> wrote:
>> > On 2011/05/24 16:05:08, Bernhard Bauer wrote:
>> >>
>> >> On Tue, May 24, 2011 at 06:05, &nbsp;<mailto:battre@chromium.org>
>> >> wrote:
>> >> > On 2011/05/24 12:59:52, Bernhard Bauer wrote:
>> >> >>
>> >> >> On 2011/05/24 10:58:45, battre wrote:
>> >> >> > This CL disables setting incognito preferences for extensions, as
>> >> >> > discussed by email.
>> >> >> >
>> >> >> > Some details:
>> >> >> >
>> >> >> > - I have left (rather than deleted) most code for handling
>> >> >> > incognito
>> >> >> preferences
>> >> >> > as this can be used for the final version in large parts.
>> >> >> >
>> >> >> > - I have renamed preferences.get() to preferences.getEffective().
>> >> >> > The
>> >> >> > reason is
>> >> >> > that set() writes preferences for the current extension, therefore
>> >> >> > one
>> >> >> > would
>> >> >> > expect get() to be symmetric and read these preferences. However,
>> >> >> > the
>> >> >> > current
>> >> >> > implementation returns the effective values. In order to be able
>> >> >> > to
>> >> >> > implement a
>> >> >> > get() method that is symmetric to set(), I needed to free the
>> >> >> > function-name.
>> >> >
>> >> >> Do we want to do that? It seems that an extension could always just
>> >> >> remember the
>> >> >> value it has set (and if that value gets cleared, get will return a
>> >> >> different levelOfControl).
>> >> >
>> >> > If the value is overridden by a policy, you cannot see that it was
>> >> > cleared.
>> >
>> >> But in that case it wouldn't really matter, as it was never effective
>> >> in the first place.
>> >
>> > Well, the policy can be disabled at runtime. Either way, as there is no
>> > symmetry
>> > between set and the old get, I consider the old approach unclean. Note
>> > that
>> > set
>> > takes a tri-state (regular, incognito_persistent, incognito_transient -
>> > or
>> > whatever we call it) and get (or getEffective) takes a boolean. That is
>> > another
>> > reason to differentiate the name.
>> >
>> > And I actually do see a very good purpose for a get() method that
>> > queries
>> > the
>> > preferences of this extension (not the effective preferences):
>> > Assume you have restarted your browser and want to populate the
>> > extension's
>> > UI
>> > with the preferences set by this extension. By having a get() method, we
>> > do
>> > not
>> > need to write everything to localstate and read it from there. The
>> > proposed
>> > approach would be significantly cleaner.
>
>> That is exactly not what I want. An extension should just display the
>> preference value, and disable the UI elements for changing it if it
>> can't be effectively changed (like we do for managed preferences).
>> Displaying a value and noting that this value is not actually what is
>> in effect seems confusing to me.
>
> But I want to allow both.
>
> This is a sample protocol for the proxy settings API.
>
> 1) Chrome is started
> 2) User clicks on button of proxy settings extension
> 3) Extension uses get() to query the values it had configured in the
> previous
> browser session (which are still stored for regular preferences and
> persistent
> incognito preferences) and populates the GUI with these values.
> 4) Extension uses getEffective() to check whether it is the extension that
> owns
> the effective preferences. If it is, everything is good. If it is not, it
> disables the controls of the UI.

But then we would show the user a different proxy configuration from
the one actually used by the browser, which seems very wrong to me.
Think of it this way: If we have a managed preference, we also don't
show the value set by the user, we show the value set by the policy.

> What I am saying is:
> - having a get() function as described above is very useful (I don't need to
> store everything to the pref system *and* local store; I am guaranteed that
> I
> don't have two stores that can be out of sync).

I actually think of it the other way around: If an extension really
wants to get back the value it had previously set, it can just use
local storage without us having to introduce an additional API.

> - we probably want to have a get() function with this functionality in the
> future and we do not want the name "get()" to be blocked by something else
> - I consider "getEffective()" to be a more appropriate name for getting the
> effective preferences.

As I said, we don't really have a concept of effective preference
values, we just have preference values. All that fancy stuff with some
extensions having precedence over others is just to keep extensions
from fighting over a preference by resetting the value all the time,
but conceptually it's still a single value.

I think we have some fundamental differences in how we see the
preference stack here, which is why I suggested moving this part to a
separate CL from the rest, which was basically already agreed upon.

> We might need to rename some functions in PrefService::GetXXX in order to
> implement the functionality outlined here, but this is not user facing and
> does
> not require changing the API.
>
>
> http://codereview.chromium.org/6990054/
>

[Bernhard Bauer, 2011-05-24 17:16:15]

As discussed offline, LGTM for everything but renaming |get| to |getEffective|
(and |onChange| to |onChangeEffective|).

[battre, 2011-05-24 17:20:54]

On 2011/05/24 17:16:15, Bernhard Bauer wrote:
> As discussed offline, LGTM for everything but renaming |get| to |getEffective|
> (and |onChange| to |onChangeEffective|).

Matt, we'd like to ask you to serve as a tie-breaker. :-)

[Matt Perry, 2011-05-24 19:41:50]

On 2011/05/24 17:20:54, battre wrote:
> On 2011/05/24 17:16:15, Bernhard Bauer wrote:
> > As discussed offline, LGTM for everything but renaming |get| to
|getEffective|
> > (and |onChange| to |onChangeEffective|).
> 
> Matt, we'd like to ask you to serve as a tie-breaker. :-)

We should stick with a single get() that returns the effective pref.

In general, we want to expose as few Chrome implementation details to extensions
as possible. This will make it easier to avoid breaking the APIs as Chrome
changes. The fact that Chrome actually keeps track of all the individual
extension settings is an implementation detail. I don't think we should have an
API that requires us to remember each setting made by every extension, and
extensions should not care how Chrome decides the final pref value. Besides,
extensions can easily work around the lack of a get() by remembering their
settings in localStorage.

I have a slight preference for keeping the original naming (no "effective"),
because every setting is "effective".

[battre, 2011-05-24 19:58:38]

'Effective' is gone.

[Bernhard Bauer, 2011-05-24 20:14:57]

Basically LGTM, but I want to make sure we get the enum right:

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
chrome/common/extensions/api/extension_api.json:5387: "enum": ["regular"],
What are going to be the future values? "incognito_temporary" and
"incognito_persistent"? I'm not sure I really like that (but OTOH I don't have a
better suggestion either).

[battre, 2011-05-24 20:16:50]

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
chrome/common/extensions/api/extension_api.json:5387: "enum": ["regular"],
On 2011/05/24 20:14:57, Bernhard Bauer wrote:
> What are going to be the future values? "incognito_temporary" and
> "incognito_persistent"? I'm not sure I really like that (but OTOH I don't have
a
> better suggestion either).

My current proposal is "incognito_persistent" and "incognito_session_only".

[Bernhard Bauer, 2011-05-24 20:26:30]

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
chrome/common/extensions/api/extension_api.json:5387: "enum": ["regular"],
On 2011/05/24 20:16:50, battre wrote:
> On 2011/05/24 20:14:57, Bernhard Bauer wrote:
> > What are going to be the future values? "incognito_temporary" and
> > "incognito_persistent"? I'm not sure I really like that (but OTOH I don't
have
> a
> > better suggestion either).
> 
> My current proposal is "incognito_persistent" and "incognito_session_only".

Fine with me. Matt, any objections?

[Matt Perry, 2011-05-24 21:07:34]

On 2011/05/24 20:26:30, Bernhard Bauer wrote:
>
http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
> File chrome/common/extensions/api/extension_api.json (right):
> 
>
http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
> chrome/common/extensions/api/extension_api.json:5387: "enum": ["regular"],
> On 2011/05/24 20:16:50, battre wrote:
> > On 2011/05/24 20:14:57, Bernhard Bauer wrote:
> > > What are going to be the future values? "incognito_temporary" and
> > > "incognito_persistent"? I'm not sure I really like that (but OTOH I don't
> have
> > a
> > > better suggestion either).
> > 
> > My current proposal is "incognito_persistent" and "incognito_session_only".
> 
> Fine with me. Matt, any objections?

Sounds good. Though I wonder about naming style. This is javascript, so should
it be incognitoPersistent? I'm looking through extension_api.json and we don't
have much consistency for enum values.

[Bernhard Bauer, 2011-05-24 23:50:00]

On Tue, May 24, 2011 at 14:07,  <mpcomplete@chromium.org> wrote:
> On 2011/05/24 20:26:30, Bernhard Bauer wrote:
>
>
http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
>>
>> File chrome/common/extensions/api/extension_api.json (right):
>
>
>
http://codereview.chromium.org/6990054/diff/1024/chrome/common/extensions/api...
>>
>> chrome/common/extensions/api/extension_api.json:5387: "enum": ["regular"],
>> On 2011/05/24 20:16:50, battre wrote:
>> > On 2011/05/24 20:14:57, Bernhard Bauer wrote:
>> > > What are going to be the future values? "incognito_temporary" and
>> > > "incognito_persistent"? I'm not sure I really like that (but OTOH I
>> > > don't
>> have
>> > a
>> > > better suggestion either).
>> >
>> > My current proposal is "incognito_persistent" and
>> > "incognito_session_only".
>
>> Fine with me. Matt, any objections?
>
> Sounds good. Though I wonder about naming style. This is javascript, so
> should
> it be incognitoPersistent? I'm looking through extension_api.json and we
> don't
> have much consistency for enum values.

It looks like the most common style is underscore_style. There are a
few enums which use CamelCase, but they are pretty recent and might
have slipped through (*cough* levelOfControl).

> http://codereview.chromium.org/6990054/
>

[Matt Perry, 2011-05-25 18:13:49]

LGTM