Add the AsyncPolicyProvider and AsyncPolicyLoader

Add the AsyncPolicyProvider and AsyncPolicyLoader, which provide the same functionality as the AsychronousPolicyProvider/Loader and FileBasedPolicyProvider/Loader.

Also has a simpler interface for implementations to override.

BUG=130918
TEST=unit_testsAsyncPolicyProviderTest.*

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

[Joao da Silva, 2012-06-03 14:04:41]

CL 1/5: adding the AsyncPolicyProvider.

The other are 2) update the mac provider 3) update the config dir provider 4)
update the win provider 5) erase the old stuff.

Please review, thanks!

[Mattias Nissler (ping if slow), 2012-06-04 08:48:25]

One high-level comment: I liked the original idea of having the file watching
outside the base class. It seems that we only have it in AsyncPolicyLoader now
because of the klunky FilePathWatcher interface (also see my inline comment on
that)? If you agree, can we please file a bug on cleaning up the watcher
interface and moving the watchers to subclasses then?

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_loader.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:33: // when the AsyncPolicyLoader
is deleted.
Note: net/dns/file_path_watcher_wrapper.h does mostly the same thing (see
http://code.google.com/p/chromium/issues/detail?id=130610 for context). The root
issue here is that FilePathWatcher should be converted to new-style callbacks,
instead of everybody writing their own wrapper.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:102: if (watcher->Watch(path, new
FilePathWatcherDelegate(reload_callback))) {
no need for curlies

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:118: const UpdateCallback&
update_callback) {
fits previous line

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:141:
reload_weak_factory_.GetWeakPtr(),
indentation

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:152: TimeDelta
kSettleInterval(TimeDelta::FromSeconds(kSettleIntervalSeconds));
const

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_loader.h (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:42: // FILE, except for the initial
Load() at startup which is invoked from UI.
say FILE _thread_, otherwise this won't make any sense to people not intimately
experienced with the project. Same from UI (also elsewhere in this file).

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:58: // seconds in the past.
explain why.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:66: void WatchPath(const FilePath&
path);
seems like this should be protected.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:97: // is invalidated whenever a new
reload is scheduled, while
use base/timer.h instead?

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:32:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
Rather use base::threading::NonThreadSafe? If you choose to do so, you should
also update the code below to call MessageLoop::current()->PostTask instead of
the BrowserThread variant.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:59: // made before this call. So
if a test has modified the policy settings and
Seems like this doesn't only apply to tests.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:65: // Reload() from triggered a
notification too early.
s/triggered/triggering/

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:83: void
AsyncPolicyProvider::PostRefreshingReload() {
The name of this function is not very descriptive, and in fact, I couldn't tell
from the name what it's supposed to do. I suggest "ReloadAfterRefreshSync".

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:86: pending_refreshes_--;
Ah, counters. In this case, it's probably not an issue, but if I were to write
this code, I would use a separate weak pointer factory (or CancellableCallback)
to just clear all earlier posted refreshes in subsequent RefreshPolicies()
invocations.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.h (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.h:18: // A policy provider that
loads its policies asynchronously in the FILE thread.
in or on? on is more familiar to me.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.h:25: AsyncPolicyLoader* loader);
should declare a scoped_ptr<AsyncPolicyLoader> argument then.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider_unittest.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:12: #include
"chrome/browser/policy/configuration_policy_provider.h"
already included in async_policy_provider.h and no explicit other usage here
AFAICS

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:66: public:
you can use protected here if you like to.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:113:
.WillRepeatedly(Return(base::Time()));
you declared this expectation in line 99 already.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:157: // Doesn't refresh
before going through FILE.
thread

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:188:
.WillRepeatedly(Return(&refreshed_bundle));
Configuring both of these as repeatedly seems ambiguous to me. Do you actually
mean WillOnce?

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:217:
base::Value::CreateStringValue("updated"));
It seems like a helper to initialize a policy bundle would be quite useful in a
number of place in the file.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:238:
.WillRepeatedly(InvokeWithoutArgs(&loop_, &MessageLoop::Quit));
Doing actual file watches in tests is very much discouraged, as the various
platform-level watcher implementations have a long history of flaking. Thus, if
you don't want people complaining about your test being flaky, you need to mock
out the watcher here.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:240: ASSERT_EQ((int)
content.size(),
use C++-style casts (also below).

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:279: // FilePathWatcher
on the mac only notices directory changes, and not file
s/mac/Mac/ according to our more picky Mac fanboys friends :)

[Joao da Silva, 2012-06-04 14:05:06]

Thanks for the extensive review! Please have another look.

WatchPath() and the watchers have been removed.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_loader.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:33: // when the AsyncPolicyLoader
is deleted.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Note: net/dns/file_path_watcher_wrapper.h does mostly the same thing (see
> http://code.google.com/p/chromium/issues/detail?id=130610 for context). The
root
> issue here is that FilePathWatcher should be converted to new-style callbacks,
> instead of everybody writing their own wrapper.

Moved this to FilePathWatcher in another CL.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:102: if (watcher->Watch(path, new
FilePathWatcherDelegate(reload_callback))) {
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> no need for curlies

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:118: const UpdateCallback&
update_callback) {
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> fits previous line

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:141:
reload_weak_factory_.GetWeakPtr(),
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> indentation

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.cc:152: TimeDelta
kSettleInterval(TimeDelta::FromSeconds(kSettleIntervalSeconds));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> const

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_loader.h (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:42: // FILE, except for the initial
Load() at startup which is invoked from UI.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> say FILE _thread_, otherwise this won't make any sense to people not
intimately
> experienced with the project. Same from UI (also elsewhere in this file).

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:58: // seconds in the past.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> explain why.

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:66: void WatchPath(const FilePath&
path);
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> seems like this should be protected.

Obsolete; this has been removed, as suggested elsewhere.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_loader.h:97: // is invalidated whenever a new
reload is scheduled, while
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> use base/timer.h instead?

The weak factory for the watchers is now gone. I tried using a Timer for the
periodic refresh task, but ScheduleNextReload() is also used to post reloads
"soon" after the settle period; resetting the Timer to trigger once with a
different delay was a bit clunky, so I've left the weak_factory for reloads
only.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:32:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Rather use base::threading::NonThreadSafe? If you choose to do so, you should
> also update the code below to call MessageLoop::current()->PostTask instead of
> the BrowserThread variant.

Done. This also required passing the MessageLoop of the provider to the loader.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:59: // made before this call. So
if a test has modified the policy settings and
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Seems like this doesn't only apply to tests.

s/test/caller

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:65: // Reload() from triggered a
notification too early.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> s/triggered/triggering/

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:83: void
AsyncPolicyProvider::PostRefreshingReload() {
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> The name of this function is not very descriptive, and in fact, I couldn't
tell
> from the name what it's supposed to do. I suggest "ReloadAfterRefreshSync".

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:86: pending_refreshes_--;
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Ah, counters. In this case, it's probably not an issue, but if I were to write
> this code, I would use a separate weak pointer factory (or
CancellableCallback)
> to just clear all earlier posted refreshes in subsequent RefreshPolicies()
> invocations.

Redone with a CancellableCallback as suggested offline.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.h (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.h:18: // A policy provider that
loads its policies asynchronously in the FILE thread.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> in or on? on is more familiar to me.

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.h:25: AsyncPolicyLoader* loader);
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> should declare a scoped_ptr<AsyncPolicyLoader> argument then.

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider_unittest.cc (right):

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:12: #include
"chrome/browser/policy/configuration_policy_provider.h"
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> already included in async_policy_provider.h and no explicit other usage here
> AFAICS

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:66: public:
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> you can use protected here if you like to.

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:113:
.WillRepeatedly(Return(base::Time()));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> you declared this expectation in line 99 already.

It is cleared in line 110.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:157: // Doesn't refresh
before going through FILE.
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> thread

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:188:
.WillRepeatedly(Return(&refreshed_bundle));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Configuring both of these as repeatedly seems ambiguous to me. Do you actually
> mean WillOnce?

Yes, done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:217:
base::Value::CreateStringValue("updated"));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> It seems like a helper to initialize a policy bundle would be quite useful in
a
> number of place in the file.

Done.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:238:
.WillRepeatedly(InvokeWithoutArgs(&loop_, &MessageLoop::Quit));
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> Doing actual file watches in tests is very much discouraged, as the various
> platform-level watcher implementations have a long history of flaking. Thus,
if
> you don't want people complaining about your test being flaky, you need to
mock
> out the watcher here.

Removed this tests, after the offline discussion. WatchPath is not part of the
AsyncPolicyLoader anymore either.

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:240: ASSERT_EQ((int)
content.size(),
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> use C++-style casts (also below).

Obsolete

http://codereview.chromium.org/10448118/diff/11003/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:279: // FilePathWatcher
on the mac only notices directory changes, and not file
On 2012/06/04 08:48:26, Mattias Nissler wrote:
> s/mac/Mac/ according to our more picky Mac fanboys friends :)

iObsolete

[Mattias Nissler (ping if slow), 2012-06-04 15:25:39]

Couple more comments. However, I'm rather tired right now, so I'd like to get
another chance to take a final look later today or first thing tomorrow.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
File chrome/browser/policy/async_policy_loader.h (right):

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_loader.h:70: void
Init(scoped_refptr<base::MessageLoopProxy> provider_loop,
Do we really need the loop? It seems that AsyncPolicyProvider could supply a
callback that can handle running on FILE and does the thread switching behind
the scenes. That way, all thread switching would be in AsyncPolicyProvider,
which would be more consistent and tidy.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_loader.h:83: // |provider_loop_| on updates..
Excess period

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
File chrome/browser/policy/async_policy_provider.h (right):

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_provider.h:26: // Takes ownership of
|loader|.
no need for the ownership comment.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_provider.h:55: 
excess blank line

[Joao da Silva, 2012-06-04 17:28:12]

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
File chrome/browser/policy/async_policy_loader.h (right):

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_loader.h:70: void
Init(scoped_refptr<base::MessageLoopProxy> provider_loop,
On 2012/06/04 15:25:39, Mattias Nissler wrote:
> Do we really need the loop? It seems that AsyncPolicyProvider could supply a
> callback that can handle running on FILE and does the thread switching behind
> the scenes. That way, all thread switching would be in AsyncPolicyProvider,
> which would be more consistent and tidy.

Done.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_loader.h:83: // |provider_loop_| on updates..
On 2012/06/04 15:25:39, Mattias Nissler wrote:
> Excess period

Done.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
File chrome/browser/policy/async_policy_provider.h (right):

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_provider.h:26: // Takes ownership of
|loader|.
On 2012/06/04 15:25:39, Mattias Nissler wrote:
> no need for the ownership comment.

Done.

http://codereview.chromium.org/10448118/diff/5010/chrome/browser/policy/async...
chrome/browser/policy/async_policy_provider.h:55: 
On 2012/06/04 15:25:39, Mattias Nissler wrote:
> excess blank line

Done.

[Mattias Nissler (ping if slow), 2012-06-04 18:09:02]

LGTM with nits.

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.cc (right):

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:125: 
remove extra blank line

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider_unittest.cc (right):

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:192:
loop_.RunAllPending();
nit: If there's a newline at 180, there should be one here as well. Actually,
you seem to have no newlines within an EXPECT_CALL - trigger - verify block in
the tests above. And below and otherwise, you have the comment before the entire
block, while here it's inlined. That's all super-picky nits, feel free to ignore
:)

[Joao da Silva, 2012-06-05 08:19:21]

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider.cc (right):

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider.cc:125: 
On 2012/06/04 18:09:03, Mattias Nissler wrote:
> remove extra blank line

Done.

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
File chrome/browser/policy/async_policy_provider_unittest.cc (right):

http://codereview.chromium.org/10448118/diff/25002/chrome/browser/policy/asyn...
chrome/browser/policy/async_policy_provider_unittest.cc:192:
loop_.RunAllPending();
On 2012/06/04 18:09:03, Mattias Nissler wrote:
> nit: If there's a newline at 180, there should be one here as well. Actually,
> you seem to have no newlines within an EXPECT_CALL - trigger - verify block in
> the tests above. And below and otherwise, you have the comment before the
entire
> block, while here it's inlined. That's all super-picky nits, feel free to
ignore
> :)

I've looked at the comments and tried to come up with a consistent pattern, but
they make sense to me where they are :-) The newlines separate "setup" blocks
and "verify" blocks. The comments either comment what a block does, or they're
inline because they comment what happens after a RunAllPending; in those cases,
having them at the start of the block isn't quite accurate, because the
condition is only verified after spinning the loop (for example, the comment on
the next line is only true after spinning the loop, but the whole block is
verifying that RefreshPolicies does its thing).

[commit-bot: I haz the power, 2012-06-05 11:38:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/10448118/33001

[commit-bot: I haz the power, 2012-06-05 12:49:52]

Change committed as 140518