Removing FakePolicyWatcher and introducing FakeAsyncPolicyLoader.

Removing FakePolicyWatcher and introducing FakeAsyncPolicyLoader.

Removal of FakePolicyWatcher means that PolicyServiceWatcher is now the only
class derived from PolicyWatcher.  A next check-in should merge PolicyWatcher
and PolicyServiceWatcher into a single class.

Testing via FakeAsyncPolicyLoader rather than FakePolicyWatcher means that
tests cover more of PolicyServiceWatcher code.

I chose to introduce FakeAsyncPolicyLoader, rather than going via an already
existing MockConfigurationPolicyProvier, because 1) this way I can cover more
of PolicyServiceWatcher code and 2) this simplifies the test code (by a) keeping
PolicySchema manipulation within the product code and b) not requiring mocking
of IsInitializationComplete and c) being able to directly call into unmodified
CreateFromPolicyLoader method in the product code).

BUG=368321
Committed: https://crrev.com/56dd1e9481b2a7cdb8fd2067046e452bb765ffa4
Cr-Commit-Position: refs/heads/master@{#312994}

[Łukasz Anforowicz, 2015-01-22 00:12:28]

lukasza@chromium.org changed reviewers:
+ mnissler@chromium.org

[Łukasz Anforowicz, 2015-01-22 00:12:29]

Mattias, could you please take a look at this changelist?

This is a relatively simple changelist that prepares for further clean-up of
stuff under src/remoting/host/policy_hack (i.e. I still have a TODO to merge
policy_service_watcher.cc into policy_watcher.cc - the current changelist
prepares me for that).

I wasn't sure if I should ask you or Sergey for the first round of code review
feedback, but I thought that I'll pick you in case you want to comment on me
introducing a FakeAsyncPolicyLoader rather than reusing
MockConfigurationPolicyProvider.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_watcher.h (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher.h:117: friend class PolicyWatcherTest;
friend declaration added to allow the test to call SignalTransientPolicyError
(previously the test was calling this via the removed FakePolicyWatcher).

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_watcher_unittest.cc (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:40: message_loop_proxy_,
policy_loader.Pass());
I am not happy with handing over a scoped_ptr<FakeAsyncPolicyLoader> to product
code, while keeping a pointer to the same FakeAsyncPolicyLoader in the test
code.  OTOH, I don't see how to make it better without adding ref-counting.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:217: class
DictionaryValueEqMatcher
I've added a quick-and-dirty DictionaryValueEqMatcher to have the test tell me
why the 2 dictionary are not equal (the old IsPolicies matcher didn't give any
details).  I hope this is ok:
1. It retains the old behavior (verified by the CHECK at the end of
MatchAndExplain method)
2. While far from perfect (i.e. silly descriptions returned from DescribeTo
method) I think it is worth preserving (especially since it is hidden inside
policy_watcher_unittest.cc / not exposed to rest of Chromium code).

[Mattias Nissler (ping if slow), 2015-01-22 16:05:08]

Looks pretty good to me already, just a couple minor things / suggestions.

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
components/policy/core/common/fake_async_policy_loader.h:8: #include
"base/values.h"
nit: you can forward-declare DictionaryValue instead.

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
components/policy/core/common/fake_async_policy_loader.h:31: void
ClearPolicies();
I'd prefer if you just made an UpdatePolicies(const PolicyBundle& bundle) call
instead ClearPolicies() and AddPolicies(). That's more useful in general and
PolicyBundle is the canonical format to pass policy settings around. Code that
needs to use a DictionaryValue for a good reason to describe policies can still
do so by doing something similar to your current implementation of AddPolicies.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_service_watcher.cc (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.cc:40: }  // anonymous
namespace
In my experience, this comment commonly looks like this:

}  // namespace

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_service_watcher.h (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:14: #include
"components/policy/core/common/schema_registry.h"
A lot of these policy dependencies are probably forward-declarable instead.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:32:
PolicyServiceWatcher(const scoped_refptr<base::SingleThreadTaskRunner>&
#include "base/memory/ref_counted.h"

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:45:
scoped_ptr<policy::PolicyService> owned_policy_service,
#include "base/memory/scoped_ptr.h"

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_watcher.h (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher.h:117: friend class PolicyWatcherTest;
On 2015/01/22 00:12:28, lukasza wrote:
> friend declaration added to allow the test to call SignalTransientPolicyError
> (previously the test was calling this via the removed FakePolicyWatcher).

Acknowledged.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_watcher_unittest.cc (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:40: message_loop_proxy_,
policy_loader.Pass());
On 2015/01/22 00:12:28, lukasza wrote:
> I am not happy with handing over a scoped_ptr<FakeAsyncPolicyLoader> to
product
> code, while keeping a pointer to the same FakeAsyncPolicyLoader in the test
> code.  OTOH, I don't see how to make it better without adding ref-counting.

I think what you have is fair. If you really feel strong about it, I'd suggest
to change PolicyServiceWatcher to not own the policy loader, but document that
the passed pointer must stay valid for the life time of the watcher. It'd then
be up to the embedding code to manage life time.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:217: class
DictionaryValueEqMatcher
On 2015/01/22 00:12:28, lukasza wrote:
> I've added a quick-and-dirty DictionaryValueEqMatcher to have the test tell me
> why the 2 dictionary are not equal (the old IsPolicies matcher didn't give any
> details).  I hope this is ok:
> 1. It retains the old behavior (verified by the CHECK at the end of
> MatchAndExplain method)
> 2. While far from perfect (i.e. silly descriptions returned from DescribeTo
> method) I think it is worth preserving (especially since it is hidden inside
> policy_watcher_unittest.cc / not exposed to rest of Chromium code).

I'm not sure there's justification enough to add all this code just for
debugging. Perhaps it's sufficient to just call Equals() and just JSON-serialize
the two dictionaries and print them if they are different? That's probably good
enough to quickly spot the reason for failure.

[Łukasz Anforowicz, 2015-01-22 18:28:06]

Mattias - I've addressed your code review feedback.  Could you please take
another look?

Thanks,

Lukasz

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
components/policy/core/common/fake_async_policy_loader.h:8: #include
"base/values.h"
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> nit: you can forward-declare DictionaryValue instead.

Done.

https://codereview.chromium.org/858303003/diff/1/components/policy/core/commo...
components/policy/core/common/fake_async_policy_loader.h:31: void
ClearPolicies();
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> I'd prefer if you just made an UpdatePolicies(const PolicyBundle& bundle) call
> instead ClearPolicies() and AddPolicies(). That's more useful in general and
> PolicyBundle is the canonical format to pass policy settings around. Code that
> needs to use a DictionaryValue for a good reason to describe policies can
still
> do so by doing something similar to your current implementation of
AddPolicies.

I removed ClearPolicies and AddPolicies(Dict) and replaced them with
SetPolicies(const PolicyBundle&).  I left PostReloadOnBackgroundThread
untouched.  I hope this is what you meant (not sure if you also wanted to roll
PostReload... into UpdatePolicies;  I think leaving them separate is fine and
more versatile).

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_service_watcher.cc (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.cc:40: }  // anonymous
namespace
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> In my experience, this comment commonly looks like this:
> 
> }  // namespace
> 

Done.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_service_watcher.h (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:14: #include
"components/policy/core/common/schema_registry.h"
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> A lot of these policy dependencies are probably forward-declarable instead.

Done.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:32:
PolicyServiceWatcher(const scoped_refptr<base::SingleThreadTaskRunner>&
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> #include "base/memory/ref_counted.h"

Done.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_service_watcher.h:45:
scoped_ptr<policy::PolicyService> owned_policy_service,
On 2015/01/22 16:05:07, Mattias Nissler wrote:
> #include "base/memory/scoped_ptr.h"

Done.

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
File remoting/host/policy_hack/policy_watcher_unittest.cc (right):

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:40: message_loop_proxy_,
policy_loader.Pass());
On 2015/01/22 16:05:08, Mattias Nissler wrote:
> On 2015/01/22 00:12:28, lukasza wrote:
> > I am not happy with handing over a scoped_ptr<FakeAsyncPolicyLoader> to
> product
> > code, while keeping a pointer to the same FakeAsyncPolicyLoader in the test
> > code.  OTOH, I don't see how to make it better without adding ref-counting.
> 
> I think what you have is fair. If you really feel strong about it, I'd suggest
> to change PolicyServiceWatcher to not own the policy loader, but document that
> the passed pointer must stay valid for the life time of the watcher. It'd then
> be up to the embedding code to manage life time.

Thanks.  I'll leave it as-is.  Moving the ownership of AsyncPolicyLoader would
result in a leaky abstraction/encapsulation for whoever needs to create a
PolicyServiceWatcher (right now it is also kind of leaky but only from
test-code-perspective - for product-code everything is nice looking).

https://codereview.chromium.org/858303003/diff/1/remoting/host/policy_hack/po...
remoting/host/policy_hack/policy_watcher_unittest.cc:217: class
DictionaryValueEqMatcher
On 2015/01/22 16:05:08, Mattias Nissler wrote:
> On 2015/01/22 00:12:28, lukasza wrote:
> > I've added a quick-and-dirty DictionaryValueEqMatcher to have the test tell
me
> > why the 2 dictionary are not equal (the old IsPolicies matcher didn't give
any
> > details).  I hope this is ok:
> > 1. It retains the old behavior (verified by the CHECK at the end of
> > MatchAndExplain method)
> > 2. While far from perfect (i.e. silly descriptions returned from DescribeTo
> > method) I think it is worth preserving (especially since it is hidden inside
> > policy_watcher_unittest.cc / not exposed to rest of Chromium code).
> 
> I'm not sure there's justification enough to add all this code just for
> debugging. Perhaps it's sufficient to just call Equals() and just
JSON-serialize
> the two dictionaries and print them if they are different? That's probably
good
> enough to quickly spot the reason for failure.

Doh... yes - this is much easier.  Thanks.

[Mattias Nissler (ping if slow), 2015-01-23 08:27:40]

LGTM w/nits

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:27:
scoped_refptr<base::SequencedTaskRunner> task_runner);
#include "base/memory/ref_counted.h"

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:47:
DISALLOW_COPY_AND_ASSIGN(FakeAsyncPolicyLoader);
#include "base/macros.h"

https://codereview.chromium.org/858303003/diff/20001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_service_watcher.cc (right):

https://codereview.chromium.org/858303003/diff/20001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.cc:12: #include
"components/policy/core/common/policy_service.h"
already in header

[Łukasz Anforowicz, 2015-01-23 17:49:54]

lukasza@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Łukasz Anforowicz, 2015-01-23 17:49:55]

I've addressed the remaining nits pointed out by Mattias.  Thanks.

Sergey - could you please take a look from src/remoting side?

Thanks,

Lukasz

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:27:
scoped_refptr<base::SequencedTaskRunner> task_runner);
On 2015/01/23 08:27:39, Mattias Nissler wrote:
> #include "base/memory/ref_counted.h"

Done.

https://codereview.chromium.org/858303003/diff/20001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:47:
DISALLOW_COPY_AND_ASSIGN(FakeAsyncPolicyLoader);
On 2015/01/23 08:27:40, Mattias Nissler wrote:
> #include "base/macros.h"

Done.

https://codereview.chromium.org/858303003/diff/20001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_service_watcher.cc (right):

https://codereview.chromium.org/858303003/diff/20001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.cc:12: #include
"components/policy/core/common/policy_service.h"
On 2015/01/23 08:27:40, Mattias Nissler wrote:
> already in header

Done.

[Sergey Ulanov, 2015-01-23 18:29:11]

lgtm

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:11: #include
"base/sequenced_task_runner.h"
nit: Don't need this include in the header. SequencedTaskRunner can be
forward-declared.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_service_watcher.h (right):

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.h:11: #include
"base/single_thread_task_runner.h"
nit: SingleThreadTaskRunner can be forward-declared.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.h:22: namespace policy_hack {
not related to this CL: I think we will want to remove the _hack suffix from
namespace and directory.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_watcher_unittest.cc (right):

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_watcher_unittest.cc:38: // retaining a pointer
to keep control over policy contents.
s/retaining/Retaining/

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_watcher_unittest.cc:39: policy_loader_ =
policy_loader.get();
nit: this code would be shorter without policy_loader variable:

policy_loader_ = new policy::FakeAsyncPolicyLoader(message_loop_proxy_);
policy_watcher_ = PolicyServiceWatcher::CreateFromPolicyLoader(
    message_loop_proxy_, make_scoped_ptr(policy_loader));

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_watcher_unittest.cc:163:
policy::FakeAsyncPolicyLoader* policy_loader_;
add a comment that this is owned by the |policy_watcher_|

[Łukasz Anforowicz, 2015-01-23 19:24:46]

Thanks Sergey.

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:11: #include
"base/sequenced_task_runner.h"
On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> nit: Don't need this include in the header. SequencedTaskRunner can be
> forward-declared.

This is tricky.  With a forward-declaration, all .cc files using this header
will in practice have to include this.  Oh, wait... this error from .cc file
(when just passing the untouched sequenced_task_runner value) is just telling me
that I should be passing scoped_refptr by const-ref, rather than by value.  This
required to also make the change in async_policy_loader.h (otoh, I limited the
change just to the constructor, even though other scoped_refptr-s here could
also be passed by const-ref).

OTOH, I see that I still have to include sequenced_task_runner in
fake_async_policy_loader.cc, because I need the full declaration when calling
PostTask on the runner.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_service_watcher.h (right):

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.h:11: #include
"base/single_thread_task_runner.h"
On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> nit: SingleThreadTaskRunner can be forward-declared.

Done.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_service_watcher.h:22: namespace policy_hack {
On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> not related to this CL: I think we will want to remove the _hack suffix from
> namespace and directory.

I plan to do this in the last changelist :-)  (I have at least 3 more planned -
merge PolicyWatcher and PolicyServiceWatcher + cleanup lots of now unnecessary
stuff from PolicyWatcher + get rid of error handling [was hoping to get this
rubberstamped by the security folks first]).

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
File remoting/host/policy_hack/policy_watcher_unittest.cc (right):

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_watcher_unittest.cc:38: // retaining a pointer
to keep control over policy contents.
On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> s/retaining/Retaining/

Done.

https://codereview.chromium.org/858303003/diff/40001/remoting/host/policy_hac...
remoting/host/policy_hack/policy_watcher_unittest.cc:39: policy_loader_ =
policy_loader.get();
On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> nit: this code would be shorter without policy_loader variable:
> 
> policy_loader_ = new policy::FakeAsyncPolicyLoader(message_loop_proxy_);
> policy_watcher_ = PolicyServiceWatcher::CreateFromPolicyLoader(
>     message_loop_proxy_, make_scoped_ptr(policy_loader));

Good point.  I'll make the change.

[Łukasz Anforowicz, 2015-01-23 19:25:02]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2015-01-23 19:26:57]

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

[commit-bot: I haz the power, 2015-01-23 19:52:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-23 19:52:56]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[Sergey Ulanov, 2015-01-23 22:24:40]

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
File components/policy/core/common/fake_async_policy_loader.h (right):

https://codereview.chromium.org/858303003/diff/40001/components/policy/core/c...
components/policy/core/common/fake_async_policy_loader.h:11: #include
"base/sequenced_task_runner.h"
On 2015/01/23 19:24:45, lukasza wrote:
> On 2015/01/23 18:29:11, Sergey Ulanov wrote:
> > nit: Don't need this include in the header. SequencedTaskRunner can be
> > forward-declared.
> 
> This is tricky.  With a forward-declaration, all .cc files using this header
> will in practice have to include this.  Oh, wait... this error from .cc file
> (when just passing the untouched sequenced_task_runner value) is just telling
me
> that I should be passing scoped_refptr by const-ref, rather than by value. 
This
> required to also make the change in async_policy_loader.h (otoh, I limited the
> change just to the constructor, even though other scoped_refptr-s here could
> also be passed by const-ref).
> 
> OTOH, I see that I still have to include sequenced_task_runner in
> fake_async_policy_loader.cc, because I need the full declaration when calling
> PostTask on the runner.

Passing scoped_refptr<> may be a bit less efficient than passing them by
const-ref, but it's not a big problem. And yes, you will need to include
sequenced_task_runner.h in .cc files that pass the value. It's still better to
have this file included in several .cc files instead of here, in the header.

[Łukasz Anforowicz, 2015-01-24 01:02:23]

Mattias,

I tweaked components/policy.gypi to make sure that
fake_async_policy_loader.cc/.h sources are excluded on Android.  I had to do
this because I see that async_policy_loader.cc/.h are excluded on Android in
components/policy/policy_common.gypi (around line 144).

Interestingly, I don't see async_policy_loader.cc/.h being excluded in
components/policy/core/common/BUILD.gn.  So, I think I don't have to do the same
magic of excluding fake_async_policy_loader.cc/.h in components/policy/BUILD.gn
(where I currently inconditionally add fake_async_policy_loader.cc/.h to
policy_component_test_support target).

Anyway - I thought that you might want a heads-up for this change.

Thanks,

Lukasz

PS. I cannot repro the ASAN failure (tried Release and Debug with GYP_DEFINES
copied from
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...).
:-(

[Łukasz Anforowicz, 2015-01-24 01:19:25]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2015-01-24 01:20:04]

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

[Łukasz Anforowicz, 2015-01-24 01:20:06]

Fixed a memory leak in policy_watcher_unittest.cc

[commit-bot: I haz the power, 2015-01-24 02:09:48]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-24 02:10:38]

Patchset 7 (id:??) landed as
https://crrev.com/56dd1e9481b2a7cdb8fd2067046e452bb765ffa4
Cr-Commit-Position: refs/heads/master@{#312994}