Differentiate TOUCH_UI builds in crash reporting.

Differentiate TOUCH_UI builds in crash reporting.

In TOUCH_UI ChromeOS builds, augment the Linux distro so that crash reports
will have lsb-release="CrOS Touch" instead of just "CrOS".  This mirrors the
change in User-Agent in TOUCH_UI builds as well (see BuildOSCpuInfo() in
webkit/glue/user_agent.cc).

BUG=chromium-os:22255
TEST=Manual


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

[Rick Byers, 2011-11-04 16:13:25]

Hey Ken,
Can you please review this trivial CL?  Let me know if you think there's a
better way to identify our crash reports - happy to use whichever solution you
think is best as long as we can rely on it to be accurate.

Thanks,
   Rick

[kmixter1, 2011-11-07 20:38:35]

LGTM but you should probably have an owner review - I'm not sure all the ways
this value is used.

[Rick Byers, 2011-11-08 19:41:30]

On 2011/11/07 20:38:35, kmixter1 wrote:
> LGTM but you should probably have an owner review - I'm not sure all the ways
> this value is used.

Thanks Ken.  As far as I can see it's used only for breakpad related stuff (but
it does get used by more than just the lsb-release field - my hunch was it was
safe based on the non-ChromeOS values here, but I could make an uglier change
downstream if we thought that was better).

Adding OWNER Brett to confirm this trivial TOUCH_UI specific change is alright.

[Rick Byers, 2011-11-09 19:33:56]

On 2011/11/08 19:41:30, Rick Byers wrote:
> On 2011/11/07 20:38:35, kmixter1 wrote:
> > LGTM but you should probably have an owner review - I'm not sure all the
ways
> > this value is used.
> 
> Thanks Ken.  As far as I can see it's used only for breakpad related stuff
(but
> it does get used by more than just the lsb-release field - my hunch was it was
> safe based on the non-ChromeOS values here, but I could make an uglier change
> downstream if we thought that was better).
> 
> Adding OWNER Brett to confirm this trivial TOUCH_UI specific change is
alright.

Brett is away, Evan can you take a quick look at this please?

[Rick Byers, 2011-11-09 19:59:40]

On 2011/11/09 19:33:56, Rick Byers wrote:
> On 2011/11/08 19:41:30, Rick Byers wrote:
> > On 2011/11/07 20:38:35, kmixter1 wrote:
> > > LGTM but you should probably have an owner review - I'm not sure all the
> ways
> > > this value is used.
> > 
> > Thanks Ken.  As far as I can see it's used only for breakpad related stuff
> (but
> > it does get used by more than just the lsb-release field - my hunch was it
was
> > safe based on the non-ChromeOS values here, but I could make an uglier
change
> > downstream if we thought that was better).
> > 
> > Adding OWNER Brett to confirm this trivial TOUCH_UI specific change is
> alright.
> 
> Brett is away, Evan can you take a quick look at this please?

Aw man, Evan is away too - on sabbatical.  Ok, now I'm just picking names at
random from OWNERS.  Nico, can you please take a look?

[Nico, 2011-11-09 20:09:05]

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc
File base/linux_util.cc (right):

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc#newcode135
base/linux_util.cc:135: #if defined(TOUCH_UI)
Would

#if defined(TOUCH_UI)
#elif defined(OS_CHROMEOS)
#else
#endif

work as well? Then you don't need a nested #if

[Nico, 2011-11-09 20:11:30]

The distro name is also used by sync:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/sync/g...

yfriedman, is this change safe from a sync perspective?

[Rick Byers, 2011-11-09 20:17:55]

Thanks Nico!

> The distro name is also used by sync:
>
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/sync/g...
> 
> yfriedman, is this change safe from a sync perspective?

Yeah, I just noticed that too.  In non-CrOS scenarios this can be a pretty
rich/arbitrary string (eg. "Ubuntu 10.04.1 LTS").  Hopefully the sync code
wouldn't be specifically testing for 'CrOS' to identify ChromeOS for some
reason...

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc
File base/linux_util.cc (right):

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc#newcode135
base/linux_util.cc:135: #if defined(TOUCH_UI)
On 2011/11/09 20:09:05, Nico wrote:
> Would
> 
> #if defined(TOUCH_UI)
> #elif defined(OS_CHROMEOS)
> #else
> #endif
> 
> work as well? Then you don't need a nested #if

That wouldn't really be correct - we can have TOUCH_UI builds that aren't
OS_CHROMEOS (although I'm not concerned with those scenarios here).  If you like
I can do "#if defined(OS_CHROMEOS) && defined(TOUCH_UI)" to avoid the nested if,
but that'll be a little redundant.

[Nico, 2011-11-09 20:20:42]

LGTM if Yaron says that sync looks ok.

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc
File base/linux_util.cc (right):

http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc#newcode135
base/linux_util.cc:135: #if defined(TOUCH_UI)
> That wouldn't really be correct - we can have TOUCH_UI builds that aren't
> OS_CHROMEOS (although I'm not concerned with those scenarios here).  If you
like
> I can do "#if defined(OS_CHROMEOS) && defined(TOUCH_UI)" to avoid the nested
if,
> but that'll be a little redundant.

I like that a bit better, but it's fine either way.

[Rick Byers, 2011-11-09 22:57:46]

On 2011/11/09 20:20:42, Nico wrote:
> LGTM if Yaron says that sync looks ok.
> 
> http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc
> File base/linux_util.cc (right):
> 
> http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc#newcode135
> base/linux_util.cc:135: #if defined(TOUCH_UI)
> > That wouldn't really be correct - we can have TOUCH_UI builds that aren't
> > OS_CHROMEOS (although I'm not concerned with those scenarios here).  If you
> like
> > I can do "#if defined(OS_CHROMEOS) && defined(TOUCH_UI)" to avoid the nested
> if,
> > but that'll be a little redundant.
> 
> I like that a bit better, but it's fine either way.

Changed - I agree it's easier to read.

[Yaron, 2011-11-10 00:40:55]

On 2011/11/09 22:57:46, Rick Byers wrote:
> On 2011/11/09 20:20:42, Nico wrote:
> > LGTM if Yaron says that sync looks ok.
> > 
> > http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc
> > File base/linux_util.cc (right):
> > 
> > http://codereview.chromium.org/8479002/diff/1/base/linux_util.cc#newcode135
> > base/linux_util.cc:135: #if defined(TOUCH_UI)
> > > That wouldn't really be correct - we can have TOUCH_UI builds that aren't
> > > OS_CHROMEOS (although I'm not concerned with those scenarios here).  If
you
> > like
> > > I can do "#if defined(OS_CHROMEOS) && defined(TOUCH_UI)" to avoid the
nested
> > if,
> > > but that'll be a little redundant.
> > 
> > I like that a bit better, but it's fine either way.
> 
> Changed - I agree it's easier to read.

lgtm for sync

[commit-bot: I haz the power, 2011-11-10 14:33:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/8479002/10001

[commit-bot: I haz the power, 2011-11-10 16:04:12]

Change committed as 109436