Create a list of policy changes before notifying observers

Notify policy change observers via a task

This prevents unexpected reentrancy in OnUpdatePolicy() in the case that an observer changes policy (leading to a recursive call to OnUpdatePolicy()).

This avoids a crash that happens if we change the SyncDisabled policy, since
this signs out the user and modifies policy while we are still iterating through
the PolicyBundles in MergeAndTriggerUpdates().

BUG=155486
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175797

[Andrew T Wilson (Slow), 2012-12-21 17:47:16]

PTAL

[Joao da Silva, 2012-12-22 21:42:00]

I'd prefer posting MergeAndTriggerUpdates from its current call sites (or adding
a helper PostMergeAndTriggerUpdates call), mainly for the following corner case:

Imagine the first update sets "foo"=1. We have two observers, ObserverA and
ObserverB. ObserverA receives the notification first, and sets "foo"=2. But now
this 2nd update will have its notifications delivered before the notifications
from the 1st update, and ObserverB will see first "foo"=2 and then "foo"=1 and
will believe "1" is the current value, which is not correct.

I'd *really* like all cases to be correct to avoid surprises in the future :-)
And as a bonus, posting is less code. WDYT?

Posting may break tests that expect policy updates to be synchronously
propagated to pref observers; a call to content::RunAllPendingInMessageLoop()
should fix those.

I'm only concerned with places that have a similar expectation in browser code.
I can only think of the sync policy update for managed users on ChromeOS after
sign-in; that ultimately depends on the call to OnPolicyServiceInitialized,
fired from CheckInitializationComplete, so that should be alright with the async
MergeAndTriggerUpdates.

Other options to break the cycle:

1. Keep the ScopedVector of queued updates as a member field, and each call to
MergeAndTriggerUpdates first queues new updates at the back and then pumps
everything from the front until it's empty. A reentrant call will queue some
more at the back but will pump from the front, and the original call will quit
after seeing an empty queue. I think this is rather messy :-)

2. Flag at the PolicyServiceImpl class whenever updates are being issued; if
policy updates come during this time, they are set in a temporary PolicyMap (so
that multiple updates are also merged into the same PolicyMap). After the
notifications are done, if there was a policy update then start the whole thing
again. This has the extra benefit of coalescing all updates during notifications
into a single update.  Again, I think this is messy.

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
File chrome/browser/policy/policy_service_impl.cc (right):

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl.cc:175: new
PolicyChangeInfo(it_old->first, *it_new->second, kEmpty));
The 2nd arg should be *it_old->second

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
File chrome/browser/policy/policy_service_impl_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl_unittest.cc:71: class
ChangePolicyObserver : public PolicyService::Observer {
Suggestion: the MockPolicyServiceObserver class above could be reused, and the
action on OnPolicyUpdated() could be set with an EXPECT_CALL and a custom
ACTION_P:

ACTION_P(UpdateChromePolicy, provider, policy) {
  provider->UpdateChromePolicy(*policy);
}

TEST(...) {
  MockPolicyServiceObserver observer;
  PolicyMap new_policy;
  // setup ...
  EXPECT_CALL(observer, OnPolicyUpdated(_, _, _,
_).WillOnce(UpdateChromePolicy(&provider0_, &new_policy));
  ...
}

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl_unittest.cc:79: const PolicyMap&
current) {
OVERRIDE

[Andrew T Wilson (Slow), 2013-01-04 15:54:48]

PTAL. I've changed this to issue tasks for the various policy notifications
rather than creating a vector of pending policy notifications. The code is
simpler, but I had to touch more tests since now we require a MessageLoop to
notify observers.

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
File chrome/browser/policy/policy_service_impl.cc (right):

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl.cc:175: new
PolicyChangeInfo(it_old->first, *it_new->second, kEmpty));
On 2012/12/22 21:42:00, Joao da Silva wrote:
> The 2nd arg should be *it_old->second

Good catch :)

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
File chrome/browser/policy/policy_service_impl_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl_unittest.cc:71: class
ChangePolicyObserver : public PolicyService::Observer {
On 2012/12/22 21:42:00, Joao da Silva wrote:
> Suggestion: the MockPolicyServiceObserver class above could be reused, and the
> action on OnPolicyUpdated() could be set with an EXPECT_CALL and a custom
> ACTION_P:
> 
> ACTION_P(UpdateChromePolicy, provider, policy) {
>   provider->UpdateChromePolicy(*policy);
> }
> 
> TEST(...) {
>   MockPolicyServiceObserver observer;
>   PolicyMap new_policy;
>   // setup ...
>   EXPECT_CALL(observer, OnPolicyUpdated(_, _, _,
> _).WillOnce(UpdateChromePolicy(&provider0_, &new_policy));
>   ...
> }

Apparently we're trying to get rid of ACTION_P:
https://code.google.com/p/chromium/issues/detail?id=165595

So I'm going to use my current Fake approach.

https://codereview.chromium.org/11667006/diff/4001/chrome/browser/policy/poli...
chrome/browser/policy/policy_service_impl_unittest.cc:79: const PolicyMap&
current) {
On 2012/12/22 21:42:00, Joao da Silva wrote:
> OVERRIDE

Done.

[Joao da Silva, 2013-01-07 09:23:51]

LGTM after fixing DisableScreenshotsFeedback and the nits.

Re DisableScreenshotsFeedback:

I think SetScreenshotPolicy() in policy_browsertest.cc needs a
content::RunAllPendingInMessageLoop() before returning.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/con...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/con...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:48:
ConfigurationPolicyPrefStoreTest() : loop_(MessageLoop::TYPE_UI) {
I think the default loop type works for this test, does it have to be TYPE_UI?
(same for other tests)

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_prefs_browsertest.cc:399: base::RunLoop loop;
#include "base/run_loop.h"

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.cc:109: scoped_ptr<PolicyChangeInfo>
changes(
#include "base/memory/scoped_ptr.h"

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.cc:133:
PolicyServiceImpl::PolicyChangeInfo::PolicyChangeInfo(
Place the definitions for PolicyChangeInfo before the definitions for
PolicyServiceImpl.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl.h (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.h:63: 
nit: single newline

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl_unittest.cc:379: }
RunUntilIdle()?

[Andrew T Wilson (Slow), 2013-01-07 14:11:32]

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/con...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/con...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:48:
ConfigurationPolicyPrefStoreTest() : loop_(MessageLoop::TYPE_UI) {
On 2013/01/07 09:23:52, Joao da Silva wrote:
> I think the default loop type works for this test, does it have to be TYPE_UI?
> (same for other tests)

Done.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_prefs_browsertest.cc:399: base::RunLoop loop;
On 2013/01/07 09:23:52, Joao da Silva wrote:
> #include "base/run_loop.h"

Done.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.cc:109: scoped_ptr<PolicyChangeInfo>
changes(
On 2013/01/07 09:23:52, Joao da Silva wrote:
> #include "base/memory/scoped_ptr.h"

Done.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.cc:133:
PolicyServiceImpl::PolicyChangeInfo::PolicyChangeInfo(
On 2013/01/07 09:23:52, Joao da Silva wrote:
> Place the definitions for PolicyChangeInfo before the definitions for
> PolicyServiceImpl.

Done.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl.h (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl.h:63: 
On 2013/01/07 09:23:52, Joao da Silva wrote:
> nit: single newline

Done.

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
File chrome/browser/policy/policy_service_impl_unittest.cc (right):

https://codereview.chromium.org/11667006/diff/12007/chrome/browser/policy/pol...
chrome/browser/policy/policy_service_impl_unittest.cc:379: }
On 2013/01/07 09:23:52, Joao da Silva wrote:
> RunUntilIdle()?

Done.

[commit-bot: I haz the power, 2013-01-07 14:11:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/31002

[commit-bot: I haz the power, 2013-01-07 14:27:45]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-07 14:33:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/31002

[commit-bot: I haz the power, 2013-01-07 14:41:29]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-07 17:10:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/31002

[commit-bot: I haz the power, 2013-01-07 19:04:54]

Retried try job too often on win_aura for step(s) interactive_ui_tests

[commit-bot: I haz the power, 2013-01-08 07:59:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/31002

[commit-bot: I haz the power, 2013-01-08 12:02:11]

Retried try job too often on win_rel for step(s) browser_tests

[commit-bot: I haz the power, 2013-01-08 13:01:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/31002

[commit-bot: I haz the power, 2013-01-08 14:44:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/38005

[commit-bot: I haz the power, 2013-01-08 17:37:07]

Retried try job too often on linux_chromeos for step(s) browser_tests

[commit-bot: I haz the power, 2013-01-09 12:12:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/38005

[commit-bot: I haz the power, 2013-01-09 12:12:47]

Failed to apply patch for chrome/browser/policy/policy_browsertest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/policy/policy_browsertest.cc
  Hunk #2 succeeded at 436 (offset -2 lines).
  Hunk #3 succeeded at 536 (offset -3 lines).
  Hunk #4 succeeded at 599 (offset -3 lines).
  Hunk #5 succeeded at 610 (offset -3 lines).
  Hunk #6 succeeded at 646 (offset -3 lines).
  Hunk #7 succeeded at 696 (offset -3 lines).
  Hunk #8 succeeded at 722 (offset -3 lines).
  Hunk #9 succeeded at 762 (offset -3 lines).
  Hunk #10 FAILED at 826.
  Hunk #11 succeeded at 895 (offset 2 lines).
  Hunk #12 succeeded at 903 (offset 2 lines).
  Hunk #13 succeeded at 916 (offset 2 lines).
  Hunk #14 succeeded at 951 (offset 2 lines).
  Hunk #15 succeeded at 977 (offset 2 lines).
  Hunk #16 succeeded at 989 (offset 2 lines).
  Hunk #17 succeeded at 1017 (offset 2 lines).
  Hunk #18 succeeded at 1026 (offset 2 lines).
  Hunk #19 succeeded at 1069 (offset 2 lines).
  Hunk #20 succeeded at 1089 (offset 2 lines).
  Hunk #21 succeeded at 1123 (offset 2 lines).
  Hunk #22 succeeded at 1140 (offset 2 lines).
  Hunk #23 succeeded at 1158 (offset 2 lines).
  Hunk #24 succeeded at 1182 (offset 2 lines).
  Hunk #25 succeeded at 1217 (offset 2 lines).
  Hunk #26 succeeded at 1238 (offset 2 lines).
  Hunk #27 succeeded at 1273 (offset 2 lines).
  Hunk #28 succeeded at 1296 (offset 2 lines).
  Hunk #29 succeeded at 1304 (offset 2 lines).
  Hunk #30 succeeded at 1321 (offset 2 lines).
  Hunk #31 succeeded at 1338 (offset 2 lines).
  Hunk #32 succeeded at 1348 (offset 2 lines).
  Hunk #33 succeeded at 1360 (offset 2 lines).
  Hunk #34 succeeded at 1382 (offset 2 lines).
  Hunk #35 succeeded at 1411 (offset 3 lines).
  Hunk #36 succeeded at 1449 (offset 3 lines).
  Hunk #37 succeeded at 1462 (offset 3 lines).
  Hunk #38 succeeded at 1520 (offset 3 lines).
  Hunk #39 succeeded at 1530 (offset 3 lines).
  1 out of 39 hunks FAILED -- saving rejects to file
chrome/browser/policy/policy_browsertest.cc.rej

Patch:       chrome/browser/policy/policy_browsertest.cc
Index: chrome/browser/policy/policy_browsertest.cc
diff --git a/chrome/browser/policy/policy_browsertest.cc
b/chrome/browser/policy/policy_browsertest.cc
index
1bec8bdf6e0ce54027b54274faba3ecf957dbcfc..6ea6ebab1dee6e0fc8b58c698d988ad249f0e375
100644
--- a/chrome/browser/policy/policy_browsertest.cc
+++ b/chrome/browser/policy/policy_browsertest.cc
@@ -14,6 +14,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"
 #include "base/string16.h"
 #include "base/stringprintf.h"
 #include "base/test/test_file_util.h"
@@ -437,7 +438,7 @@ class PolicyTest : public InProcessBrowserTest {
     PolicyMap policies;
     policies.Set(key::kDisableScreenshots, POLICY_LEVEL_MANDATORY,
                  POLICY_SCOPE_USER, base::Value::CreateBooleanValue(!enabled));
-    provider_.UpdateChromePolicy(policies);
+    UpdateProviderPolicy(policies);
   }
 
   void TestScreenshotFeedback(bool enabled) {
@@ -538,6 +539,13 @@ class PolicyTest : public InProcessBrowserTest {
     observer.Wait();
   }
 
+  void UpdateProviderPolicy(const PolicyMap& policy) {
+    provider_.UpdateChromePolicy(policy);
+    DCHECK(MessageLoop::current());
+    base::RunLoop loop;
+    loop.RunUntilIdle();
+  }
+
   MockConfigurationPolicyProvider provider_;
 };
 
@@ -594,7 +602,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, BookmarkBarEnabled) {
   PolicyMap policies;
   policies.Set(key::kBookmarkBarEnabled, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_TRUE(prefs->IsManagedPreference(prefs::kShowBookmarkBar));
   EXPECT_TRUE(prefs->GetBoolean(prefs::kShowBookmarkBar));
   EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
@@ -605,14 +613,14 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, BookmarkBarEnabled) {
 
   policies.Set(key::kBookmarkBarEnabled, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_TRUE(prefs->IsManagedPreference(prefs::kShowBookmarkBar));
   EXPECT_FALSE(prefs->GetBoolean(prefs::kShowBookmarkBar));
   // The bookmark bar is hidden in the NTP when disabled by policy.
   EXPECT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
 
   policies.Clear();
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_FALSE(prefs->IsManagedPreference(prefs::kShowBookmarkBar));
   EXPECT_FALSE(prefs->GetBoolean(prefs::kShowBookmarkBar));
   // The bookmark bar is shown detached in the NTP, when disabled by prefs
only.
@@ -641,7 +649,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, PRE_ClearSiteDataOnExit)
{
   PolicyMap policies;
   policies.Set(key::kClearSiteDataOnExit, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
 }
 
 IN_PROC_BROWSER_TEST_F(PolicyTest, ClearSiteDataOnExit) {
@@ -691,7 +699,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DefaultSearchProvider) {
   policies.Set(key::kDefaultSearchProviderSearchTermsReplacementKey,
                POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                base::Value::CreateStringValue(kSearchTermsReplacementKey));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   default_search = service->GetDefaultSearchProvider();
   ASSERT_TRUE(default_search);
   EXPECT_EQ(kKeyword, default_search->keyword());
@@ -717,7 +725,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DefaultSearchProvider) {
   policies.Set(key::kDefaultSearchProviderEnabled, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false));
   EXPECT_TRUE(service->GetDefaultSearchProvider());
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_FALSE(service->GetDefaultSearchProvider());
   ui_test_utils::SendToOmniboxAndSubmit(location_bar, "should not work");
   // This means that submitting won't trigger any action.
@@ -757,7 +765,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ForceSafeSearch) {
   PolicyMap policies;
   policies.Set(key::kForceSafeSearch, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
 
   EXPECT_TRUE(prefs->IsManagedPreference(prefs::kForceSafeSearch));
   EXPECT_TRUE(prefs->GetBoolean(prefs::kForceSafeSearch));
@@ -818,7 +826,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
   alternate_urls->AppendString(kAlternateURL1);
   policies.Set(key::kDefaultSearchProviderAlternateURLs,
POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, alternate_urls);
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   default_search = service->GetDefaultSearchProvider();
   ASSERT_TRUE(default_search);
   EXPECT_EQ(kKeyword, default_search->keyword());
@@ -885,7 +893,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, Disable3DAPIs) {
   PolicyMap policies;
   policies.Set(key::kDisable3DAPIs, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   // Crash and reload the tab to get a new renderer.
   content::CrashTab(contents);
   EXPECT_TRUE(chrome::ExecuteCommand(browser(), IDC_RELOAD));
@@ -893,7 +901,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, Disable3DAPIs) {
   // Enable with a policy.
   policies.Set(key::kDisable3DAPIs, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   content::CrashTab(contents);
   EXPECT_TRUE(chrome::ExecuteCommand(browser(), IDC_RELOAD));
   EXPECT_TRUE(IsWebGLEnabled(contents));
@@ -906,14 +914,14 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DisableSpdy) {
   PolicyMap policies;
   policies.Set(key::kDisableSpdy, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   content::RunAllPendingInMessageLoop();
   EXPECT_FALSE(net::HttpStreamFactory::spdy_enabled());
   // Verify that it can be force-enabled too.
   browser()->profile()->GetPrefs()->SetBoolean(prefs::kDisableSpdy, true);
   policies.Set(key::kDisableSpdy, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false));
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   content::RunAllPendingInMessageLoop();
   EXPECT_TRUE(net::HttpStreamFactory::spdy_enabled());
 }
@@ -941,7 +949,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DisabledPlugins) {
   PolicyMap policies;
   policies.Set(key::kDisabledPlugins, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, disabled_plugins.DeepCopy());
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_FALSE(plugin_prefs->IsPluginEnabled(*flash));
   // The user shouldn't be able to enable it.
   EXPECT_FALSE(SetPluginEnabled(plugin_prefs, flash, true));
@@ -967,7 +975,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest,
DisabledPluginsExceptions) {
   PolicyMap policies;
   policies.Set(key::kDisabledPlugins, POLICY_LEVEL_MANDATORY,
                POLICY_SCOPE_USER, disabled_plugins.DeepCopy());
-  provider_.UpdateChromePolicy(policies);
+  UpdateProviderPolicy(policies);
   EXPECT_FALSE(plugin_prefs->IsPluginEnabled(*flash));
   // The user shouldn't be able to enable it.
   EXPECT_FALSE(SetPluginEnabled(plugin_prefs, flash, true));
@@ -979,7 +987,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DisabledPluginsExceptio…
(message too large)

[commit-bot: I haz the power, 2013-01-09 12:34:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/11667006/53001

[commit-bot: I haz the power, 2013-01-09 14:28:14]

Change committed as 175797