Add synchronization around access when using ReloadLocaleResources

I tried to add a pak override in the command line but for some reason it is stripped before it gets to the browser initialization.  I think that adding synchronization to this particular race is the best way to solve the flaky test.

Previous description:

Add DCHECK for possible flake place - will fail with ASSERT rather than crash.

Crashes in comment 15
http://code.google.com/p/chromium/issues/detail?id=95425#c15
Seem to indicate that data has not been loaded/reloaded.  There are two possible places that could happen and logging in non-test code + ASSERTION in test code can at least get us a bit further in determining where to look.

BUG=95425
TEST=out/Debug/browser_tests --gtest_filter=WebUIBidiCheckerBrowserTestRTL.TestSettingsClearBrowserDataPage

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

[tony, 2012-06-05 23:40:20]

LGTM

You might want to say 'why' you're making this change in the description.  E.g.,
Hoping to track down why blah blah test is flaky.

[Sheridan Rawlins, 2012-06-05 23:49:29]

On 2012/06/05 23:40:20, tony wrote:
> You might want to say 'why' you're making this change in the description. 
E.g.,
> Hoping to track down why blah blah test is flaky.

Done.  Also added logging at least in the other case that might cause this -
empty locale path.

[tony, 2012-06-06 00:05:46]

On 2012/06/05 23:49:29, Sheridan Rawlins wrote:
> On 2012/06/05 23:40:20, tony wrote:
> > You might want to say 'why' you're making this change in the description. 
> E.g.,
> > Hoping to track down why blah blah test is flaky.
> 
> Done.  Also added logging at least in the other case that might cause this -
> empty locale path.

Makes sense.  LGTM

[Sheridan Rawlins, 2012-06-07 04:03:28]

Tony, I had to go for the synchronization solution.  Armed with this fix (rather
than just DLOG/DCHECK), I'd like a new L.G. and will watch to see if current
flakes are quelled.

[tony, 2012-06-07 16:41:16]

On 2012/06/07 04:03:28, Sheridan Rawlins wrote:
> Tony, I had to go for the synchronization solution.  Armed with this fix
(rather
> than just DLOG/DCHECK), I'd like a new L.G. and will watch to see if current
> flakes are quelled.

I don't think we should be adding more complex locking logic to handle a race
that should never happen in shipping Chrome.

It seems like the test is poorly written if it's triggering the race.  I would
rather we try to fix the test.

[Sheridan Rawlins, 2012-06-12 18:26:23]

I explored both adding a flag and there are several places (at least 3) where
the flags get stripped when launching a child process or entering a sandbox,
etc.  Then I explored hooking into the test early enough to set the locale
override before the initial loading, but that point is at the test runner phase
before the suite is even instantiated, etc and would be challenging to do in a
reasonably generic fashion.

Then, today I had the insight - what if the test just navigated to about:blank
before doing its reload of the locale?  Then there would be no race of some page
(NTP4, home page, e.g.) loading resources without thread safety.

Can we run with this one?

[tony, 2012-06-12 18:33:06]

LGTM, thanks!

http://codereview.chromium.org/10543015/diff/14001/ui/base/resource/resource_...
File ui/base/resource/resource_bundle.cc (right):

http://codereview.chromium.org/10543015/diff/14001/ui/base/resource/resource_...
ui/base/resource/resource_bundle.cc:307: // TODO(tony) Firm up locking for or
constraints of calling
Nit: add a : after the close paren.

[commit-bot: I haz the power, 2012-06-12 19:41:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scr@chromium.org/10543015/8006

[commit-bot: I haz the power, 2012-06-12 19:42:01]

Presubmit check for 10543015-8006 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/webui

Presubmit checks took 9.2s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Evan Stade, 2012-06-12 19:57:34]

lgtm

[commit-bot: I haz the power, 2012-06-12 20:10:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scr@chromium.org/10543015/8006

[commit-bot: I haz the power, 2012-06-12 21:31:01]

Change committed as 141739