[Extensions Cleanup] Make BaseSetOperators::insert() take a unique_ptr

Update BaseSetOperators::insert() (and the related method
APIpermissionSet::insert()) accept a unique_ptr as a parameter rather
than a raw pointer. This makes it significantly more clear that
ownership is taken as a result of the method call.

Update all call sites appropriately.

Bug: 907661
Change-Id: I08568711a01d7a371343dc9137aefedf86d43c7a
Reviewed-on: https://chromium-review.googlesource.com/c/1347621
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611201}

chrome/browser/chromeos/extensions/permissions_updater_delegate_chromeos_unittest.cc

@@ -45,8 +45,8 @@ std::unique_ptr<const PermissionSet> CreatePermissions(
   if (include_clipboard)
     apis.insert(APIPermission::kClipboardRead);
   ManifestPermissionSet manifest;
-  manifest.insert(new MockManifestPermission("author"));
-  manifest.insert(new MockManifestPermission("background"));
+  manifest.insert(std::make_unique<MockManifestPermission>("author"));
+  manifest.insert(std::make_unique<MockManifestPermission>("background"));
   URLPatternSet explicit_hosts({
       URLPattern(URLPattern::SCHEME_ALL, "http://www.google.com/*"),
       URLPattern(URLPattern::SCHEME_ALL, "<all_urls>")});

chrome/browser/extensions/api/permissions/permissions_api_helpers.cc

@@ -90,14 +90,15 @@ std::unique_ptr<const PermissionSet> UnpackPermissionSet(
           return NULL;
         }
 
-        APIPermission* permission = NULL;
+        std::unique_ptr<APIPermission> permission;
 
         // Explicitly check the permissions that accept arguments until the bug
         // referenced above is fixed.
         const APIPermissionInfo* usb_device_permission_info =
             info->GetByID(APIPermission::kUsbDevice);
         if (permission_name == usb_device_permission_info->name()) {
-          permission = new UsbDevicePermission(usb_device_permission_info);
+          permission =
+              std::make_unique<UsbDevicePermission>(usb_device_permission_info);
         } else {
           *error = kUnsupportedPermissionId;
           return NULL;
@@ -108,7 +109,7 @@ std::unique_ptr<const PermissionSet> UnpackPermissionSet(
           *error = ErrorUtils::FormatErrorMessage(kInvalidParameter, *it);
           return NULL;
         }
-        apis.insert(permission);
+        apis.insert(std::move(permission));
       } else {
         const APIPermissionInfo* permission_info = info->GetByName(*it);
         if (!permission_info) {

chrome/browser/extensions/extension_prefs_unittest.cc

@@ -176,7 +176,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
       value->AppendString("udp-send-to::8888");
       ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
     }
-    api_perm_set1_.insert(permission.release());
+    api_perm_set1_.insert(std::move(permission));
 
     api_perm_set2_.insert(APIPermission::kHistory);
 

chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc

@@ -125,7 +125,7 @@ TEST_F(ChromePermissionMessageProviderUnittest,
   devices_list->Append(
       UsbDevicePermissionData(0x02ad, 0x138d, -1, -1).ToValue());
   ASSERT_TRUE(usb->FromValue(devices_list.get(), nullptr, nullptr));
-  permissions.insert(usb.release());
+  permissions.insert(std::move(usb));
 
   PermissionMessages messages =
       GetMessages(permissions, Manifest::TYPE_EXTENSION);

chrome/common/extensions/permissions/permission_set_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -323,8 +324,6 @@ TEST(PermissionsTest, ExplicitAccessToOrigin) {
 }
 
 TEST(PermissionsTest, CreateUnion) {
-  APIPermission* permission = nullptr;
-
   ManifestPermissionSet manifest_permissions;
   APIPermissionSet apis1;
   APIPermissionSet apis2;
@@ -346,7 +345,8 @@ TEST(PermissionsTest, CreateUnion) {
 
   const APIPermissionInfo* permission_info =
     PermissionsInfo::GetInstance()->GetByID(APIPermission::kSocket);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -358,10 +358,10 @@ TEST(PermissionsTest, CreateUnion) {
   // Union with an empty set.
   apis1.insert(APIPermission::kTab);
   apis1.insert(APIPermission::kBackground);
-  apis1.insert(permission->Clone());
+  apis1.insert(base::WrapUnique(permission->Clone()));
   expected_apis.insert(APIPermission::kTab);
   expected_apis.insert(APIPermission::kBackground);
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   AddPattern(&explicit_hosts1, "http://*.google.com/*");
   AddPattern(&expected_explicit_hosts, "http://*.google.com/*");
@@ -389,20 +389,20 @@ TEST(PermissionsTest, CreateUnion) {
   apis2.insert(APIPermission::kProxy);
   apis2.insert(APIPermission::kClipboardWrite);
 
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kTab);
   expected_apis.insert(APIPermission::kProxy);
   expected_apis.insert(APIPermission::kClipboardWrite);
 
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -412,7 +412,7 @@ TEST(PermissionsTest, CreateUnion) {
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
   // Insert a new permission socket permisssion which will replace the old one.
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   AddPattern(&explicit_hosts2, "http://*.example.com/*");
   AddPattern(&scriptable_hosts2, "http://*.google.com/*");
@@ -441,8 +441,6 @@ TEST(PermissionsTest, CreateUnion) {
 }
 
 TEST(PermissionsTest, CreateIntersection) {
-  APIPermission* permission = nullptr;
-
   ManifestPermissionSet manifest_permissions;
   APIPermissionSet apis1;
   APIPermissionSet apis2;
@@ -468,7 +466,8 @@ TEST(PermissionsTest, CreateIntersection) {
   // Intersection with an empty set.
   apis1.insert(APIPermission::kTab);
   apis1.insert(APIPermission::kBackground);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -476,7 +475,7 @@ TEST(PermissionsTest, CreateIntersection) {
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  apis1.insert(permission);
+  apis1.insert(std::move(permission));
 
   AddPattern(&explicit_hosts1, "http://*.google.com/*");
   AddPattern(&scriptable_hosts1, "http://www.reddit.com/*");
@@ -503,7 +502,7 @@ TEST(PermissionsTest, CreateIntersection) {
   apis2.insert(APIPermission::kTab);
   apis2.insert(APIPermission::kProxy);
   apis2.insert(APIPermission::kClipboardWrite);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
@@ -511,17 +510,17 @@ TEST(PermissionsTest, CreateIntersection) {
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kTab);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   AddPattern(&explicit_hosts2, "http://*.example.com/*");
   AddPattern(&explicit_hosts2, "http://*.google.com/*");
@@ -550,8 +549,6 @@ TEST(PermissionsTest, CreateIntersection) {
 }
 
 TEST(PermissionsTest, CreateDifference) {
-  APIPermission* permission = nullptr;
-
   ManifestPermissionSet manifest_permissions;
   APIPermissionSet apis1;
   APIPermissionSet apis2;
@@ -577,7 +574,8 @@ TEST(PermissionsTest, CreateDifference) {
   // Difference with an empty set.
   apis1.insert(APIPermission::kTab);
   apis1.insert(APIPermission::kBackground);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -585,7 +583,7 @@ TEST(PermissionsTest, CreateDifference) {
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  apis1.insert(permission);
+  apis1.insert(std::move(permission));
 
   AddPattern(&explicit_hosts1, "http://*.google.com/*");
   AddPattern(&scriptable_hosts1, "http://www.reddit.com/*");
@@ -601,24 +599,24 @@ TEST(PermissionsTest, CreateDifference) {
   apis2.insert(APIPermission::kTab);
   apis2.insert(APIPermission::kProxy);
   apis2.insert(APIPermission::kClipboardWrite);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kBackground);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), nullptr, nullptr));
   }
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   AddPattern(&explicit_hosts2, "http://*.example.com/*");
   AddPattern(&explicit_hosts2, "http://*.google.com/*");

extensions/common/extension_messages.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include <memory>
+#include <utility>
 
 #include "content/public/common/common_param_traits.h"
 #include "extensions/common/extension.h"
@@ -216,7 +217,7 @@ bool ParamTraits<APIPermissionSet>::Read(const base::Pickle* m,
     std::unique_ptr<APIPermission> p(permission_info->CreateAPIPermission());
     if (!p->Read(m, iter))
       return false;
-    r->insert(p.release());
+    r->insert(std::move(p));
   }
   return true;
 }
@@ -253,7 +254,7 @@ bool ParamTraits<ManifestPermissionSet>::Read(const base::Pickle* m,
       return false;
     if (!p->Read(m, iter))
       return false;
-    r->insert(p.release());
+    r->insert(std::move(p));
   }
   return true;
 }

extensions/common/manifest_handler.cc

@@ -182,10 +182,10 @@ void ManifestHandlerRegistry::AddExtensionInitialRequiredPermissions(
     const Extension* extension, ManifestPermissionSet* permission_set) {
   for (ManifestHandlerMap::const_iterator it = handlers_.begin();
       it != handlers_.end(); ++it) {
-    ManifestPermission* permission =
-        it->second->CreateInitialRequiredPermission(extension);
+    std::unique_ptr<ManifestPermission> permission(
+        it->second->CreateInitialRequiredPermission(extension));
     if (permission) {
-      permission_set->insert(permission);
+      permission_set->insert(std::move(permission));
     }
   }
 }

extensions/common/manifest_handlers/content_capabilities_handler.cc

@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ -110,7 +111,8 @@ bool ContentCapabilitiesHandler::Parse(Extension* extension,
           keys::kContentCapabilities,
           permission_name));
     } else {
-      info->permissions.insert(permission_info->CreateAPIPermission());
+      info->permissions.insert(
+          base::WrapUnique(permission_info->CreateAPIPermission()));
     }
   }
 

extensions/common/manifest_handlers/permissions_parser.cc

@@ -387,7 +387,8 @@ void PermissionsParser::AddAPIPermission(Extension* extension,
                                          APIPermission* permission) {
   DCHECK(extension->permissions_parser());
   extension->permissions_parser()
-      ->initial_required_permissions_->api_permissions.insert(permission);
+      ->initial_required_permissions_->api_permissions.insert(
+          base::WrapUnique(permission));
 }
 
 // static

extensions/common/permissions/api_permission_set.cc

@@ -5,6 +5,7 @@
 #include "extensions/common/permissions/api_permission_set.h"
 
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
@@ -68,7 +69,7 @@ bool CreateAPIPermission(
       }
       LOG(WARNING) << "Parse permission failed.";
     } else {
-      api_permissions->insert(permission.release());
+      api_permissions->insert(std::move(permission));
     }
     return true;
   }
@@ -132,11 +133,11 @@ void APIPermissionSet::insert(APIPermission::ID id) {
   const APIPermissionInfo* permission_info =
       PermissionsInfo::GetInstance()->GetByID(id);
   DCHECK(permission_info);
-  insert(permission_info->CreateAPIPermission());
+  insert(base::WrapUnique(permission_info->CreateAPIPermission()));
 }
 
-void APIPermissionSet::insert(APIPermission* permission) {
-  BaseSetOperators<APIPermissionSet>::insert(permission);
+void APIPermissionSet::insert(std::unique_ptr<APIPermission> permission) {
+  BaseSetOperators<APIPermissionSet>::insert(std::move(permission));
 }
 
 // static

extensions/common/permissions/api_permission_set.h

@@ -43,9 +43,8 @@ class APIPermissionSet : public BaseSetOperators<APIPermissionSet> {
 
   void insert(APIPermission::ID id);
 
-  // Insert |permission| into the APIPermissionSet. The APIPermissionSet will
-  // take the ownership of |permission|,
-  void insert(APIPermission* permission);
+  // Inserts |permission| into the APIPermissionSet.
+  void insert(std::unique_ptr<APIPermission> permission);
 
   // Parses permissions from |permissions| and adds the parsed permissions to
   // |api_permissions|. If |source| is kDisallowInternalPermissions, treat

extensions/common/permissions/api_permission_set_unittest.cc

@@ -33,8 +33,6 @@ TEST(APIPermissionSetTest, General) {
 }
 
 TEST(APIPermissionSetTest, CreateUnion) {
-  APIPermission* permission = NULL;
-
   APIPermissionSet apis1;
   APIPermissionSet apis2;
   APIPermissionSet expected_apis;
@@ -42,7 +40,8 @@ TEST(APIPermissionSetTest, CreateUnion) {
 
   const APIPermissionInfo* permission_info =
     PermissionsInfo::GetInstance()->GetByID(APIPermission::kSocket);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -54,10 +53,10 @@ TEST(APIPermissionSetTest, CreateUnion) {
   // Union with an empty set.
   apis1.insert(APIPermission::kAudioCapture);
   apis1.insert(APIPermission::kDns);
-  apis1.insert(permission->Clone());
+  apis1.insert(base::WrapUnique(permission->Clone()));
   expected_apis.insert(APIPermission::kAudioCapture);
   expected_apis.insert(APIPermission::kDns);
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   ASSERT_TRUE(apis2.empty());
   APIPermissionSet::Union(apis1, apis2, &result);
@@ -77,21 +76,21 @@ TEST(APIPermissionSetTest, CreateUnion) {
   apis2.insert(APIPermission::kPower);
   apis2.insert(APIPermission::kSerial);
 
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kAudioCapture);
   expected_apis.insert(APIPermission::kHid);
   expected_apis.insert(APIPermission::kPower);
   expected_apis.insert(APIPermission::kSerial);
 
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -101,7 +100,7 @@ TEST(APIPermissionSetTest, CreateUnion) {
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
   // Insert a new socket permission which will replace the old one.
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   APIPermissionSet::Union(apis1, apis2, &result);
 
@@ -116,8 +115,6 @@ TEST(APIPermissionSetTest, CreateUnion) {
 }
 
 TEST(APIPermissionSetTest, CreateIntersection) {
-  APIPermission* permission = NULL;
-
   APIPermissionSet apis1;
   APIPermissionSet apis2;
   APIPermissionSet expected_apis;
@@ -129,7 +126,8 @@ TEST(APIPermissionSetTest, CreateIntersection) {
   // Intersection with an empty set.
   apis1.insert(APIPermission::kAudioCapture);
   apis1.insert(APIPermission::kDns);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -137,7 +135,7 @@ TEST(APIPermissionSetTest, CreateIntersection) {
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis1.insert(permission);
+  apis1.insert(std::move(permission));
 
   ASSERT_TRUE(apis2.empty());
   APIPermissionSet::Intersection(apis1, apis2, &result);
@@ -157,7 +155,7 @@ TEST(APIPermissionSetTest, CreateIntersection) {
   apis2.insert(APIPermission::kHid);
   apis2.insert(APIPermission::kPower);
   apis2.insert(APIPermission::kSerial);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
@@ -165,17 +163,17 @@ TEST(APIPermissionSetTest, CreateIntersection) {
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kAudioCapture);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   APIPermissionSet::Intersection(apis1, apis2, &result);
 
@@ -190,8 +188,6 @@ TEST(APIPermissionSetTest, CreateIntersection) {
 }
 
 TEST(APIPermissionSetTest, CreateDifference) {
-  APIPermission* permission = NULL;
-
   APIPermissionSet apis1;
   APIPermissionSet apis2;
   APIPermissionSet expected_apis;
@@ -203,7 +199,8 @@ TEST(APIPermissionSetTest, CreateDifference) {
   // Difference with an empty set.
   apis1.insert(APIPermission::kAudioCapture);
   apis1.insert(APIPermission::kDns);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -211,7 +208,7 @@ TEST(APIPermissionSetTest, CreateDifference) {
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis1.insert(permission);
+  apis1.insert(std::move(permission));
 
   ASSERT_TRUE(apis2.empty());
   APIPermissionSet::Difference(apis1, apis2, &result);
@@ -223,24 +220,24 @@ TEST(APIPermissionSetTest, CreateDifference) {
   apis2.insert(APIPermission::kHid);
   apis2.insert(APIPermission::kPower);
   apis2.insert(APIPermission::kSerial);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
     value->AppendString("udp-send-to::8899");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis2.insert(permission);
+  apis2.insert(std::move(permission));
 
   expected_apis.insert(APIPermission::kDns);
-  permission = permission_info->CreateAPIPermission();
+  permission.reset(permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("udp-bind::8080");
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  expected_apis.insert(permission);
+  expected_apis.insert(std::move(permission));
 
   APIPermissionSet::Difference(apis1, apis2, &result);
 
@@ -256,8 +253,6 @@ TEST(APIPermissionSetTest, CreateDifference) {
 }
 
 TEST(APIPermissionSetTest, IPC) {
-  APIPermission* permission = NULL;
-
   APIPermissionSet apis;
   APIPermissionSet expected_apis;
 
@@ -266,7 +261,8 @@ TEST(APIPermissionSetTest, IPC) {
 
   apis.insert(APIPermission::kAudioCapture);
   apis.insert(APIPermission::kDns);
-  permission = permission_info->CreateAPIPermission();
+  std::unique_ptr<APIPermission> permission(
+      permission_info->CreateAPIPermission());
   {
     std::unique_ptr<base::ListValue> value(new base::ListValue());
     value->AppendString("tcp-connect:*.example.com:80");
@@ -274,7 +270,7 @@ TEST(APIPermissionSetTest, IPC) {
     value->AppendString("udp-send-to::8888");
     ASSERT_TRUE(permission->FromValue(value.get(), NULL, NULL));
   }
-  apis.insert(permission);
+  apis.insert(std::move(permission));
 
   EXPECT_NE(apis, expected_apis);
 

extensions/common/permissions/base_set_operators.h

@@ -9,8 +9,10 @@
 
 #include <iterator>
 #include <map>
+#include <memory>
 
 #include "base/memory/linked_ptr.h"
+#include "base/memory/ptr_util.h"
 
 namespace extensions {
 
@@ -29,7 +31,9 @@ class BaseSetOperators {
  public:
   typedef typename BaseSetOperatorsTraits<T>::ElementType ElementType;
   typedef typename BaseSetOperatorsTraits<T>::ElementIDType ElementIDType;
-  typedef std::map<ElementIDType, linked_ptr<ElementType> > Map;
+
+  // TODO(devlin): Un-link-ptr-ify this.
+  using Map = std::map<ElementIDType, linked_ptr<ElementType>>;
 
   class const_iterator :
     public std::iterator<std::input_iterator_tag, const ElementType*> {
@@ -94,7 +98,7 @@ class BaseSetOperators {
     const_iterator it = rhs.begin();
     const const_iterator end = rhs.end();
     while (it != end) {
-      insert(it->Clone());
+      insert(base::WrapUnique(it->Clone()));
       ++it;
     }
     return *static_cast<T*>(this);
@@ -152,21 +156,21 @@ class BaseSetOperators {
 
     while (it1 != end1 && it2 != end2) {
       if (it1->id() < it2->id()) {
-        set3->insert(it1->Clone());
+        set3->insert(base::WrapUnique(it1->Clone()));
         ++it1;
       } else if (it1->id() > it2->id()) {
         ++it2;
       } else {
         ElementType* p = it1->Diff(*it2);
         if (p)
-          set3->insert(p);
+          set3->insert(base::WrapUnique(p));
         ++it1;
         ++it2;
       }
     }
 
     while (it1 != end1) {
-      set3->insert(it1->Clone());
+      set3->insert(base::WrapUnique(it1->Clone()));
       ++it1;
     }
   }
@@ -188,7 +192,7 @@ class BaseSetOperators {
       } else {
         ElementType* p = it1->Intersect(*it2);
         if (p)
-          set3->insert(p);
+          set3->insert(base::WrapUnique(p));
         ++it1;
         ++it2;
       }
@@ -207,73 +211,55 @@ class BaseSetOperators {
     while (true) {
       if (it1 == end1) {
         while (it2 != end2) {
-          set3->insert(it2->Clone());
+          set3->insert(base::WrapUnique(it2->Clone()));
           ++it2;
         }
         break;
       }
       if (it2 == end2) {
         while (it1 != end1) {
-          set3->insert(it1->Clone());
+          set3->insert(base::WrapUnique(it1->Clone()));
           ++it1;
         }
         break;
       }
       if (it1->id() < it2->id()) {
-        set3->insert(it1->Clone());
+        set3->insert(base::WrapUnique(it1->Clone()));
         ++it1;
       } else if (it1->id() > it2->id()) {
-        set3->insert(it2->Clone());
+        set3->insert(base::WrapUnique(it2->Clone()));
         ++it2;
       } else {
-        set3->insert(it1->Union(*it2));
+        set3->insert(base::WrapUnique(it1->Union(*it2)));
         ++it1;
         ++it2;
       }
     }
   }
 
-  const_iterator begin() const {
-    return const_iterator(map().begin());
-  }
+  const_iterator begin() const { return const_iterator(map_.begin()); }
 
-  const_iterator end() const {
-    return map().end();
-  }
+  const_iterator end() const { return map_.end(); }
 
-  const_iterator find(ElementIDType id) const {
-    return map().find(id);
-  }
+  const_iterator find(ElementIDType id) const { return map_.find(id); }
 
-  size_t count(ElementIDType id) const {
-    return map().count(id);
-  }
+  size_t count(ElementIDType id) const { return map_.count(id); }
 
-  bool empty() const {
-    return map().empty();
-  }
+  bool empty() const { return map_.empty(); }
 
-  size_t erase(ElementIDType id) {
-    return map().erase(id);
-  }
+  size_t erase(ElementIDType id) { return map_.erase(id); }
 
-  // Take ownership and insert |item| into the set.
-  void insert(ElementType* item) {
-    map_[item->id()].reset(item);
+  void insert(std::unique_ptr<ElementType> item) {
+    ElementIDType id = item->id();
+    map_[id].reset(item.release());
   }
 
-  size_t size() const {
-    return map().size();
-  }
+  size_t size() const { return map_.size(); }
 
   const Map& map() const {
     return map_;
   }
 
-  Map& map() {
-    return map_;
-  }
-
   void clear() {
     map_.clear();
   }

extensions/common/permissions/manifest_permission_set.cc

@@ -48,7 +48,7 @@ bool CreateManifestPermission(
     LOG(WARNING) << "Parse permission failed.";
     return true;
   } else {
-    manifest_permissions->insert(permission.release());
+    manifest_permissions->insert(std::move(permission));
     return true;
   }
 }

extensions/common/permissions/manifest_permission_set_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "extensions/common/permissions/manifest_permission_set.h"
 
+#include "base/memory/ptr_util.h"
 #include "base/pickle.h"
 #include "base/values.h"
 #include "extensions/common/permissions/mock_manifest_permission.h"
@@ -14,11 +15,11 @@ namespace extensions {
 
 TEST(ManifestPermissionSetTest, General) {
   ManifestPermissionSet set;
-  set.insert(new MockManifestPermission("p1"));
-  set.insert(new MockManifestPermission("p2"));
-  set.insert(new MockManifestPermission("p3"));
-  set.insert(new MockManifestPermission("p4"));
-  set.insert(new MockManifestPermission("p5"));
+  set.insert(std::make_unique<MockManifestPermission>("p1"));
+  set.insert(std::make_unique<MockManifestPermission>("p2"));
+  set.insert(std::make_unique<MockManifestPermission>("p3"));
+  set.insert(std::make_unique<MockManifestPermission>("p4"));
+  set.insert(std::make_unique<MockManifestPermission>("p5"));
 
   EXPECT_EQ(set.find("p1")->id(), "p1");
   EXPECT_TRUE(set.find("p10") == set.end());
@@ -38,15 +39,15 @@ TEST(ManifestPermissionSetTest, CreateUnion) {
   ManifestPermissionSet expected_permissions;
   ManifestPermissionSet result;
 
-  ManifestPermission* permission = new MockManifestPermission("p3");
+  auto permission = std::make_unique<MockManifestPermission>("p3");
 
   // Union with an empty set.
-  permissions1.insert(new MockManifestPermission("p1"));
-  permissions1.insert(new MockManifestPermission("p2"));
-  permissions1.insert(permission->Clone());
-  expected_permissions.insert(new MockManifestPermission("p1"));
-  expected_permissions.insert(new MockManifestPermission("p2"));
-  expected_permissions.insert(permission);
+  permissions1.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p2"));
+  permissions1.insert(base::WrapUnique(permission->Clone()));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p1"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p2"));
+  expected_permissions.insert(std::move(permission));
 
   ManifestPermissionSet::Union(permissions1, permissions2, &result);
 
@@ -60,27 +61,27 @@ TEST(ManifestPermissionSetTest, CreateUnion) {
   EXPECT_EQ(expected_permissions, result);
 
   // Now use a real second set.
-  permissions2.insert(new MockManifestPermission("p1"));
-  permissions2.insert(new MockManifestPermission("p2"));
-  permissions2.insert(new MockManifestPermission("p33"));
-  permissions2.insert(new MockManifestPermission("p4"));
-  permissions2.insert(new MockManifestPermission("p5"));
-
-  expected_permissions.insert(new MockManifestPermission("p1"));
-  expected_permissions.insert(new MockManifestPermission("p2"));
-  expected_permissions.insert(new MockManifestPermission("p3"));
-  expected_permissions.insert(new MockManifestPermission("p4"));
-  expected_permissions.insert(new MockManifestPermission("p5"));
-  expected_permissions.insert(new MockManifestPermission("p33"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p2"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p33"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p4"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p5"));
+
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p1"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p2"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p3"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p4"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p5"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p33"));
 
   ManifestPermissionSet::Union(permissions1, permissions2, &result);
 
   {
     ManifestPermissionSet set1;
-    set1.insert(new MockManifestPermission("p1"));
-    set1.insert(new MockManifestPermission("p2"));
+    set1.insert(std::make_unique<MockManifestPermission>("p1"));
+    set1.insert(std::make_unique<MockManifestPermission>("p2"));
     ManifestPermissionSet set2;
-    set2.insert(new MockManifestPermission("p3"));
+    set2.insert(std::make_unique<MockManifestPermission>("p3"));
 
     EXPECT_FALSE(set1.Contains(set2));
     EXPECT_FALSE(set2.Contains(set1));
@@ -103,9 +104,9 @@ TEST(ManifestPermissionSetTest, CreateIntersection) {
   ManifestPermissionSet result;
 
   // Intersection with an empty set.
-  permissions1.insert(new MockManifestPermission("p1"));
-  permissions1.insert(new MockManifestPermission("p2"));
-  permissions1.insert(new MockManifestPermission("p3"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p2"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p3"));
 
   ManifestPermissionSet::Intersection(permissions1, permissions2, &result);
   EXPECT_TRUE(permissions1.Contains(result));
@@ -119,13 +120,13 @@ TEST(ManifestPermissionSetTest, CreateIntersection) {
   EXPECT_EQ(expected_permissions, result);
 
   // Now use a real second set.
-  permissions2.insert(new MockManifestPermission("p1"));
-  permissions2.insert(new MockManifestPermission("p3"));
-  permissions2.insert(new MockManifestPermission("p4"));
-  permissions2.insert(new MockManifestPermission("p5"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p3"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p4"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p5"));
 
-  expected_permissions.insert(new MockManifestPermission("p1"));
-  expected_permissions.insert(new MockManifestPermission("p3"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p1"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p3"));
 
   ManifestPermissionSet::Intersection(permissions1, permissions2, &result);
 
@@ -146,22 +147,22 @@ TEST(ManifestPermissionSetTest, CreateDifference) {
   ManifestPermissionSet result;
 
   // Difference with an empty set.
-  permissions1.insert(new MockManifestPermission("p1"));
-  permissions1.insert(new MockManifestPermission("p2"));
-  permissions1.insert(new MockManifestPermission("p3"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p2"));
+  permissions1.insert(std::make_unique<MockManifestPermission>("p3"));
 
   ManifestPermissionSet::Difference(permissions1, permissions2, &result);
 
   EXPECT_EQ(permissions1, result);
 
   // Now use a real second set.
-  permissions2.insert(new MockManifestPermission("p1"));
-  permissions2.insert(new MockManifestPermission("p2"));
-  permissions2.insert(new MockManifestPermission("p4"));
-  permissions2.insert(new MockManifestPermission("p5"));
-  permissions2.insert(new MockManifestPermission("p6"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p1"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p2"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p4"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p5"));
+  permissions2.insert(std::make_unique<MockManifestPermission>("p6"));
 
-  expected_permissions.insert(new MockManifestPermission("p3"));
+  expected_permissions.insert(std::make_unique<MockManifestPermission>("p3"));
 
   ManifestPermissionSet::Difference(permissions1, permissions2, &result);