Policy refresh hits the registry (an I/O operation) on the main thread....

Policy refresh hits the registry (an I/O operation) on the main thread.

BUG=66453
TEST=none


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

[danno, 2010-12-10 22:19:56]

http://codereview.chromium.org/5762002/diff/4001/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_provider_win.cc (right):

http://codereview.chromium.org/5762002/diff/4001/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_provider_win.cc:248:
base::ThreadRestrictions::ScopedAllowIO allow_io;
I don't think this is needed. Provide only gets called on the UI thread early at
startup with the PrefStore is constructed (where you don't need this, right?).
After that, the refresh mechanism already goes to great lengths to make sure
that it gets called on the FILE thread, take a look at PrefValueStore.

[Avi (use Gerrit), 2010-12-10 23:10:25]

Huh. I'm at home now, but what happened was that I'd built a copy on my
Windows box and came back to find it in the debugger, on the main thread,
complaining about hitting the io warning. This was definitely after launch.

And if this is only launch, we still need to deal with it.

Will look further.

On Fri, Dec 10, 2010 at 5:19 PM, <danno@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/5762002/diff/4001/chrome/browser/policy/config...
> File chrome/browser/policy/configuration_policy_provider_win.cc (right):
>
>
>
http://codereview.chromium.org/5762002/diff/4001/chrome/browser/policy/config...
> chrome/browser/policy/configuration_policy_provider_win.cc:248:
>
> base::ThreadRestrictions::ScopedAllowIO allow_io;
> I don't think this is needed. Provide only gets called on the UI thread
> early at startup with the PrefStore is constructed (where you don't need
> this, right?). After that, the refresh mechanism already goes to great
> lengths to make sure that it gets called on the FILE thread, take a look
> at PrefValueStore.
>
>
> http://codereview.chromium.org/5762002/
>

[Avi (use Gerrit), 2010-12-13 22:32:31]

"After that, the refresh mechanism already goes to great lengths to make sure
that it gets called on the FILE thread, take a look at PrefValueStore."

This is not what is actually happening according to my breakpoint. To repro:
drop a breakpoint on ConfigurationPolicyProviderWin::Provide, and let Chrome
sit. It will break on that breakpoint on the main thread. Backtrace in the bug.

This is real. Please approve.

[danno, 2010-12-14 08:06:43]

lgtm