Supervised user blacklist: Don't download the blacklist file on every startup

Supervised user blacklist: Don't download the blacklist file on every startup

BUG=410824
Committed: https://crrev.com/2fd18739de7bae8c59fcc5b58fad0138d6657886
Cr-Commit-Position: refs/heads/master@{#325462}

[Marc Treib, 2015-04-16 13:50:33]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org

[Bernhard Bauer, 2015-04-16 13:58:06]

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/supervised_user_service.cc:625:
BrowserThread::IO,
I don't think you want to do this on the IO thread (which, sadly, is not the
thread to do disk I/O on: it's only for non-blocking I/O, which in practice
means IPC...). You probably want BrowserThread::GetBlockingPool() (with
base::PostTaskAndReplyWithResult()).

[Marc Treib, 2015-04-16 14:04:16]

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/supervised_user_service.cc:625:
BrowserThread::IO,
On 2015/04/16 13:58:06, Bernhard Bauer wrote:
> I don't think you want to do this on the IO thread (which, sadly, is not the
> thread to do disk I/O on: it's only for non-blocking I/O, which in practice
> means IPC...). You probably want BrowserThread::GetBlockingPool() (with
> base::PostTaskAndReplyWithResult()).

Argh, yeah, I ran into this before (the IO thread not being allowed to do IO ;)
)
I meant to use BrowserThread::FILE. Is GetBlockingPool preferable to that?

[Bernhard Bauer, 2015-04-16 14:09:16]

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/supervised_user_service.cc:625:
BrowserThread::IO,
On 2015/04/16 14:04:16, Marc Treib wrote:
> On 2015/04/16 13:58:06, Bernhard Bauer wrote:
> > I don't think you want to do this on the IO thread (which, sadly, is not the
> > thread to do disk I/O on: it's only for non-blocking I/O, which in practice
> > means IPC...). You probably want BrowserThread::GetBlockingPool() (with
> > base::PostTaskAndReplyWithResult()).
> 
> Argh, yeah, I ran into this before (the IO thread not being allowed to do IO
;)
> )
> I meant to use BrowserThread::FILE. Is GetBlockingPool preferable to that?

Yes, because it will use more than one thread, so there is less risk of
contention. It also doesn't block shutdown by default (I think).

[Marc Treib, 2015-04-16 14:40:49]

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1086393002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/supervised_user_service.cc:625:
BrowserThread::IO,
On 2015/04/16 14:09:16, Bernhard Bauer wrote:
> On 2015/04/16 14:04:16, Marc Treib wrote:
> > On 2015/04/16 13:58:06, Bernhard Bauer wrote:
> > > I don't think you want to do this on the IO thread (which, sadly, is not
the
> > > thread to do disk I/O on: it's only for non-blocking I/O, which in
practice
> > > means IPC...). You probably want BrowserThread::GetBlockingPool() (with
> > > base::PostTaskAndReplyWithResult()).
> > 
> > Argh, yeah, I ran into this before (the IO thread not being allowed to do IO
> ;)
> > )
> > I meant to use BrowserThread::FILE. Is GetBlockingPool preferable to that?
> 
> Yes, because it will use more than one thread, so there is less risk of
> contention. It also doesn't block shutdown by default (I think).

Got it, thanks! Done.

[Bernhard Bauer, 2015-04-16 15:11:15]

lgtm

[Marc Treib, 2015-04-16 15:12:18]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2015-04-16 15:12:28]

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

[commit-bot: I haz the power, 2015-04-16 17:19:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-16 17:21:06]

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