Use std::unique_ptr to express ownership of base::Value in PolicyMap::Entry

components/policy/core/browser/proxy_policy_handler_unittest.cc

Index: components/policy/core/browser/proxy_policy_handler_unittest.cc
diff --git a/components/policy/core/browser/proxy_policy_handler_unittest.cc b/components/policy/core/browser/proxy_policy_handler_unittest.cc
index bfed47dd441ee3db1afb4f438b48f1f597e67b2f..6c8dbb427c448325118ca8c6432b487b789723d7 100644
--- a/components/policy/core/browser/proxy_policy_handler_unittest.cc
+++ b/components/policy/core/browser/proxy_policy_handler_unittest.cc
@@ -76,26 +76,20 @@ class ProxyPolicyHandlerTest
 
 TEST_F(ProxyPolicyHandlerTest, ManualOptions) {
   PolicyMap policy;
-  policy.Set(key::kProxyBypassList,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://chromium.org/override"),
-             NULL);
-  policy.Set(key::kProxyServer,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+  policy.Set(
+      key::kProxyBypassList, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://chromium.org/override")),
+      nullptr);
+  policy.Set(key::kProxyServer, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::StringValue("chromium.org"),
-             NULL);
+             base::WrapUnique(new base::StringValue("chromium.org")), nullptr);
   policy.Set(
-      key::kProxyServerMode,
-      POLICY_LEVEL_MANDATORY,
-      POLICY_SCOPE_USER,
+      key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
       POLICY_SOURCE_CLOUD,
-      new base::FundamentalValue(
-          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE),
-      NULL);
+      base::WrapUnique(new base::FundamentalValue(
+          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE)),
+      nullptr);
   UpdateProviderPolicy(policy);
 
   VerifyProxyPrefs("chromium.org",
@@ -107,25 +101,19 @@ TEST_F(ProxyPolicyHandlerTest, ManualOptions) {
 TEST_F(ProxyPolicyHandlerTest, ManualOptionsReversedApplyOrder) {
   PolicyMap policy;
   policy.Set(
-      key::kProxyServerMode,
-      POLICY_LEVEL_MANDATORY,
-      POLICY_SCOPE_USER,
+      key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
       POLICY_SOURCE_CLOUD,
-      new base::FundamentalValue(
-          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE),
-      NULL);
-  policy.Set(key::kProxyBypassList,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://chromium.org/override"),
-             NULL);
-  policy.Set(key::kProxyServer,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+      base::WrapUnique(new base::FundamentalValue(
+          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE)),
+      nullptr);
+  policy.Set(
+      key::kProxyBypassList, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://chromium.org/override")),
+      nullptr);
+  policy.Set(key::kProxyServer, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::StringValue("chromium.org"),
-             NULL);
+             base::WrapUnique(new base::StringValue("chromium.org")), nullptr);
   UpdateProviderPolicy(policy);
 
   VerifyProxyPrefs("chromium.org",
@@ -137,13 +125,11 @@ TEST_F(ProxyPolicyHandlerTest, ManualOptionsReversedApplyOrder) {
 TEST_F(ProxyPolicyHandlerTest, ManualOptionsInvalid) {
   PolicyMap policy;
   policy.Set(
-      key::kProxyServerMode,
-      POLICY_LEVEL_MANDATORY,
-      POLICY_SCOPE_USER,
+      key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
       POLICY_SOURCE_CLOUD,
-      new base::FundamentalValue(
-          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE),
-      NULL);
+      base::WrapUnique(new base::FundamentalValue(
+          ProxyPolicyHandler::PROXY_MANUALLY_CONFIGURED_PROXY_SERVER_MODE)),
+      nullptr);
   UpdateProviderPolicy(policy);
 
   const base::Value* value = NULL;
@@ -152,12 +138,10 @@ TEST_F(ProxyPolicyHandlerTest, ManualOptionsInvalid) {
 
 TEST_F(ProxyPolicyHandlerTest, NoProxyServerMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyServerMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::FundamentalValue(ProxyPolicyHandler::PROXY_SERVER_MODE),
-             NULL);
+  policy.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::FundamentalValue(
+                                      ProxyPolicyHandler::PROXY_SERVER_MODE)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(
       std::string(), std::string(), std::string(), ProxyPrefs::MODE_DIRECT);
@@ -165,12 +149,11 @@ TEST_F(ProxyPolicyHandlerTest, NoProxyServerMode) {
 
 TEST_F(ProxyPolicyHandlerTest, NoProxyModeName) {
   PolicyMap policy;
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kDirectProxyModeName),
-             NULL);
+  policy.Set(
+      key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue(ProxyPrefs::kDirectProxyModeName)),
+      nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(
       std::string(), std::string(), std::string(), ProxyPrefs::MODE_DIRECT);
@@ -178,13 +161,11 @@ TEST_F(ProxyPolicyHandlerTest, NoProxyModeName) {
 
 TEST_F(ProxyPolicyHandlerTest, AutoDetectProxyServerMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyServerMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+  policy.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::FundamentalValue(
-                 ProxyPolicyHandler::PROXY_AUTO_DETECT_PROXY_SERVER_MODE),
-             NULL);
+             base::WrapUnique(new base::FundamentalValue(
+                 ProxyPolicyHandler::PROXY_AUTO_DETECT_PROXY_SERVER_MODE)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(std::string(),
                    std::string(),
@@ -194,12 +175,10 @@ TEST_F(ProxyPolicyHandlerTest, AutoDetectProxyServerMode) {
 
 TEST_F(ProxyPolicyHandlerTest, AutoDetectProxyModeName) {
   PolicyMap policy;
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kAutoDetectProxyModeName),
-             NULL);
+  policy.Set(key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::StringValue(
+                                      ProxyPrefs::kAutoDetectProxyModeName)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(std::string(),
                    std::string(),
@@ -209,18 +188,15 @@ TEST_F(ProxyPolicyHandlerTest, AutoDetectProxyModeName) {
 
 TEST_F(ProxyPolicyHandlerTest, PacScriptProxyMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyPacUrl,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://short.org/proxy.pac"),
-             NULL);
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kPacScriptProxyModeName),
-             NULL);
+  policy.Set(
+      key::kProxyPacUrl, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://short.org/proxy.pac")),
+      nullptr);
+  policy.Set(key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::StringValue(
+                                      ProxyPrefs::kPacScriptProxyModeName)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(std::string(),
                    "http://short.org/proxy.pac",
@@ -230,11 +206,9 @@ TEST_F(ProxyPolicyHandlerTest, PacScriptProxyMode) {
 
 TEST_F(ProxyPolicyHandlerTest, PacScriptProxyModeInvalid) {
   PolicyMap policy;
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kPacScriptProxyModeName),
+  policy.Set(key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::StringValue(
+                                      ProxyPrefs::kPacScriptProxyModeName)),
              NULL);
   UpdateProviderPolicy(policy);
   const base::Value* value = NULL;
@@ -245,24 +219,18 @@ TEST_F(ProxyPolicyHandlerTest, PacScriptProxyModeInvalid) {
 // for unset properties.
 TEST_F(ProxyPolicyHandlerTest, PacScriptProxyModeBug78016) {
   PolicyMap policy;
-  policy.Set(key::kProxyServer,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+  policy.Set(key::kProxyServer, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::StringValue(std::string()),
-             NULL);
-  policy.Set(key::kProxyPacUrl,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://short.org/proxy.pac"),
-             NULL);
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kPacScriptProxyModeName),
-             NULL);
+             base::WrapUnique(new base::StringValue(std::string())), nullptr);
+  policy.Set(
+      key::kProxyPacUrl, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://short.org/proxy.pac")),
+      nullptr);
+  policy.Set(key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::StringValue(
+                                      ProxyPrefs::kPacScriptProxyModeName)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(std::string(),
                    "http://short.org/proxy.pac",
@@ -272,13 +240,11 @@ TEST_F(ProxyPolicyHandlerTest, PacScriptProxyModeBug78016) {
 
 TEST_F(ProxyPolicyHandlerTest, UseSystemProxyServerMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyServerMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+  policy.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::FundamentalValue(
-                 ProxyPolicyHandler::PROXY_USE_SYSTEM_PROXY_SERVER_MODE),
-             NULL);
+             base::WrapUnique(new base::FundamentalValue(
+                 ProxyPolicyHandler::PROXY_USE_SYSTEM_PROXY_SERVER_MODE)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(
       std::string(), std::string(), std::string(), ProxyPrefs::MODE_SYSTEM);
@@ -286,12 +252,11 @@ TEST_F(ProxyPolicyHandlerTest, UseSystemProxyServerMode) {
 
 TEST_F(ProxyPolicyHandlerTest, UseSystemProxyMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kSystemProxyModeName),
-             NULL);
+  policy.Set(
+      key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue(ProxyPrefs::kSystemProxyModeName)),
+      nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(
       std::string(), std::string(), std::string(), ProxyPrefs::MODE_SYSTEM);
@@ -300,18 +265,14 @@ TEST_F(ProxyPolicyHandlerTest, UseSystemProxyMode) {
 TEST_F(ProxyPolicyHandlerTest,
        ProxyModeOverridesProxyServerMode) {
   PolicyMap policy;
-  policy.Set(key::kProxyServerMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::FundamentalValue(ProxyPolicyHandler::PROXY_SERVER_MODE),
-             NULL);
-  policy.Set(key::kProxyMode,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue(ProxyPrefs::kAutoDetectProxyModeName),
-             NULL);
+  policy.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::FundamentalValue(
+                                      ProxyPolicyHandler::PROXY_SERVER_MODE)),
+             nullptr);
+  policy.Set(key::kProxyMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+             POLICY_SOURCE_CLOUD, base::WrapUnique(new base::StringValue(
+                                      ProxyPrefs::kAutoDetectProxyModeName)),
+             nullptr);
   UpdateProviderPolicy(policy);
   VerifyProxyPrefs(std::string(),
                    std::string(),
@@ -322,33 +283,25 @@ TEST_F(ProxyPolicyHandlerTest,
 TEST_F(ProxyPolicyHandlerTest, ProxyInvalid) {
   // No mode expects all three parameters being set.
   PolicyMap policy;
-  policy.Set(key::kProxyPacUrl,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://short.org/proxy.pac"),
-             NULL);
-  policy.Set(key::kProxyBypassList,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
-             POLICY_SOURCE_CLOUD,
-             new base::StringValue("http://chromium.org/override"),
-             NULL);
-  policy.Set(key::kProxyServer,
-             POLICY_LEVEL_MANDATORY,
-             POLICY_SCOPE_USER,
+  policy.Set(
+      key::kProxyPacUrl, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,

[Andrew T Wilson (Slow), 2016/05/03 11:59:03]

I think if you are wrapping after the open paren, you are supposed to put
one-param-per-line.
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

Here and below also.

[dcheng, 2016/05/03 23:40:40]

This is from clang-format. I believe that this is permissible according to the
style guide: there's an example where this is true, but the list of guidelines
doesn't seem to explicitly prohibit this either.

Note that there's one statement that would rule this out, but this particular
guideline is also at odds with one of the examples provided in the same section.
Specifically:
All parameters should be aligned if possible.

Doesn't seem compatible with the second example:
ReturnType ClassName::ReallyLongFunctionName(Type par_name1, Type par_name2,
                                             Type par_name3) {
  DoSomething();
  ...
}

In light of this, I'd prefer to keep whatever clang-format came up with.

+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://short.org/proxy.pac")),
+      nullptr);
+  policy.Set(
+      key::kProxyBypassList, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD,
+      base::WrapUnique(new base::StringValue("http://chromium.org/override")),
+      nullptr);
+  policy.Set(key::kProxyServer, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              POLICY_SOURCE_CLOUD,
-             new base::StringValue("chromium.org"),
-             NULL);
+             base::WrapUnique(new base::StringValue("chromium.org")), nullptr);
   for (int i = 0; i < ProxyPolicyHandler::MODE_COUNT; ++i) {
-    policy.Set(key::kProxyServerMode,
-               POLICY_LEVEL_MANDATORY,
-               POLICY_SCOPE_USER,
+    policy.Set(key::kProxyServerMode, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                POLICY_SOURCE_CLOUD,
-               new base::FundamentalValue(i),
-               NULL);
+               base::WrapUnique(new base::FundamentalValue(i)), nullptr);
     UpdateProviderPolicy(policy);
-    const base::Value* value = NULL;
+    const base::Value* value = nullptr;
     EXPECT_FALSE(store_->GetValue(proxy_config::prefs::kProxy, &value));
   }
 }