Removed Labs section from Chrome OS settings page.

Removed Labs section from Chrome OS settings page.

BUG=chromium-os:9035
TEST=Go to chrome://settings and observe that there's no Labs section.

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

[arv (Not doing code reviews), 2010-11-15 20:55:20]

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
File chrome/browser/resources/options.html (left):

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
chrome/browser/resources/options.html:195: 
I don't understand why you are removing these?

[whywhat, 2010-11-15 21:03:29]

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
File chrome/browser/resources/options.html (left):

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
chrome/browser/resources/options.html:195: 
On 2010/11/15 20:55:20, arv wrote:
> I don't understand why you are removing these?

I'm removing Labs section under chrome://settings. Though when I go there, I
don't see anything Hangul, Chewing or Pinyin related, only three features I'm
moving to about:flags which pages are not added there. Guess these are labs we
want to remove completely for now until I'm missing something. Zel?

[zel, 2010-11-15 22:03:14]

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
File chrome/browser/resources/options.html (left):

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
chrome/browser/resources/options.html:201: OptionsPage.register(labsOptions);
only 198-201 should be removed here, the rest of these subpages are still used

[whywhat, 2010-11-15 22:41:50]

Thanks for reviewing. I'm still looking for a way to bind the flags with the
features so enabling/disabling actually works.

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
File chrome/browser/resources/options.html (left):

http://codereview.chromium.org/5025001/diff/2002/chrome/browser/resources/opt...
chrome/browser/resources/options.html:201: OptionsPage.register(labsOptions);
On 2010/11/15 22:03:14, zel wrote:
> only 198-201 should be removed here, the rest of these subpages are still used


Ok, I guess my assumption that registerSubPage means all these pages are inside
the labsPage is wrong. Will return these pages back :)

[whywhat, 2010-11-16 17:22:47]

PTAL. It works as discussed with Nico.

[Nico, 2010-11-16 17:32:06]

LG!

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:269: result->insert(kVerticalTabsExperimentName);
do you want to remove the booleans from |prefs| after migrating them?

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:295: MigrateChromeOSLabsPrefs(prefs, result);
I'd do this at the spot marked XXX below instead.

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:437: std::set<std::string> enabled_experiments;
XXX

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
chrome/common/chrome_switches.cc:1236: const char kEnableAdvancedFileSystem[]   
  = "enable-advanced-fs";
i believe the flags in this file are supposed to be in alphabetical order. also
in the .h file.

[zel, 2010-11-16 17:38:16]

LGTM

[whywhat, 2010-11-16 17:55:14]

Thanks!

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:269: result->insert(kVerticalTabsExperimentName);
On 2010/11/16 17:32:07, Nico wrote:
> do you want to remove the booleans from |prefs| after migrating them?

Can I remove them? I only found how I can set them to false which should be
enough for now.

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:295: MigrateChromeOSLabsPrefs(prefs, result);
On 2010/11/16 17:32:07, Nico wrote:
> I'd do this at the spot marked XXX below instead.

Done.

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:437: std::set<std::string> enabled_experiments;
On 2010/11/16 17:32:07, Nico wrote:
> XXX

Done.

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
chrome/common/chrome_switches.cc:1236: const char kEnableAdvancedFileSystem[]   
  = "enable-advanced-fs";
On 2010/11/16 17:32:07, Nico wrote:
> i believe the flags in this file are supposed to be in alphabetical order.
also
> in the .h file.
I think it's too late for that, especially in .cc file. It's hard to sort these
lines ignoring the comments and making the comments follow their constants. I
think it's not worth it.

[Nico, 2010-11-16 18:01:30]

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:269: result->insert(kVerticalTabsExperimentName);
On 2010/11/16 17:55:15, whywhat wrote:
> On 2010/11/16 17:32:07, Nico wrote:
> > do you want to remove the booleans from |prefs| after migrating them?
> 
> Can I remove them? I only found how I can set them to false which should be
> enough for now.

PrefService::ClearPref

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/common/chrome_switche...
chrome/common/chrome_switches.cc:1236: const char kEnableAdvancedFileSystem[]   
  = "enable-advanced-fs";
On 2010/11/16 17:55:15, whywhat wrote:
> On 2010/11/16 17:32:07, Nico wrote:
> > i believe the flags in this file are supposed to be in alphabetical order.
> also
> > in the .h file.
> I think it's too late for that, especially in .cc file. It's hard to sort
these
> lines ignoring the comments and making the comments follow their constants. I
> think it's not worth it.

Nevermind, I confused this file with another file :-/

[whywhat, 2010-11-16 18:05:03]

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:269: result->insert(kVerticalTabsExperimentName);
On 2010/11/16 18:01:31, Nico wrote:
> On 2010/11/16 17:55:15, whywhat wrote:
> > On 2010/11/16 17:32:07, Nico wrote:
> > > do you want to remove the booleans from |prefs| after migrating them?
> > 
> > Can I remove them? I only found how I can set them to false which should be
> > enough for now.
> 
> PrefService::ClearPref

This sets the value to default one. If it ever becomes true, we'll turn it on on
the next login even if user turns it off via about:flags.

[Nico, 2010-11-16 18:06:41]

On Tue, Nov 16, 2010 at 7:05 PM,  <avayvod@chromium.org> wrote:
>
> http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.cc
> File chrome/browser/about_flags.cc (right):
>
>
http://codereview.chromium.org/5025001/diff/9001/chrome/browser/about_flags.c...
> chrome/browser/about_flags.cc:269:
> result->insert(kVerticalTabsExperimentName);
> On 2010/11/16 18:01:31, Nico wrote:
>>
>> On 2010/11/16 17:55:15, whywhat wrote:
>> > On 2010/11/16 17:32:07, Nico wrote:
>> > > do you want to remove the booleans from |prefs| after migrating
>
> them?
>>
>> >
>> > Can I remove them? I only found how I can set them to false which
>
> should be
>>
>> > enough for now.
>
>> PrefService::ClearPref
>
> This sets the value to default one. If it ever becomes true, we'll turn
> it on on the next login even if user turns it off via about:flags.

Yes, but since this CL removes that pref, this is unlikely to happen.

[xiyuan, 2010-11-16 18:09:45]

LGTM