Read ChromeOS version numbers from file before sandboxing and cache them.

Read ChromeOS version numbers from file before sandboxing and cache them.

BUG=chromiumos-os:11573
TEST=Check that user agent in javascript contains correct CrOS version.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=80438

[dgozman, 2011-03-29 15:32:26]

Chris,

May you have a look?

Thanks,
Dmitry

[Chris Masone, 2011-03-29 15:43:25]

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc
File base/sys_info_chromeos.cc (right):

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc#newco...
base/sys_info_chromeos.cc:19: static const char kLinuxStandardBaseVersionKey[] =
"CHROMEOS_RELEASE_VERSION";
What will happen when this is run on a non-chrome-os platform, as many people do
while testing?  This was the entire purpose of the GOOGLE_CHROME_BUILD stuff, so
that we'd fail over in the case of chromium builds.  Like, will this change
cause failures on the trybots and builders?

[dgozman, 2011-03-30 12:30:17]

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc
File base/sys_info_chromeos.cc (right):

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc#newco...
base/sys_info_chromeos.cc:19: static const char kLinuxStandardBaseVersionKey[] =
"CHROMEOS_RELEASE_VERSION";
On 2011/03/29 15:43:25, Chris Masone wrote:
> What will happen when this is run on a non-chrome-os platform, as many people
do
> while testing?  This was the entire purpose of the GOOGLE_CHROME_BUILD stuff,
so
> that we'd fail over in the case of chromium builds.  Like, will this change
> cause failures on the trybots and builders?

I think, when you run this on a non-chrome-os, you will not get ChromeOS version
anyway (just a linux version from DISTRIB_RELEASE in the best case).
And GOOGLE_CHROME_BUILD stuff fails when you gmerge Chrome to the ChromeOS
image.

Probably, we can try different strings here, but I'm not sure.

Btw, try run succeeded (chromeos, win, linux, mac):
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/23727

[Chris Masone, 2011-03-30 14:38:02]

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc
File base/sys_info_chromeos.cc (right):

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc#newco...
base/sys_info_chromeos.cc:19: static const char kLinuxStandardBaseVersionKey[] =
"CHROMEOS_RELEASE_VERSION";
On 2011/03/30 12:30:17, dgozman wrote:
> On 2011/03/29 15:43:25, Chris Masone wrote:
> > What will happen when this is run on a non-chrome-os platform, as many
people
> do
> > while testing?  This was the entire purpose of the GOOGLE_CHROME_BUILD
stuff,
> so
> > that we'd fail over in the case of chromium builds.  Like, will this change
> > cause failures on the trybots and builders?
> 
> I think, when you run this on a non-chrome-os, you will not get ChromeOS
version
> anyway (just a linux version from DISTRIB_RELEASE in the best case).

I know, but that was deemed to better than returning nothing by my original code
reviewers.

> And GOOGLE_CHROME_BUILD stuff fails when you gmerge Chrome to the ChromeOS
> image.
> 
> Probably, we can try different strings here, but I'm not sure.
> 
> Btw, try run succeeded (chromeos, win, linux, mac):
>
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
> http://build.chromium.org/p/tryserver.chromium/builders/win/builds/23727
> 

Yeah, but is there even a unit test for this code? I seem to recall it being
disabled at some point.

[dgozman, 2011-04-01 15:28:31]

Chris,

Could you please look again?

Thanks,
Dmitry

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc
File base/sys_info_chromeos.cc (right):

http://codereview.chromium.org/6771012/diff/1/base/sys_info_chromeos.cc#newco...
base/sys_info_chromeos.cc:19: static const char kLinuxStandardBaseVersionKey[] =
"CHROMEOS_RELEASE_VERSION";
On 2011/03/30 14:38:02, Chris Masone wrote:
> On 2011/03/30 12:30:17, dgozman wrote:
> > On 2011/03/29 15:43:25, Chris Masone wrote:
> > > What will happen when this is run on a non-chrome-os platform, as many
> people
> > do
> > > while testing?  This was the entire purpose of the GOOGLE_CHROME_BUILD
> stuff,
> > so
> > > that we'd fail over in the case of chromium builds.  Like, will this
change
> > > cause failures on the trybots and builders?
> > 
> > I think, when you run this on a non-chrome-os, you will not get ChromeOS
> version
> > anyway (just a linux version from DISTRIB_RELEASE in the best case).
> 
> I know, but that was deemed to better than returning nothing by my original
code
> reviewers.

I added checking for multiple keys.

> 
> > And GOOGLE_CHROME_BUILD stuff fails when you gmerge Chrome to the ChromeOS
> > image.
> > 
> > Probably, we can try different strings here, but I'm not sure.
> > 
> > Btw, try run succeeded (chromeos, win, linux, mac):
> >
>
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
> > http://build.chromium.org/p/tryserver.chromium/builders/win/builds/23727
> > 
> 
> Yeah, but is there even a unit test for this code? I seem to recall it being
> disabled at some point.

[Chris Masone, 2011-04-01 17:20:28]

looks better...can you do a trybot run?

[dgozman, 2011-04-04 12:30:17]

On 2011/04/01 17:20:28, Chris Masone wrote:
> looks better...can you do a trybot run?

Sure. Sucess.
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/19934

[Chris Masone, 2011-04-04 15:43:55]

LGTM