Make SimpleCache creation threadsafe.

Make SimpleCache creation threadsafe.

It was using a single global worker thread pool in a non-threadsafe
manner.

This imposed the restriction that all instances of SimpleCache must be
initialized on a single thread, or weird things may, in very rare and
unlikely cases, occur.

BUG=442321
Committed: https://crrev.com/f233a2c48951a4f0e246773d61465052ab230ca9
Cr-Commit-Position: refs/heads/master@{#317071}

[mmenke, 2015-01-14 21:47:05]

mmenke@chromium.org changed reviewers:
+ gavinp@chromium.org

[mmenke, 2015-01-14 21:47:06]

mef: FYI

[mmenke, 2015-01-14 23:02:01]

Hrm...looks like I broke something.  Tests failing despite claiming they
pass...I'll look into it when I have time.  :(

[gavinp, 2015-01-15 18:09:59]

On 2015/01/14 23:02:01, mmenke wrote:
> Hrm...looks like I broke something.  Tests failing despite claiming they
> pass...I'll look into it when I have time.  :(

I am surprised you broke anything with this. I'm wondering about initialization
though; isn't the initialization always on the IO thread?

[mmenke, 2015-01-15 18:53:38]

On 2015/01/15 18:09:59, gavinp wrote:
> On 2015/01/14 23:02:01, mmenke wrote:
> > Hrm...looks like I broke something.  Tests failing despite claiming they
> > pass...I'll look into it when I have time.  :(
> 
> I am surprised you broke anything with this. I'm wondering about
initialization
> though; isn't the initialization always on the IO thread?

In Cronet it certainly isn't, since we currently use one thread per request
context.  Could change this, but it means we don't have to have as many globals,
which are kinda odd when dealing with calls from Java...

GpuProcessHost::CreateChannelCache and PnaclTranslationCache do look to be set
up on the IO thread, as are the profile's HttpCaches...  But do we really want
to force random stuff to do stuff on the IO thread, particularly when we can't
actually DCHECK on it?  Just seems like a really bad idea, and isn't really a
fundamental property of the simple cache.

[mmenke, 2015-01-15 18:57:15]

On 2015/01/15 18:53:38, mmenke wrote:
> On 2015/01/15 18:09:59, gavinp wrote:
> > On 2015/01/14 23:02:01, mmenke wrote:
> > > Hrm...looks like I broke something.  Tests failing despite claiming they
> > > pass...I'll look into it when I have time.  :(
> > 
> > I am surprised you broke anything with this. I'm wondering about
> initialization
> > though; isn't the initialization always on the IO thread?
> 
> In Cronet it certainly isn't, since we currently use one thread per request
> context.  Could change this, but it means we don't have to have as many
globals,
> which are kinda odd when dealing with calls from Java...

Erm...global IO thread would be kinda weird in Cronet, rather.

> GpuProcessHost::CreateChannelCache and PnaclTranslationCache do look to be set
> up on the IO thread, as are the profile's HttpCaches...  But do we really want
> to force random stuff to do stuff on the IO thread, particularly when we can't
> actually DCHECK on it?  Just seems like a really bad idea, and isn't really a
> fundamental property of the simple cache.

[mmenke, 2015-02-02 17:21:09]

Gavin:  Finally had time to look at it.  Turns out LazyInstances aren't leaky by
default.  PTAL, thanks!

[mmenke, 2015-02-18 22:17:37]

mmenke@chromium.org changed reviewers:
+ pasko@chromium.org

[mmenke, 2015-02-18 22:17:37]

mmenke@chromium.org changed reviewers:
+ pasko@chromium.org

[mmenke, 2015-02-18 22:17:37]

Oops, forgot about this one.

[mmenke, 2015-02-18 22:17:38]

Oops, forgot about this one.

[mmenke, 2015-02-18 22:17:52]

[+pasko]:  PTAL.

[pasko, 2015-02-19 12:09:13]

I like the LazyInstance approach, seems like the path to less surprises for
readers.

More comments on the bug.

https://codereview.chromium.org/851013003/diff/60001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_backend_impl.cc (right):

https://codereview.chromium.org/851013003/diff/60001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_backend_impl.cc:67:
base::FieldTrialList::FindFullName("SimpleCacheMaxThreads");
I removed this code for the experiment recently, please rebase.

[mmenke, 2015-02-19 16:53:07]

Thanks!

https://codereview.chromium.org/851013003/diff/60001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_backend_impl.cc (right):

https://codereview.chromium.org/851013003/diff/60001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_backend_impl.cc:67:
base::FieldTrialList::FindFullName("SimpleCacheMaxThreads");
On 2015/02/19 12:09:13, pasko wrote:
> I removed this code for the experiment recently, please rebase.

Done.

[pasko, 2015-02-19 17:06:47]

lgtm given bots pass, thank you

[mmenke, 2015-02-19 17:11:53]

On 2015/02/19 17:06:47, pasko wrote:
> lgtm given bots pass, thank you

Thanks!

[mmenke, 2015-02-19 17:11:57]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2015-02-19 17:12:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/851013003/80001

[commit-bot: I haz the power, 2015-02-19 17:43:07]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-19 17:43:30]

Patchset 5 (id:??) landed as
https://crrev.com/f233a2c48951a4f0e246773d61465052ab230ca9
Cr-Commit-Position: refs/heads/master@{#317071}