[cros] RLZ tracking can be turned off via a flag file.

[cros] RLZ tracking can be turned off via a flag file.

Also loads brand code from file for previous users.


BUG=157348
TBR=pkasting@chromium.org


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173472

[Ivan Korotkov, 2012-12-11 10:50:37]

PTAL:
Nikita: whole CL, mainly chromeos/
Peter: google_util/ and search_engines/
Roger: rlz/

Roger, is it normal if occasionally (normally once -- after out-of-box wizard),
RLZTracker::Init() is called asynchronously after Profile creation (so that for
example RLZTracker::RecordProductEvent can be called before)? I've modified
RLZTracker to make sure that all calls use the same sequenced runner, including
DelayedInit.

[Ivan Korotkov, 2012-12-11 10:53:19]

https://codereview.chromium.org/11506006/diff/7001/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/11506006/diff/7001/build/common.gypi#oldcode1452
build/common.gypi:1452: ['branding=="Chrome" and (OS=="win" or OS=="mac" or
chromeos==1)', {
For trybots

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/search_engi...
File chrome/browser/search_engines/template_url_service_test_util.cc (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/search_engi...
chrome/browser/search_engines/template_url_service_test_util.cc:184:
google_util::chromeos::SetTemporaryBrand("");
This is needed for unit_tests that call VerifyLoad() which may result in
{google:rlz} being substituted

[Nikita (slow), 2012-12-11 13:02:54]

chromeos/login lgtm

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:524: // Initialize RLZ
tracking.
nit: Remove this comment.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:525:
LoginUtils::Get()->InitRlzDelayed();
Pass profile() here.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:14: #include
"base/debug/trace_event.h"
I'm assuming tracing will be removed.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:670: InitRlzDelayed();
makes sense to pass user_profile here and remove GetDefaultProfile() call from
InitRlzDelayed().

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.h (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.h:126: // Initialize RLZ tracking.
nit: just "Initialize RLZ"

[Ivan Korotkov, 2012-12-11 15:44:52]

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:524: // Initialize RLZ
tracking.
On 2012/12/11 13:02:54, Nikita Kostylev wrote:
> nit: Remove this comment.

Done.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/ch...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:525:
LoginUtils::Get()->InitRlzDelayed();
On 2012/12/11 13:02:54, Nikita Kostylev wrote:
> Pass profile() here.

Done.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:14: #include
"base/debug/trace_event.h"
On 2012/12/11 13:02:54, Nikita Kostylev wrote:
> I'm assuming tracing will be removed.

Done. Takes 0.17 msec on Alex to stat() a file in $HOME

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:670: InitRlzDelayed();
On 2012/12/11 13:02:54, Nikita Kostylev wrote:
> makes sense to pass user_profile here and remove GetDefaultProfile() call from
> InitRlzDelayed().

Done.

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.h (right):

https://codereview.chromium.org/11506006/diff/7001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.h:126: // Initialize RLZ tracking.
On 2012/12/11 13:02:54, Nikita Kostylev wrote:
> nit: just "Initialize RLZ"

Done.

[Roger Tawa OOO till Jul 10th, 2012-12-11 15:47:30]

rlz lgtm.  Thanks for cleaning up the code.

[Peter Kasting, 2012-12-11 20:21:49]

I don't understand why you've added SetTemporaryBrand().  Why not just make
SetBrand() public instead, and call that?

There also seems to be a lot of needless complexity and some other badness here.
 For example, you do a ScopedAllowIO in InitRlzDelayed().  You have a comment
saying "this should be fast", but that's not the point.  Doing IO on the UI
thread is banned even when it "should be" fast because the filesystem may in
fact _not_ be fast at the moment for various reasons.  It seems like all the
complexity here should be pushed into google_util_chromeos.cc instead and the
API changed to be much less generic.  Then the implementation can basically be:

* Check for RLZ pref.  If not found, post task to worker pool.
* In worker pool, stat the flag file.  If not found, read the RLZ file.  Return
the results of all this
* Back on the UI thread, set the relevant pref and call
InitRlzFromProfileDelayed().

Note that this basically nukes everything in login_utils.cc, gets rid of your
ScopedAllowIO, and in general should be simpler and easier to understand.

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/google/goog...
File chrome/browser/google/google_util_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/google/goog...
chrome/browser/google/google_util_chromeos.cc:60: if (!(g_temporary_brand ==
NULL))
Nit: Why not just use !=?

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/search_engi...
File chrome/browser/search_engines/template_url_service_test_util.cc (right):

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/search_engi...
chrome/browser/search_engines/template_url_service_test_util.cc:184:
google_util::chromeos::SetTemporaryBrand("");
Nit: std::string()

[Ivan Korotkov, 2012-12-12 13:05:56]

PTAL

SetTemporaryBrand (I've renamed it to SetBrandForCurrentSession to reduce
confusion) sets brand code for the current run only (not persisted to Local
State).

The whole RLZ flow work as follows:
(*) brand code is cached in Local State from a file in OEM customization package
(*) there is a per-user flag file in $HOME, which is created or removed by crosh
(*) if this file is present at session start, brand code for this session is set
to an empty string so that RLZTracker doesn't send financial pings etc.
(*) otherwise, brand code from Local State is used and RLZTracker works as
expected.

My bad about trying to sneak in another ScopedAllowIO. Removed it -- as
discussed with Roger, it's OK to always call RLZTracker::Init() asynchronously.

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/google/goog...
File chrome/browser/google/google_util_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/google/goog...
chrome/browser/google/google_util_chromeos.cc:60: if (!(g_temporary_brand ==
NULL))
On 2012/12/11 20:21:49, Peter Kasting wrote:
> Nit: Why not just use !=?

LazyInstance has no operator!=. This looks like the only way to check that
LazyInstance hasn't been initialized. I can add it in a separate CL if you think
that would be useful.

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/search_engi...
File chrome/browser/search_engines/template_url_service_test_util.cc (right):

https://codereview.chromium.org/11506006/diff/5009/chrome/browser/search_engi...
chrome/browser/search_engines/template_url_service_test_util.cc:184:
google_util::chromeos::SetTemporaryBrand("");
On 2012/12/11 20:21:49, Peter Kasting wrote:
> Nit: std::string()

Done.

[Ivan Korotkov, 2012-12-14 15:04:26]

Time moves on:
snow falls, branch point nears.
Please, take a look!

[Roger Tawa OOO till Jul 10th, 2012-12-14 15:27:07]

Looks fine Ivan.  One question below.

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/login_utils_browsertest.cc (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/login_utils_browsertest.cc:537: // RLZ value for
homepage access point should have been initialized.
I'm not sure I understand this comment.  What are you trying to test?  That the
RLZ string caching works?

[Ivan Korotkov, 2012-12-14 15:32:44]

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/login_utils_browsertest.cc (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/login_utils_browsertest.cc:537: // RLZ value for
homepage access point should have been initialized.
On 2012/12/14 15:27:07, Roger Tawa wrote:
> I'm not sure I understand this comment.  What are you trying to test?  That
the
> RLZ string caching works?

More likely that Init() has been called (otherwise this call would return
|false| as rlz value isn't in cache yet). I haven't come up with a better way to
test it with the given RLZTracker API.

[Roger Tawa OOO till Jul 10th, 2012-12-14 16:17:19]

I see.  Looks fine as is.  You could also call RLZTracker::set_tracker() with a
mock RLZTrack, and put an expectation that ScheduleDelayedInit() is called.

[Nikita (slow), 2012-12-14 21:39:20]

On 2012/12/14 15:04:26, Ivan Korotkov wrote:
> Time moves on:
> snow falls, branch point nears.
> Please, take a look!

It seems that Peter is ooo this week.

[Ivan Korotkov, 2012-12-14 21:41:36]

Ilya, can you please take a look at google_util?

[Ilya Sherman, 2012-12-15 00:57:16]

https://codereview.chromium.org/11506006/diff/30001/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/11506006/diff/30001/build/common.gypi#oldcode...
build/common.gypi:1462: ['branding=="Chrome" and (OS=="win" or OS=="mac" or
chromeos==1)', {
This change doesn't look appropriate -- RLZ still shouldn't be enabled for
non-branded builds, right?

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
File chrome/browser/google/google_util_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
chrome/browser/google/google_util_chromeos.cc:60: if (!(g_temporary_brand ==
NULL))
nit: This is usually written as "if (g_temporary_brand)", or at least as "if
(g_temporary_brand != NULL)"

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
File chrome/browser/google/google_util_chromeos.h (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
chrome/browser/google/google_util_chromeos.h:21: void
SetBrandForCurrentSession(const std::string& brand);
It looks like you only ever use this method to set an empty brand code.  If I'm
understanding that correctly, it would be better to narrow the API to only
support clearing out the brand code, rather than being able to set an arbitrary
one (which I found quite surprising upon first encounter.)

[Ivan Korotkov, 2012-12-17 09:05:01]

https://codereview.chromium.org/11506006/diff/30001/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/11506006/diff/30001/build/common.gypi#oldcode...
build/common.gypi:1462: ['branding=="Chrome" and (OS=="win" or OS=="mac" or
chromeos==1)', {
On 2012/12/15 00:57:16, Ilya Sherman wrote:
> This change doesn't look appropriate -- RLZ still shouldn't be enabled for
> non-branded builds, right?

This is for trybots, they are useless otherwise.

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
File chrome/browser/google/google_util_chromeos.cc (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
chrome/browser/google/google_util_chromeos.cc:60: if (!(g_temporary_brand ==
NULL))
On 2012/12/15 00:57:16, Ilya Sherman wrote:
> nit: This is usually written as "if (g_temporary_brand)", or at least as "if
> (g_temporary_brand != NULL)"

Like I said above, LazyInstance only has operator== as means for checking if
initialized or not. LazyInstance is gone now, anyway.

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
File chrome/browser/google/google_util_chromeos.h (right):

https://codereview.chromium.org/11506006/diff/30001/chrome/browser/google/goo...
chrome/browser/google/google_util_chromeos.h:21: void
SetBrandForCurrentSession(const std::string& brand);
On 2012/12/15 00:57:16, Ilya Sherman wrote:
> It looks like you only ever use this method to set an empty brand code.  If
I'm
> understanding that correctly, it would be better to narrow the API to only
> support clearing out the brand code, rather than being able to set an
arbitrary
> one (which I found quite surprising upon first encounter.)

Makes sense. I wanted to be as generic as possible but it's unlikely there'll
ever be another use for this function. Done.

[Ivan Korotkov, 2012-12-17 12:50:56]

All of the google_util-related comments have been fulfilled, TBRing Peter since
there's not much time left before the branch.

[commit-bot: I haz the power, 2012-12-17 12:51:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11506006/39023

[commit-bot: I haz the power, 2012-12-17 13:41:06]

Retried try job too often on win_rel for step(s) content_browsertests

[commit-bot: I haz the power, 2012-12-17 13:56:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11506006/47022

[commit-bot: I haz the power, 2012-12-17 15:58:28]

Change committed as 173472

[Peter Kasting, 2012-12-17 20:14:03]

LGTM, but the phrase "financial ping" is problematic, both because it's unclear
and because I don't think we want to imply anything in the code about what in
particular RLZ might be used for on the server side.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:502:
RLZTracker::EnableZeroDelayForTesting();
Must we call this here?  Can we instead enable in individual tests that care, as
you've already done elsewhere in this CL?

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:384: // Initializes RLZ. If
|disabled| is true, financial pings are turned off.
Nit: What is a "financial ping"?  Do you just mean to say "RLZ is disabled"? 
What's the difference between non-initialized RLZ state and
initialized-but-disabled state?

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:695:
base::WorkerPool::GetTaskRunner(false /* task_is_slow */),
Nit: Don't document argument names like this in general, people can look at the
declaration if they need to know.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:705: // Empty brand code turns
financial pings off.
Nit: Again, this phrase that has no context or explanation.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/mock_login_utils.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/mock_login_utils.h:52:
MOCK_METHOD1(InitRlzDelayed, void(Profile*));
Nit: If possible please declare mock methods in the same order as in the parent
class declaration.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/test_login_utils.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/test_login_utils.h:62: virtual void
InitRlzDelayed(Profile* user_profile) OVERRIDE;
Nit: Also try to match the parent class declaration order here.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
File chrome/browser/rlz/rlz.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
chrome/browser/rlz/rlz.cc:251: int delay,
This should be a TimeDelta, as should |min_delay_|.  Using ints is unclear and
error-prone.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
File chrome/browser/rlz/rlz.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
chrome/browser/rlz/rlz.h:204: // Minimum delay before sending financial ping
after initialization.
Nit: Another instance of "financial ping"

[Ivan Korotkov, 2012-12-18 12:25:58]

Thanks for looking, I'll address comments in a follow-up CL.

[Ivan Korotkov, 2012-12-20 09:20:42]

Uploaded https://codereview.chromium.org/11645040/ with fixes

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:502:
RLZTracker::EnableZeroDelayForTesting();
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Must we call this here?  Can we instead enable in individual tests that care,
as
> you've already done elsewhere in this CL?

I'd like to leave it here (like WizardController::SetZeroDelays in
PostProfileInit) because otherwise any browser test that uses real LoginUtils
and calls GetBlockingPool()->FlushForTesting() will be slowed down by 20 seconds
without an obvious reason.
login_utils_browsertest doesn't go through this path (it's not a real
InProcessBrowserTest, despite it's name), so a separate call is needed there.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:384: // Initializes RLZ. If
|disabled| is true, financial pings are turned off.
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: What is a "financial ping"?  Do you just mean to say "RLZ is disabled"? 
> What's the difference between non-initialized RLZ state and
> initialized-but-disabled state?

Financial pings are pings with recorded product events sent to google servers.
RLZTracker in initialized-but-disabled state behaves like in organic Chrome(ium)
builds: it doesn't send anything but still records these events.

From RLZTracker comments:
// If the chrome brand is organic (no partners) then the pings don't occur.

Let's rephrase it to 'RLZ pings are disabled' because financial ping is too
specific indeed.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:695:
base::WorkerPool::GetTaskRunner(false /* task_is_slow */),
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: Don't document argument names like this in general, people can look at
the
> declaration if they need to know.

Done.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/login_utils.cc:705: // Empty brand code turns
financial pings off.
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: Again, this phrase that has no context or explanation.

Rephrased.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/mock_login_utils.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/mock_login_utils.h:52:
MOCK_METHOD1(InitRlzDelayed, void(Profile*));
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: If possible please declare mock methods in the same order as in the
parent
> class declaration.

Done.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/login/test_login_utils.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/login/test_login_utils.h:62: virtual void
InitRlzDelayed(Profile* user_profile) OVERRIDE;
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: Also try to match the parent class declaration order here.

Done.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
File chrome/browser/rlz/rlz.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
chrome/browser/rlz/rlz.cc:251: int delay,
On 2012/12/17 20:14:03, Peter Kasting wrote:
> This should be a TimeDelta, as should |min_delay_|.  Using ints is unclear and
> error-prone.

Done.

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
File chrome/browser/rlz/rlz.h (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/rlz...
chrome/browser/rlz/rlz.h:204: // Minimum delay before sending financial ping
after initialization.
On 2012/12/17 20:14:03, Peter Kasting wrote:
> Nit: Another instance of "financial ping"

Well, here it's OK since these are RLZTracker guts and there are already
functions like SendFinancialPing

[Peter Kasting, 2012-12-20 19:32:00]

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:502:
RLZTracker::EnableZeroDelayForTesting();
On 2012/12/20 09:20:42, Ivan Korotkov wrote:
> On 2012/12/17 20:14:03, Peter Kasting wrote:
> > Must we call this here?  Can we instead enable in individual tests that
care,
> as
> > you've already done elsewhere in this CL?
> 
> I'd like to leave it here (like WizardController::SetZeroDelays in
> PostProfileInit) because otherwise any browser test that uses real LoginUtils
> and calls GetBlockingPool()->FlushForTesting() will be slowed down by 20
seconds
> without an obvious reason.

Then can we make one of the following happen instead:
* In testing mode, the RLZ tracker _always_ runs in zero-delay mode (i.e. we do
this check in the RLZ code itself).
* In testing mode, the RLZ tracker doesn't do anything at all unless you
explicitly enable it, or perhaps DCHECKs or something.

Either one of these is better than having some somewhat random code added to the
startup functions; startup is so hairy we should go to great lengths to avoid
making it do anything further.

[Ivan Korotkov, 2012-12-21 09:59:48]

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://chromiumcodereview.appspot.com/11506006/diff/47022/chrome/browser/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:502:
RLZTracker::EnableZeroDelayForTesting();
On 2012/12/20 19:32:00, Peter Kasting wrote:
> On 2012/12/20 09:20:42, Ivan Korotkov wrote:
> > On 2012/12/17 20:14:03, Peter Kasting wrote:
> > > Must we call this here?  Can we instead enable in individual tests that
> care,
> > as
> > > you've already done elsewhere in this CL?
> > 
> > I'd like to leave it here (like WizardController::SetZeroDelays in
> > PostProfileInit) because otherwise any browser test that uses real
LoginUtils
> > and calls GetBlockingPool()->FlushForTesting() will be slowed down by 20
> seconds
> > without an obvious reason.
> 
> Then can we make one of the following happen instead:
> * In testing mode, the RLZ tracker _always_ runs in zero-delay mode (i.e. we
do
> this check in the RLZ code itself).
> * In testing mode, the RLZ tracker doesn't do anything at all unless you
> explicitly enable it, or perhaps DCHECKs or something.
> 
> Either one of these is better than having some somewhat random code added to
the
> startup functions; startup is so hairy we should go to great lengths to avoid
> making it do anything further.

Ok, let's do the 1st option.