From e7fce50d090e0b5bb7fd47382839367d172432e8 Mon Sep 17 00:00:00 2001
From: "danno@chromium.org"
<danno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 10 Dec 2010 12:30:59 +0000
Subject: [PATCH] Fix memory leaks in AsynchronousPolicyProvider.
BUG=66102, 66054
TEST=valgrind on linux and mac
Review URL: http://codereview.chromium.org/5748002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68844 0039d316-1c4b-4281-b951-d872f2087c98
---
.../policy/asynchronous_policy_loader.cc | 5 +++--
.../policy/asynchronous_policy_loader.h | 15 ++++++-------
tools/valgrind/memcheck/suppressions.txt | 21 -------------------
tools/valgrind/memcheck/suppressions_mac.txt | 9 --------
4 files changed, 11 insertions(+), 39 deletions(-)
diff --git a/chrome/browser/policy/asynchronous_policy_loader.cc b/chrome/browser/policy/asynchronous_policy_loader.cc
index ea27bb767dfd4..f244eda5c00ad 100644
--- a/chrome/browser/policy/asynchronous_policy_loader.cc
+++ b/chrome/browser/policy/asynchronous_policy_loader.cc
@@ -61,9 +61,10 @@ void AsynchronousPolicyLoader::PostUpdatePolicyTask(
}
void AsynchronousPolicyLoader::UpdatePolicy(DictionaryValue* new_policy_raw) {
+ scoped_ptr<DictionaryValue> new_policy(new_policy_raw);
DCHECK(policy_.get());
- if (!policy_->Equals(new_policy_raw)) {
- policy_.reset(new_policy_raw);
+ if (!policy_->Equals(new_policy.get())) {
+ policy_.reset(new_policy.release());
// TODO(danno): Change the notification between the provider and the
// PrefStore into a notification mechanism, removing the need for the
// WeakPtr for the provider.
diff --git a/chrome/browser/policy/asynchronous_policy_loader.h b/chrome/browser/policy/asynchronous_policy_loader.h
index 2600847133875..30878f5553b64 100644
--- a/chrome/browser/policy/asynchronous_policy_loader.h
+++ b/chrome/browser/policy/asynchronous_policy_loader.h
@@ -47,15 +47,10 @@ class AsynchronousPolicyLoader
friend class base::RefCountedThreadSafe<AsynchronousPolicyLoader>;
virtual ~AsynchronousPolicyLoader() {}
- // Schedules a call to UpdatePolicy on |origin_loop_|.
+ // Schedules a call to UpdatePolicy on |origin_loop_|. Takes ownership of
+ // |new_policy|.
void PostUpdatePolicyTask(DictionaryValue* new_policy);
- // Replaces the existing policy to value map with a new one, sending
- // notification to the provider if there is a policy change. Must be called on
- // |origin_loop_| so that it's safe to call back into the provider, which is
- // not thread-safe.
- void UpdatePolicy(DictionaryValue* new_policy);
-
AsynchronousPolicyProvider::Delegate* delegate() {
return delegate_.get();
}
@@ -67,6 +62,12 @@ class AsynchronousPolicyLoader
private:
friend class AsynchronousPolicyLoaderTest;
+ // Replaces the existing policy to value map with a new one, sending
+ // notification to the provider if there is a policy change. Must be called on
+ // |origin_loop_| so that it's safe to call back into the provider, which is
+ // not thread-safe. Takes ownership of |new_policy|.
+ void UpdatePolicy(DictionaryValue* new_policy);
+
// Provides the low-level mechanics for loading policy.
scoped_ptr<AsynchronousPolicyProvider::Delegate> delegate_;
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index 70f44186d1c12..9b21ae7810350 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -3470,27 +3470,6 @@
fun:_ZN8remoting8protocol14RtpVideoReader18RebuildVideoPacketESt15_Deque_iteratorINS1_17PacketsQueueEntryERS3_PS3_ES6_
fun:_ZN8remoting8protocol14RtpVideoReader15CheckFullPacketESt15_Deque_iteratorINS1_17PacketsQueueEntryERS3_PS3_E
}
-{
- bug_66054_a
- Memcheck:Leak
- fun:_Znw*
- ...
- fun:_ZNK6policy36CreateSequencedTestDictionaryActionPIPiE10gmock_ImplIFP15DictionaryValuevEE17gmock_PerformImplIN7testing8internal12ExcessiveArgESB_SB_SB_SB_SB_SB_SB_SB_SB_EES5_RKNSt3tr15tupleINSC_10_NullClassESE_SE_SE_SE_SE_SE_SE_SE_SE_EET_T0_T1_T2_T3_T4_T5_T6_T7_T8_
- fun:_ZN7testing8internal12ActionHelperIP15DictionaryValueN6policy36CreateSequencedTestDictionaryActionPIPiE10gmock_ImplIFS3_vEEEE7PerformEPSA_RKNSt3tr15tupleINSD_10_NullClassESF_SF_SF_SF_SF_SF_SF_SF_SF_EE
- fun:_ZN6policy36CreateSequencedTestDictionaryActionPIPiE10gmock_ImplIFP15DictionaryValuevEE7PerformERKNSt3tr15tupleINS8_10_NullClassESA_SA_SA_SA_SA_SA_SA_SA_SA_EE
- fun:_ZNK7testing6ActionIFP15DictionaryValuevEE7PerformERKNSt3tr15tupleINS5_10_NullClassES7_S7_S7_S7_S7_S7_S7_S7_S7_EE
- fun:_ZN7testing8internal18FunctionMockerBaseIFP15DictionaryValuevEE10InvokeWithERKNSt3tr15tupleINS6_10_NullClassES8_S8_S8_S8_S8_S8_S8_S8_S8_EE
- fun:_ZN7testing8internal14FunctionMockerIFP15DictionaryValuevEE6InvokeEv
- fun:_ZN6policy20ProviderDelegateMock4LoadEv
- fun:_ZN6policy24AsynchronousPolicyLoader6ReloadEv
- fun:_ZN6policy68AsynchronousPolicyLoaderTest_ProviderNotificationOnPolicyChange_Test8TestBodyEv
-}
-{
- bug_66054_b
- Memcheck:Leak
- fun:_Znw*
- fun:_ZN6policy47AsynchronousPolicyTestBase_ProviderRefresh_Test8TestBodyEv
-}
{
bug_66277
Memcheck:Leak
diff --git a/tools/valgrind/memcheck/suppressions_mac.txt b/tools/valgrind/memcheck/suppressions_mac.txt
index 058c38edae50d..f876ceac66c9f 100644
--- a/tools/valgrind/memcheck/suppressions_mac.txt
+++ b/tools/valgrind/memcheck/suppressions_mac.txt
@@ -1173,12 +1173,3 @@
fun:_ZN13UtilityThreadC2Ev
fun:_ZN13UtilityThreadC1Ev
}
-{
- bug_66102
- Memcheck:Leak
- fun:_Znw*
- ...
- fun:_ZN6policy21FileBasedPolicyLoader11InitWatcherEv
- ...
- fun:_ZN54PrefValueStorePolicyRefreshTest_TestPolicyRefresh_Test8TestBodyEv
-}
--
GitLab