Policy: reduce number of copy for policy conflicts

Policy conflicts used to be deep copied when merging policies, they are
now simply moved using std::move instead of being deep copied.

Bug: 1006422
Change-Id: I71239424fd7c2b31fc39c56e6f703bf68b6bd241
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1827545Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Yann Dago <ydago@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703711}

components/policy/core/common/config_dir_policy_loader_unittest.cc

@@ -246,7 +246,7 @@ TEST_F(ConfigDirPolicyLoaderTest, ReadPrefsMergePrefs) {
     conflict_policy.value = std::make_unique<base::Value>("http://bar.com");
     expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
         .GetMutable(kHomepageLocation)
-        ->AddConflictingPolicy(conflict_policy);
+        ->AddConflictingPolicy(std::move(conflict_policy));
     expected_bundle.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
         .GetMutable(kHomepageLocation)
         ->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);

components/policy/core/common/policy_bundle_unittest.cc

@@ -194,9 +194,9 @@ TEST(PolicyBundleTest, MergeFrom) {
   expected.Set(kPolicyClashing0, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(0), nullptr);
   expected.GetMutable(kPolicyClashing0)
-      ->AddConflictingPolicy(*policy1.Get(kPolicyClashing0));
+      ->AddConflictingPolicy(policy1.Get(kPolicyClashing0)->DeepCopy());
   expected.GetMutable(kPolicyClashing0)
-      ->AddConflictingPolicy(*policy2.Get(kPolicyClashing0));
+      ->AddConflictingPolicy(policy2.Get(kPolicyClashing0)->DeepCopy());
   expected.GetMutable(kPolicyClashing0)
       ->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   expected.GetMutable(kPolicyClashing0)
@@ -204,9 +204,9 @@ TEST(PolicyBundleTest, MergeFrom) {
   expected.Set(kPolicyClashing1, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
                POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(1), nullptr);
   expected.GetMutable(kPolicyClashing1)
-      ->AddConflictingPolicy(*policy0.Get(kPolicyClashing1));
+      ->AddConflictingPolicy(policy0.Get(kPolicyClashing1)->DeepCopy());
   expected.GetMutable(kPolicyClashing1)
-      ->AddConflictingPolicy(*policy2.Get(kPolicyClashing1));
+      ->AddConflictingPolicy(policy2.Get(kPolicyClashing1)->DeepCopy());
   expected.GetMutable(kPolicyClashing1)
       ->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   expected.GetMutable(kPolicyClashing1)

components/policy/core/common/policy_loader_win_unittest.cc

@@ -464,7 +464,7 @@ TEST_F(PolicyLoaderWinTest, HKLMOverHKCU) {
                             std::make_unique<base::Value>("hkcu"), nullptr);
   expected.Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
       .GetMutable(test_keys::kKeyString)
-      ->AddConflictingPolicy(conflict);
+      ->AddConflictingPolicy(std::move(conflict));
   EXPECT_TRUE(Matches(expected));
 }
 
@@ -530,9 +530,12 @@ TEST_F(PolicyLoaderWinTest, Merge3rdPartyPolicies) {
   PolicyMap::Entry a_conflict_3(
       POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, POLICY_SOURCE_PLATFORM,
       std::make_unique<base::Value>(kUserRecommended), nullptr);
-  expected_policy.GetMutable("a")->AddConflictingPolicy(a_conflict_1);
-  expected_policy.GetMutable("a")->AddConflictingPolicy(a_conflict_2);
-  expected_policy.GetMutable("a")->AddConflictingPolicy(a_conflict_3);
+  expected_policy.GetMutable("a")->AddConflictingPolicy(
+      std::move(a_conflict_1));
+  expected_policy.GetMutable("a")->AddConflictingPolicy(
+      std::move(a_conflict_2));
+  expected_policy.GetMutable("a")->AddConflictingPolicy(
+      std::move(a_conflict_3));
 
   expected_policy.Set("b", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                       POLICY_SOURCE_PLATFORM,
@@ -546,8 +549,10 @@ TEST_F(PolicyLoaderWinTest, Merge3rdPartyPolicies) {
   PolicyMap::Entry b_conflict_2(
       POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, POLICY_SOURCE_PLATFORM,
       std::make_unique<base::Value>(kUserRecommended), nullptr);
-  expected_policy.GetMutable("b")->AddConflictingPolicy(b_conflict_1);
-  expected_policy.GetMutable("b")->AddConflictingPolicy(b_conflict_2);
+  expected_policy.GetMutable("b")->AddConflictingPolicy(
+      std::move(b_conflict_1));
+  expected_policy.GetMutable("b")->AddConflictingPolicy(
+      std::move(b_conflict_2));
 
   expected_policy.Set("c", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_MACHINE,
                       POLICY_SOURCE_PLATFORM,
@@ -558,7 +563,8 @@ TEST_F(PolicyLoaderWinTest, Merge3rdPartyPolicies) {
   PolicyMap::Entry c_conflict_1(
       POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER, POLICY_SOURCE_PLATFORM,
       std::make_unique<base::Value>(kUserRecommended), nullptr);
-  expected_policy.GetMutable("c")->AddConflictingPolicy(c_conflict_1);
+  expected_policy.GetMutable("c")->AddConflictingPolicy(
+      std::move(c_conflict_1));
 
   expected_policy.Set("d", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
                       POLICY_SOURCE_PLATFORM,

components/policy/core/common/policy_map.cc

@@ -55,34 +55,24 @@ PolicyMap::Entry::Entry(Entry&&) noexcept = default;
 PolicyMap::Entry& PolicyMap::Entry::operator=(Entry&&) noexcept = default;
 
 PolicyMap::Entry PolicyMap::Entry::DeepCopy() const {
-  Entry copy;
-  copy.level = level;
-  copy.scope = scope;
-  copy.source = source;
-  if (value)
-    copy.value = value->CreateDeepCopy();
+  Entry copy(level, scope, source, value ? value->CreateDeepCopy() : nullptr,
+             external_data_fetcher
+                 ? std::make_unique<ExternalDataFetcher>(*external_data_fetcher)
+                 : nullptr);
   copy.error_strings_ = error_strings_;
   copy.error_message_ids_ = error_message_ids_;
   copy.warning_message_ids_ = warning_message_ids_;
-  if (external_data_fetcher) {
-    copy.external_data_fetcher.reset(
-        new ExternalDataFetcher(*external_data_fetcher));
-  }
+  copy.conflicts.reserve(conflicts.size());
   for (const auto& conflict : conflicts) {
-    copy.AddConflictingPolicy(conflict);
+    copy.AddConflictingPolicy(conflict.DeepCopy());
   }
   return copy;
 }
 
 bool PolicyMap::Entry::has_higher_priority_than(
     const PolicyMap::Entry& other) const {
-  if (level != other.level)
-    return level > other.level;
-
-  if (scope != other.scope)
-    return scope > other.scope;
-
-  return source > other.source;
+  return std::tie(level, scope, source) >
+         std::tie(other.level, other.scope, other.source);
 }
 
 bool PolicyMap::Entry::Equals(const PolicyMap::Entry& other) const {
@@ -116,19 +106,15 @@ void PolicyMap::Entry::AddWarning(int message_id) {
   warning_message_ids_.insert(message_id);
 }
 
-void PolicyMap::Entry::AddConflictingPolicy(const Entry& conflict) {
-  Entry conflicted_policy_copy = conflict.DeepCopy();
-
-  for (const auto& conflict : conflicted_policy_copy.conflicts) {
-    AddConflictingPolicy(conflict);
-  }
+void PolicyMap::Entry::AddConflictingPolicy(Entry&& conflict) {
+  // Move all of the newly conflicting Entry's conflicts into this Entry.
+  std::move(conflict.conflicts.begin(), conflict.conflicts.end(),
+            std::back_inserter(conflicts));
 
   // Avoid conflict nesting
-  conflicted_policy_copy.conflicts.clear();
-  conflicted_policy_copy.error_message_ids_.clear();
-  conflicted_policy_copy.warning_message_ids_.clear();
-  conflicted_policy_copy.error_strings_.clear();
-  conflicts.push_back(std::move(conflicted_policy_copy));
+  conflicts.emplace_back(conflict.level, conflict.scope, conflict.source,
+                         std::move(conflict.value),
+                         std::move(conflict.external_data_fetcher));
 }
 
 void PolicyMap::Entry::ClearConflicts() {
@@ -278,35 +264,37 @@ std::unique_ptr<PolicyMap> PolicyMap::DeepCopy() const {
 }
 
 void PolicyMap::MergeFrom(const PolicyMap& other) {
-  for (const auto& it : other) {
-    Entry* current_policy = GetMutableUntrusted(it.first);
-    auto other_policy = it.second.DeepCopy();
+  for (const auto& policy_and_entry : other) {
+    Entry* current_policy = GetMutableUntrusted(policy_and_entry.first);
+    Entry other_policy = policy_and_entry.second.DeepCopy();
 
     if (!current_policy) {
-      Set(it.first, std::move(other_policy));
+      Set(policy_and_entry.first, std::move(other_policy));
       continue;
     }
 
-    auto& new_policy = other_policy.has_higher_priority_than(*current_policy)
-                           ? other_policy
-                           : *current_policy;
-    auto& conflict =
-        current_policy == &new_policy ? other_policy : *current_policy;
+    const bool other_is_higher_priority =
+        policy_and_entry.second.has_higher_priority_than(*current_policy);
+
+    Entry& higher_policy =
+        other_is_higher_priority ? other_policy : *current_policy;
+    Entry& conflicting_policy =
+        other_is_higher_priority ? *current_policy : other_policy;
 
-    bool overwriting_default_policy =
-        new_policy.source != conflict.source &&
-        conflict.source == POLICY_SOURCE_ENTERPRISE_DEFAULT;
+    const bool overwriting_default_policy =
+        higher_policy.source != conflicting_policy.source &&
+        conflicting_policy.source == POLICY_SOURCE_ENTERPRISE_DEFAULT;
     if (!overwriting_default_policy) {
-      new_policy.AddConflictingPolicy(conflict);
-      new_policy.AddWarning(
+      higher_policy.AddConflictingPolicy(std::move(conflicting_policy));
+      higher_policy.AddWarning(
           (current_policy->value &&
-           it.second.value->Equals(current_policy->value.get()))
+           *policy_and_entry.second.value == *current_policy->value)
               ? IDS_POLICY_CONFLICT_SAME_VALUE
               : IDS_POLICY_CONFLICT_DIFF_VALUE);
     }
 
-    if (current_policy != &new_policy)
-      Set(it.first, std::move(new_policy));
+    if (other_is_higher_priority)
+      *current_policy = std::move(other_policy);
   }
 }
 

components/policy/core/common/policy_map.h

@@ -73,7 +73,7 @@ class POLICY_EXPORT PolicyMap {
     void AddWarning(int message_id);
 
     // Adds a conflicting policy.
-    void AddConflictingPolicy(const Entry& conflict);
+    void AddConflictingPolicy(Entry&& conflict);
 
     // Removes all the conflicts.
     void ClearConflicts();

components/policy/core/common/policy_map_unittest.cc

@@ -264,31 +264,34 @@ TEST_F(PolicyMapTest, MergeFrom) {
         POLICY_SOURCE_CLOUD, std::make_unique<base::Value>("chromium.org"),
         nullptr);
   c.GetMutable(kTestPolicyName1)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
-  c.GetMutable(kTestPolicyName1)->AddConflictingPolicy(conflicted_policy_1);
+  c.GetMutable(kTestPolicyName1)
+      ->AddConflictingPolicy(std::move(conflicted_policy_1));
   // |a| has precedence over |b|.
   c.Set(kTestPolicyName2, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
         POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(true), nullptr);
   c.GetMutable(kTestPolicyName2)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   c.GetMutable(kTestPolicyName2)
-      ->AddConflictingPolicy(*b.Get(kTestPolicyName2));
+      ->AddConflictingPolicy(b.Get(kTestPolicyName2)->DeepCopy());
   c.Set(kTestPolicyName3, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
         POLICY_SOURCE_ENTERPRISE_DEFAULT, nullptr,
         CreateExternalDataFetcher("a"));
   c.GetMutable(kTestPolicyName3)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   c.GetMutable(kTestPolicyName3)
-      ->AddConflictingPolicy(*b.Get(kTestPolicyName3));
+      ->AddConflictingPolicy(b.Get(kTestPolicyName3)->DeepCopy());
   // POLICY_SCOPE_MACHINE over POLICY_SCOPE_USER for POLICY_LEVEL_RECOMMENDED.
   c.Set(kTestPolicyName4, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_MACHINE,
         POLICY_SOURCE_DEVICE_LOCAL_ACCOUNT_OVERRIDE,
         std::make_unique<base::Value>(true), nullptr);
   c.GetMutable(kTestPolicyName4)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
-  c.GetMutable(kTestPolicyName4)->AddConflictingPolicy(conflicted_policy_4);
+  c.GetMutable(kTestPolicyName4)
+      ->AddConflictingPolicy(std::move(conflicted_policy_4));
   // POLICY_LEVEL_MANDATORY over POLICY_LEVEL_RECOMMENDED.
   c.Set(kTestPolicyName5, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
         POLICY_SOURCE_PLATFORM, std::make_unique<base::Value>(std::string()),
         nullptr);
   c.GetMutable(kTestPolicyName5)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
-  c.GetMutable(kTestPolicyName5)->AddConflictingPolicy(conflicted_policy_5);
+  c.GetMutable(kTestPolicyName5)
+      ->AddConflictingPolicy(std::move(conflicted_policy_5));
   // Merge new ones.
   c.Set(kTestPolicyName6, POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
         POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(true), nullptr);
@@ -302,7 +305,8 @@ TEST_F(PolicyMapTest, MergeFrom) {
         POLICY_SOURCE_ACTIVE_DIRECTORY,
         std::make_unique<base::Value>("blocked AD policy"), nullptr);
   c.GetMutable(kTestPolicyName8)->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
-  c.GetMutable(kTestPolicyName8)->AddConflictingPolicy(conflicted_policy_8);
+  c.GetMutable(kTestPolicyName8)
+      ->AddConflictingPolicy(std::move(conflicted_policy_8));
   c.GetMutable(kTestPolicyName8)->SetBlocked();
 
   EXPECT_TRUE(a.Equals(c));
@@ -363,7 +367,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
   PolicyMap::Entry expected_case1(POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                                   POLICY_SOURCE_MERGED,
                                   std::make_unique<base::Value>(abcd), nullptr);
-  expected_case1.AddConflictingPolicy(case1);
+  expected_case1.AddConflictingPolicy(case1.DeepCopy());
 
   // Case 2 - kTestPolicyName2
   // Policies should only be merged with other policies with the same target,
@@ -383,7 +387,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
   PolicyMap::Entry expected_case2(
       POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_MACHINE, POLICY_SOURCE_MERGED,
       std::make_unique<base::Value>(int1234), nullptr);
-  expected_case2.AddConflictingPolicy(case2);
+  expected_case2.AddConflictingPolicy(case2.DeepCopy());
 
   // Case 3 - kTestPolicyName3
   // Trivial case with 2 sources.
@@ -400,7 +404,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
                                   std::make_unique<base::Value>(abcd), nullptr);
   auto case3_blocked_by_group = expected_case3.DeepCopy();
   case3_blocked_by_group.SetIgnoredByPolicyAtomicGroup();
-  expected_case3.AddConflictingPolicy(case3);
+  expected_case3.AddConflictingPolicy(case3.DeepCopy());
 
   // Case 4 - kTestPolicyName4
   // Policies with a single source should stay the same.
@@ -410,7 +414,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
   PolicyMap::Entry expected_case4(POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
                                   POLICY_SOURCE_MERGED,
                                   std::make_unique<base::Value>(ef), nullptr);
-  expected_case4.AddConflictingPolicy(case4);
+  expected_case4.AddConflictingPolicy(case4.DeepCopy());
 
   // Case 5 - kTestPolicyName5
   // Policies that are not lists should not be merged.
@@ -440,7 +444,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
   PolicyMap::Entry expected_case6(POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                                   POLICY_SOURCE_MERGED,
                                   std::make_unique<base::Value>(ab), nullptr);
-  expected_case6.AddConflictingPolicy(case6);
+  expected_case6.AddConflictingPolicy(case6.DeepCopy());
 
   // Case 7 - kTestPolicyName7
   // Lists of dictionaries should not have duplicates.
@@ -460,7 +464,7 @@ TEST_F(PolicyMapTest, MergeValuesList) {
   PolicyMap::Entry expected_case7(
       POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, POLICY_SOURCE_MERGED,
       std::make_unique<base::Value>(list_dict_abcd), nullptr);
-  expected_case7.AddConflictingPolicy(case7);
+  expected_case7.AddConflictingPolicy(case7.DeepCopy());
 
   PolicyMap policy_not_merged;
   policy_not_merged.Set(kTestPolicyName1, case1.DeepCopy());
@@ -570,7 +574,7 @@ TEST_F(PolicyMapTest, MergeDictionaryValues) {
   PolicyMap::Entry expected_case1(
       POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE, POLICY_SOURCE_MERGED,
       base::Value::ToUniquePtrValue(merged_dict_case1.Clone()), nullptr);
-  expected_case1.AddConflictingPolicy(case1);
+  expected_case1.AddConflictingPolicy(case1.DeepCopy());
 
   // Case  - kTestPolicyName2
   // Policies should only be merged with other policies with the same target,
@@ -595,7 +599,7 @@ TEST_F(PolicyMapTest, MergeDictionaryValues) {
   PolicyMap::Entry expected_case2(
       POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_MACHINE, POLICY_SOURCE_MERGED,
       base::Value::ToUniquePtrValue(merged_dict_case2.Clone()), nullptr);
-  expected_case2.AddConflictingPolicy(case2);
+  expected_case2.AddConflictingPolicy(case2.DeepCopy());
 
   // Case 3 - kTestPolicyName3
   // Enterprise default policies should not be merged with other sources.
@@ -624,7 +628,7 @@ TEST_F(PolicyMapTest, MergeDictionaryValues) {
   PolicyMap::Entry expected_case3(
       POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, POLICY_SOURCE_MERGED,
       base::Value::ToUniquePtrValue(merged_dict_case3.Clone()), nullptr);
-  expected_case3.AddConflictingPolicy(case3);
+  expected_case3.AddConflictingPolicy(case3.DeepCopy());
 
   // Case 4 - kTestPolicyName4
   // Policies with a single source should be merged.
@@ -635,7 +639,7 @@ TEST_F(PolicyMapTest, MergeDictionaryValues) {
   PolicyMap::Entry expected_case4(
       POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE, POLICY_SOURCE_MERGED,
       base::Value::ToUniquePtrValue(dict_a.Clone()), nullptr);
-  expected_case4.AddConflictingPolicy(case4);
+  expected_case4.AddConflictingPolicy(case4.DeepCopy());
 
   // Case 5 - kTestPolicyName5
   // Policies that are not dictionaries should not be merged.
@@ -666,7 +670,7 @@ TEST_F(PolicyMapTest, MergeDictionaryValues) {
   PolicyMap::Entry expected_case6(
       POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, POLICY_SOURCE_MERGED,
       base::Value::ToUniquePtrValue(dict_a.Clone()), nullptr);
-  expected_case6.AddConflictingPolicy(case6);
+  expected_case6.AddConflictingPolicy(case6.DeepCopy());
 
   // Case 7 - kTestPolicyName7
   // Policies that are not dictionaries should not be merged.
@@ -939,8 +943,8 @@ TEST_F(PolicyMapTest, EntryAddConflict) {
   PolicyMap::Entry entry_c = entry_a.DeepCopy();
   entry_c.source = POLICY_SOURCE_PLATFORM;
 
-  entry_b.AddConflictingPolicy(entry_c);
-  entry_a.AddConflictingPolicy(entry_b);
+  entry_b.AddConflictingPolicy(entry_c.DeepCopy());
+  entry_a.AddConflictingPolicy(entry_b.DeepCopy());
 
   EXPECT_TRUE(entry_a.conflicts.size() == 2);
   EXPECT_TRUE(entry_b.conflicts.size() == 1);

components/policy/core/common/policy_merger.cc

@@ -122,7 +122,7 @@ void PolicyListMerger::DoMerge(PolicyMap::Entry* policy) const {
     policy->value.reset(new_value);
   }
   policy->ClearConflicts();
-  policy->AddConflictingPolicy(new_conflict);
+  policy->AddConflictingPolicy(std::move(new_conflict));
   policy->source = POLICY_SOURCE_MERGED;
 }
 
@@ -212,7 +212,7 @@ void PolicyDictionaryMerger::DoMerge(PolicyMap::Entry* policy) const {
     policy->value = base::Value::ToUniquePtrValue(std::move(merged_dictionary));
 
   policy->ClearConflicts();
-  policy->AddConflictingPolicy(new_conflict);
+  policy->AddConflictingPolicy(std::move(new_conflict));
   policy->source = POLICY_SOURCE_MERGED;
 }
 

components/policy/core/common/policy_service_impl_unittest.cc

@@ -403,8 +403,10 @@ TEST_F(PolicyServiceTest, Priorities) {
   provider0_.UpdateChromePolicy(policy0_);
   provider1_.UpdateChromePolicy(policy1_);
   provider2_.UpdateChromePolicy(policy2_);
-  expected.GetMutable("aaa")->AddConflictingPolicy(*policy1_.Get("aaa"));
-  expected.GetMutable("aaa")->AddConflictingPolicy(*policy2_.Get("aaa"));
+  expected.GetMutable("aaa")->AddConflictingPolicy(
+      policy1_.Get("aaa")->DeepCopy());
+  expected.GetMutable("aaa")->AddConflictingPolicy(
+      policy2_.Get("aaa")->DeepCopy());
 
   EXPECT_TRUE(VerifyPolicies(
       PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()), expected));
@@ -414,7 +416,8 @@ TEST_F(PolicyServiceTest, Priorities) {
   expected.GetMutable("aaa")->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   policy0_.Erase("aaa");
   provider0_.UpdateChromePolicy(policy0_);
-  expected.GetMutable("aaa")->AddConflictingPolicy(*policy2_.Get("aaa"));
+  expected.GetMutable("aaa")->AddConflictingPolicy(
+      policy2_.Get("aaa")->DeepCopy());
   EXPECT_TRUE(VerifyPolicies(
       PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()), expected));
 
@@ -423,7 +426,8 @@ TEST_F(PolicyServiceTest, Priorities) {
   expected.GetMutable("aaa")->AddWarning(IDS_POLICY_CONFLICT_SAME_VALUE);
   policy1_.Set("aaa", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
                POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(1), nullptr);
-  expected.GetMutable("aaa")->AddConflictingPolicy(*policy2_.Get("aaa"));
+  expected.GetMutable("aaa")->AddConflictingPolicy(
+      policy2_.Get("aaa")->DeepCopy());
   provider1_.UpdateChromePolicy(policy2_);
   EXPECT_TRUE(VerifyPolicies(
       PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()), expected));
@@ -577,12 +581,14 @@ TEST_F(PolicyServiceTest, NamespaceMerge) {
       ->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   expected.GetMutable(kSameLevelPolicy)
       ->AddConflictingPolicy(
-          *bundle1->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
-               .Get(kSameLevelPolicy));
+          bundle1->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
+              .Get(kSameLevelPolicy)
+              ->DeepCopy());
   expected.GetMutable(kSameLevelPolicy)
       ->AddConflictingPolicy(
-          *bundle2->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
-               .Get(kSameLevelPolicy));
+          bundle2->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
+              .Get(kSameLevelPolicy)
+              ->DeepCopy());
   // For policies with different levels and scopes, the highest priority
   // level/scope combination takes precedence, on every namespace.
   expected.Set(kDiffLevelPolicy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
@@ -592,12 +598,14 @@ TEST_F(PolicyServiceTest, NamespaceMerge) {
       ->AddWarning(IDS_POLICY_CONFLICT_DIFF_VALUE);
   expected.GetMutable(kDiffLevelPolicy)
       ->AddConflictingPolicy(
-          *bundle0->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
-               .Get(kDiffLevelPolicy));
+          bundle0->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
+              .Get(kDiffLevelPolicy)
+              ->DeepCopy());
   expected.GetMutable(kDiffLevelPolicy)
       ->AddConflictingPolicy(
-          *bundle1->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
-               .Get(kDiffLevelPolicy));
+          bundle1->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
+              .Get(kDiffLevelPolicy)
+              ->DeepCopy());
 
   provider0_.UpdatePolicy(std::move(bundle0));
   provider1_.UpdatePolicy(std::move(bundle1));

components/policy/core/common/schema_map_unittest.cc

@@ -186,7 +186,7 @@ TEST_F(SchemaMapTest, FilterBundle) {
   expected_bundle.Get(chrome_ns)
       .GetMutable("ChromePolicy")
       ->AddConflictingPolicy(
-          *expected_bundle.Get(chrome_ns).Get("ChromePolicy"));
+          expected_bundle.Get(chrome_ns).Get("ChromePolicy")->DeepCopy());
   expected_bundle.Get(chrome_ns)
       .GetMutable("ChromePolicy")
       ->AddWarning(IDS_POLICY_CONFLICT_SAME_VALUE);