Fix for issue 51693, which was a regression from integrating the open source...

Fix for issue 51693, which was a regression from integrating the open source
version of RLZ into Chromium.  Chromium should never send RLZ pings to Google,
only Google Chrome should do so.

Removing dependency from rlz chrome extenion api to RLZTracker since it is
not required.

BUG=51693
TEST=Install Chromium on a clean machine and make sure no pings are sent to
http://client1.google.com/tools/pso/ping?...


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

[Glenn Wilson, 2010-08-11 16:01:15]

Adding Carlos, who knows all the RLZ references better than I...

http://codereview.chromium.org/3146007/diff/19002/22004
File chrome/browser/rlz/rlz.cc (right):

http://codereview.chromium.org/3146007/diff/19002/22004#newcode169
chrome/browser/rlz/rlz.cc:169: if (brand.empty() ||
GoogleUpdateSettings::IsOrganic(brand))
Should probably be

if (!GoogleUpdateSettings::GetBrand(&brand) ||
    brand.empty() || GoogleUpdateSettings::IsOrganic(brand))

in the case that the regkey read fails.  Carlos, do you agree?

[Roger Tawa OOO till Jul 10th, 2010-08-11 18:33:44]

Thanks Glenn.  Your suggestion applied.  Will wait for Carlos.

http://codereview.chromium.org/3146007/diff/19002/22004
File chrome/browser/rlz/rlz.cc (right):

http://codereview.chromium.org/3146007/diff/19002/22004#newcode169
chrome/browser/rlz/rlz.cc:169: if (brand.empty() ||
GoogleUpdateSettings::IsOrganic(brand))
On 2010/08/11 16:01:15, Glenn Wilson wrote:
> Should probably be
> 
> if (!GoogleUpdateSettings::GetBrand(&brand) ||
>     brand.empty() || GoogleUpdateSettings::IsOrganic(brand))
> 
> in the case that the regkey read fails.  Carlos, do you agree?

Done.

[cpu_(ooo_6.6-7.5), 2010-08-12 00:57:35]

http://codereview.chromium.org/3146007/diff/19003/13002
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/3146007/diff/19003/13002#newcode1261
chrome/browser/browser_main.cc:1261: win_util::ScopedCOMInitializer
com_initializer;
the COM initializer... I think that was there before I put rlz in. This might
break things.

http://codereview.chromium.org/3146007/diff/19003/13005
File chrome/browser/rlz/rlz.cc (right):

http://codereview.chromium.org/3146007/diff/19003/13005#newcode169
chrome/browser/rlz/rlz.cc:169: GoogleUpdateSettings::IsOrganic(brand))
wouldn't it be better the empty check inside IsOrganic() ?

http://codereview.chromium.org/3146007/diff/19003/13008
File chrome/browser/search_engines/template_url_unittest.cc (right):

http://codereview.chromium.org/3146007/diff/19003/13008#newcode336
chrome/browser/search_engines/template_url_unittest.cc:336: TemplateURLRef
ref("http://bar/?{google:RLZ}{searchTerms}", 1, 2);
how about that {google:RZ} thing. I think Peter Kasting might need to give the
ok here.

[Roger Tawa OOO till Jul 10th, 2010-08-12 03:07:18]

Thanks Carlos.  I will ping PK about this.

http://codereview.chromium.org/3146007/diff/19003/13002
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/3146007/diff/19003/13002#newcode1261
chrome/browser/browser_main.cc:1261: win_util::ScopedCOMInitializer
com_initializer;
On 2010/08/12 00:57:35, cpu wrote:
> the COM initializer... I think that was there before I put rlz in. This might
> break things.

Done.

http://codereview.chromium.org/3146007/diff/19003/13005
File chrome/browser/rlz/rlz.cc (right):

http://codereview.chromium.org/3146007/diff/19003/13005#newcode169
chrome/browser/rlz/rlz.cc:169: GoogleUpdateSettings::IsOrganic(brand))
On 2010/08/12 00:57:35, cpu wrote:
> wouldn't it be better the empty check inside IsOrganic() ?

No.  An empty string is not organic.
We just don't want to send pings with an empty string.
I have confirmed with the PSO guys that this is OK.  In fact, they already strip
out empty brand codes.

[Peter Kasting, 2010-08-12 17:19:37]

LGTM, just nits, mostly about not adding comments that don't enhance the code
any.

http://codereview.chromium.org/3146007/diff/28003/22008
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22008#newcode1265
chrome/browser/browser_main.cc:1265: // RLZ applies only to Google Chrome and
not Chromium.
Nit: Put the #if above the comment block too, and drop the comment you added
(since the code is self-explanatory).

http://codereview.chromium.org/3146007/diff/28003/22009
File chrome/browser/browser_shutdown.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22009#newcode142
chrome/browser/browser_shutdown.cc:142: // RLZ applies only to Google Chrome and
not Chromium.
Nit: Drop this comment (self-explanatory)

http://codereview.chromium.org/3146007/diff/28003/22012
File chrome/browser/search_engines/template_url.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22012#newcode343
chrome/browser/search_engines/template_url.cc:343: // RLZ applies only to Google
Chrome and not Chromium.
Nit: Drop this comment (self-explanatory)

http://codereview.chromium.org/3146007/diff/28003/22014
File chrome/browser/search_engines/template_url_unittest.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22014#newcode1
chrome/browser/search_engines/template_url_unittest.cc:1: // Copyright (c)
2006-2008 The Chromium Authors. All rights reserved.
Nit: 2010

[Roger Tawa OOO till Jul 10th, 2010-08-12 18:55:51]

Thanks Peter, Carlos, Glenn for your comments.  I have confirmed with PSO about
the blank brand code, so all looks good.

All code changes are uploaded, I will do a final pass with the try bots before
committing.  Try bot errors with previous patches have not been related to my
change, so I'll how it goes.

http://codereview.chromium.org/3146007/diff/28003/22008
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22008#newcode1265
chrome/browser/browser_main.cc:1265: // RLZ applies only to Google Chrome and
not Chromium.
On 2010/08/12 17:19:37, Peter Kasting wrote:
> Nit: Put the #if above the comment block too, and drop the comment you added
> (since the code is self-explanatory).

Done.

http://codereview.chromium.org/3146007/diff/28003/22009
File chrome/browser/browser_shutdown.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22009#newcode142
chrome/browser/browser_shutdown.cc:142: // RLZ applies only to Google Chrome and
not Chromium.
On 2010/08/12 17:19:37, Peter Kasting wrote:
> Nit: Drop this comment (self-explanatory)

Done.

http://codereview.chromium.org/3146007/diff/28003/22012
File chrome/browser/search_engines/template_url.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22012#newcode343
chrome/browser/search_engines/template_url.cc:343: // RLZ applies only to Google
Chrome and not Chromium.
On 2010/08/12 17:19:37, Peter Kasting wrote:
> Nit: Drop this comment (self-explanatory)

Done.

http://codereview.chromium.org/3146007/diff/28003/22014
File chrome/browser/search_engines/template_url_unittest.cc (right):

http://codereview.chromium.org/3146007/diff/28003/22014#newcode1
chrome/browser/search_engines/template_url_unittest.cc:1: // Copyright (c)
2006-2008 The Chromium Authors. All rights reserved.
On 2010/08/12 17:19:37, Peter Kasting wrote:
> Nit: 2010

Done.

[Glenn Wilson, 2010-08-12 19:01:10]

lgtm