[Extensions] Make PermissionSet ctor take URLPatternSet by value

Make the PermissionSet constructor accept URLPatternSet parameters by
value, enabling std::move()ing the parameters rather than necessitating
a copy. Update call sites that are easily modified.

Bug: 908536
Change-Id: I80ce85497b3272285a5b2a2299f8dc153530df51
Reviewed-on: https://chromium-review.googlesource.com/c/1383537
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635210}

chrome/browser/chromeos/extensions/permissions_updater_delegate_chromeos_unittest.cc

@@ -53,7 +53,8 @@ std::unique_ptr<const PermissionSet> CreatePermissions(
   URLPatternSet scriptable_hosts({
     URLPattern(URLPattern::SCHEME_ALL, "http://www.wikipedia.com/*")});
   auto permissions = std::make_unique<const PermissionSet>(
-      std::move(apis), std::move(manifest), explicit_hosts, scriptable_hosts);
+      std::move(apis), std::move(manifest), std::move(explicit_hosts),
+      std::move(scriptable_hosts));
   return permissions;
 }
 

chrome/browser/extensions/active_tab_permission_granter.cc

@@ -154,7 +154,7 @@ void ActiveTabPermissionGranter::GrantIfRequested(const Extension* extension) {
   if (!new_apis.empty() || !new_hosts.is_empty()) {
     granted_extensions_.Insert(extension);
     PermissionSet new_permissions(std::move(new_apis), ManifestPermissionSet(),
-                                  new_hosts, new_hosts);
+                                  new_hosts.Clone(), new_hosts.Clone());
     permissions_data->UpdateTabSpecificPermissions(tab_id_, new_permissions);
     content::NavigationEntry* navigation_entry =
         web_contents()->GetController().GetVisibleEntry();

chrome/browser/extensions/api/developer_private/developer_private_api.cc

@@ -2014,7 +2014,8 @@ DeveloperPrivateAddHostPermissionFunction::Run() {
       .GrantRuntimePermissions(
           *extension,
           PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                        new_host_permissions, new_host_permissions),
+                        new_host_permissions.Clone(),
+                        new_host_permissions.Clone()),
           base::BindOnce(&DeveloperPrivateAddHostPermissionFunction::
                              OnRuntimePermissionsGranted,
                          base::RetainedRef(this)));
@@ -2054,7 +2055,8 @@ DeveloperPrivateRemoveHostPermissionFunction::Run() {
   std::unique_ptr<const PermissionSet> permissions_to_remove =
       PermissionSet::CreateIntersection(
           PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                        host_permissions_to_remove, host_permissions_to_remove),
+                        host_permissions_to_remove.Clone(),
+                        host_permissions_to_remove.Clone()),
           *scripting_modifier.GetRevokablePermissions(),
           URLPatternSet::IntersectionBehavior::kDetailed);
   if (permissions_to_remove->IsEmpty())

chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc

@@ -1644,8 +1644,8 @@ TEST_F(DeveloperPrivateApiUnitTest,
 
   URLPatternSet hosts({URLPattern(Extension::kValidHostPermissionSchemes,
                                   "https://example.com/*")});
-  PermissionSet permissions(APIPermissionSet(), ManifestPermissionSet(), hosts,
-                            hosts);
+  PermissionSet permissions(APIPermissionSet(), ManifestPermissionSet(),
+                            hosts.Clone(), hosts.Clone());
   permissions_test_util::GrantRuntimePermissionsAndWaitForCompletion(
       profile(), *extension, permissions);
 

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

@@ -163,7 +163,7 @@ ExtensionFunction::ResponseAction PermissionsRemoveFunction::Run() {
 
   PermissionSet permissions(
       std::move(unpack_result->optional_apis), ManifestPermissionSet(),
-      unpack_result->optional_explicit_hosts, URLPatternSet());
+      std::move(unpack_result->optional_explicit_hosts), URLPatternSet());
 
   // Only try and remove those permissions that are active on the extension.
   // For backwards compatability with behavior before this check was added, just
@@ -255,15 +255,15 @@ ExtensionFunction::ResponseAction PermissionsRequestFunction::Run() {
   // are "new", i.e. aren't already active on the extension.
   requested_optional_ = std::make_unique<const PermissionSet>(
       std::move(unpack_result->optional_apis), ManifestPermissionSet(),
-      unpack_result->optional_explicit_hosts, URLPatternSet());
+      std::move(unpack_result->optional_explicit_hosts), URLPatternSet());
   requested_optional_ =
       PermissionSet::CreateDifference(*requested_optional_, active_permissions);
 
   // Do the same for withheld permissions.
   requested_withheld_ = std::make_unique<const PermissionSet>(
       APIPermissionSet(), ManifestPermissionSet(),
-      unpack_result->required_explicit_hosts,
-      unpack_result->required_scriptable_hosts);
+      std::move(unpack_result->required_explicit_hosts),
+      std::move(unpack_result->required_scriptable_hosts));
   requested_withheld_ =
       PermissionSet::CreateDifference(*requested_withheld_, active_permissions);
 

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

@@ -44,9 +44,9 @@ TEST(ExtensionPermissionsAPIHelpers, Pack) {
        URLPattern(UserScript::ValidUserScriptSchemes(), "http://d.com/*")});
 
   // Pack the permission set to value and verify its contents.
-  std::unique_ptr<Permissions> pack_result(
-      PackPermissionSet(PermissionSet(std::move(apis), ManifestPermissionSet(),
-                                      explicit_hosts, scriptable_hosts)));
+  std::unique_ptr<Permissions> pack_result(PackPermissionSet(
+      PermissionSet(std::move(apis), ManifestPermissionSet(),
+                    std::move(explicit_hosts), std::move(scriptable_hosts))));
   ASSERT_TRUE(pack_result);
   ASSERT_TRUE(pack_result->permissions);
   EXPECT_THAT(*pack_result->permissions,
@@ -75,9 +75,9 @@ TEST(ExtensionPermissionsAPIHelpers, Unpack_Basic) {
   optional_apis.insert(APIPermission::kTab);
   URLPatternSet optional_explicit_hosts(
       {URLPattern(Extension::kValidHostPermissionSchemes, "http://a.com/*")});
-  PermissionSet optional_permissions(std::move(optional_apis),
-                                     ManifestPermissionSet(),
-                                     optional_explicit_hosts, URLPatternSet());
+  PermissionSet optional_permissions(
+      std::move(optional_apis), ManifestPermissionSet(),
+      std::move(optional_explicit_hosts), URLPatternSet());
 
   // Origins shouldn't have to be present.
   {
@@ -229,11 +229,11 @@ TEST(ExtensionPermissionsAPIHelpers, Unpack_HostSeparation) {
   });
 
   PermissionSet required_permissions(
-      APIPermissionSet(), ManifestPermissionSet(), required_explicit_hosts,
-      required_scriptable_hosts);
-  PermissionSet optional_permissions(APIPermissionSet(),
-                                     ManifestPermissionSet(),
-                                     optional_explicit_hosts, URLPatternSet());
+      APIPermissionSet(), ManifestPermissionSet(),
+      std::move(required_explicit_hosts), std::move(required_scriptable_hosts));
+  PermissionSet optional_permissions(
+      APIPermissionSet(), ManifestPermissionSet(),
+      std::move(optional_explicit_hosts), URLPatternSet());
 
   Permissions permissions_object;
   permissions_object.origins =

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

@@ -294,7 +294,7 @@ TEST_F(PermissionsAPIUnitTest, ContainsAndGetAllWithRuntimeHostPermissions) {
     permissions_test_util::GrantRuntimePermissionsAndWaitForCompletion(
         profile(), *extension,
         PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                      explicit_hosts, URLPatternSet()));
+                      std::move(explicit_hosts), URLPatternSet()));
     const GURL example_url("https://example.com");
     const PermissionSet& active_permissions =
         extension->permissions_data()->active_permissions();

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

@@ -89,8 +89,8 @@ IN_PROC_BROWSER_TEST_F(PermissionsApiTest, OptionalPermissionsGranted) {
   ExtensionPrefs* prefs = ExtensionPrefs::Get(browser()->profile());
   prefs->AddRuntimeGrantedPermissions(
       "kjmkgkdkpedkejedfhmfcenooemhbpbo",
-      PermissionSet(std::move(apis), ManifestPermissionSet(), explicit_hosts,
-                    URLPatternSet()));
+      PermissionSet(std::move(apis), ManifestPermissionSet(),
+                    std::move(explicit_hosts), URLPatternSet()));
 
   PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);
   ASSERT_TRUE(StartEmbeddedTestServer());

chrome/browser/extensions/extension_prefs_unittest.cc

@@ -204,8 +204,6 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
     AddPattern(&shost_permissions_, "http://somesite.com/*");
     AddPattern(&shost_permissions_, "http://example.com/*");
 
-    URLPatternSet empty_extent;
-
     // Make sure both granted api and host permissions start empty.
     EXPECT_TRUE(prefs()->GetGrantedPermissions(extension_id_)->IsEmpty());
 
@@ -214,7 +212,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
       prefs()->AddGrantedPermissions(
           extension_id_,
           PermissionSet(api_perm_set1_.Clone(), ManifestPermissionSet(),
-                        empty_extent, empty_extent));
+                        URLPatternSet(), URLPatternSet()));
       std::unique_ptr<const PermissionSet> granted_permissions =
           prefs()->GetGrantedPermissions(extension_id_);
       EXPECT_TRUE(granted_permissions.get());
@@ -228,7 +226,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
       prefs()->AddGrantedPermissions(
           extension_id_,
           PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                        ehost_perm_set1_, empty_extent));
+                        ehost_perm_set1_.Clone(), URLPatternSet()));
       std::unique_ptr<const PermissionSet> granted_permissions =
           prefs()->GetGrantedPermissions(extension_id_);
       EXPECT_FALSE(granted_permissions->IsEmpty());
@@ -242,7 +240,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
       prefs()->AddGrantedPermissions(
           extension_id_,
           PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                        empty_extent, shost_perm_set1_));
+                        URLPatternSet(), shost_perm_set1_.Clone()));
       std::unique_ptr<const PermissionSet> granted_permissions =
           prefs()->GetGrantedPermissions(extension_id_);
       EXPECT_FALSE(granted_permissions->IsEmpty());
@@ -261,7 +259,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest {
       prefs()->AddGrantedPermissions(
           extension_id_,
           PermissionSet(api_perm_set2_.Clone(), ManifestPermissionSet(),
-                        ehost_perm_set2_, shost_perm_set2_));
+                        ehost_perm_set2_.Clone(), shost_perm_set2_.Clone()));
 
       std::unique_ptr<const PermissionSet> granted_permissions =
           prefs()->GetGrantedPermissions(extension_id_);
@@ -309,22 +307,25 @@ class ExtensionPrefsActivePermissions : public ExtensionPrefsTest {
   void Initialize() override {
     extension_id_ = prefs_.AddExtensionAndReturnId("test");
 
-    APIPermissionSet api_perms;
-    api_perms.insert(APIPermission::kTab);
-    api_perms.insert(APIPermission::kBookmark);
-    api_perms.insert(APIPermission::kHistory);
-
-    URLPatternSet ehosts;
-    AddPattern(&ehosts, "http://*.google.com/*");
-    AddPattern(&ehosts, "http://example.com/*");
-    AddPattern(&ehosts, "chrome://favicon/*");
-
-    URLPatternSet shosts;
-    AddPattern(&shosts, "https://*.google.com/*");
-    AddPattern(&shosts, "http://reddit.com/r/test/*");
-
-    active_perms_.reset(new PermissionSet(
-        api_perms.Clone(), ManifestPermissionSet(), ehosts, shosts));
+    {
+      APIPermissionSet api_perms;
+      api_perms.insert(APIPermission::kTab);
+      api_perms.insert(APIPermission::kBookmark);
+      api_perms.insert(APIPermission::kHistory);
+
+      URLPatternSet ehosts;
+      AddPattern(&ehosts, "http://*.google.com/*");
+      AddPattern(&ehosts, "http://example.com/*");
+      AddPattern(&ehosts, "chrome://favicon/*");
+
+      URLPatternSet shosts;
+      AddPattern(&shosts, "https://*.google.com/*");
+      AddPattern(&shosts, "http://reddit.com/r/test/*");
+
+      active_perms_.reset(
+          new PermissionSet(std::move(api_perms), ManifestPermissionSet(),
+                            std::move(ehosts), std::move(shosts)));
+    }
 
     // Make sure the active permissions start empty.
     std::unique_ptr<const PermissionSet> active =
@@ -955,11 +956,12 @@ class ExtensionPrefsComponentExtension : public ExtensionPrefsTest {
     api_perms.insert(APIPermission::kBookmark);
     api_perms.insert(APIPermission::kHistory);
 
-    URLPatternSet ehosts, shosts;
+    URLPatternSet shosts;
     AddPattern(&shosts, "chrome://print/*");
 
-    active_perms_.reset(new PermissionSet(
-        std::move(api_perms), ManifestPermissionSet(), ehosts, shosts));
+    active_perms_.reset(new PermissionSet(std::move(api_perms),
+                                          ManifestPermissionSet(),
+                                          URLPatternSet(), std::move(shosts)));
     // Set the active permissions.
     prefs()->SetActivePermissions(component_extension_->id(), *active_perms_);
     prefs()->SetActivePermissions(no_component_extension_->id(),
@@ -1031,8 +1033,8 @@ class ExtensionPrefsRuntimeGrantedPermissions : public ExtensionPrefsTest {
       // correctly added.
       URLPatternSet added_urls({example_com, chromium_org});
       PermissionSet added_permissions(APIPermissionSet(),
-                                      ManifestPermissionSet(), added_urls,
-                                      URLPatternSet());
+                                      ManifestPermissionSet(),
+                                      std::move(added_urls), URLPatternSet());
       prefs()->AddRuntimeGrantedPermissions(extension_a_->id(),
                                             added_permissions);
 
@@ -1046,16 +1048,16 @@ class ExtensionPrefsRuntimeGrantedPermissions : public ExtensionPrefsTest {
       // Remove one of the hosts. The only remaining host should be
       // example.com
       URLPatternSet removed_urls({chromium_org});
-      PermissionSet removed_permissions(APIPermissionSet(),
-                                        ManifestPermissionSet(), removed_urls,
-                                        URLPatternSet());
+      PermissionSet removed_permissions(
+          APIPermissionSet(), ManifestPermissionSet(), std::move(removed_urls),
+          URLPatternSet());
       prefs()->RemoveRuntimeGrantedPermissions(extension_a_->id(),
                                                removed_permissions);
 
       URLPatternSet remaining_urls({example_com});
-      PermissionSet remaining_permissions(APIPermissionSet(),
-                                          ManifestPermissionSet(),
-                                          remaining_urls, URLPatternSet());
+      PermissionSet remaining_permissions(
+          APIPermissionSet(), ManifestPermissionSet(),
+          std::move(remaining_urls), URLPatternSet());
       std::unique_ptr<const PermissionSet> retrieved_permissions =
           prefs()->GetRuntimeGrantedPermissions(extension_a_->id());
       ASSERT_TRUE(retrieved_permissions);
@@ -1073,9 +1075,9 @@ class ExtensionPrefsRuntimeGrantedPermissions : public ExtensionPrefsTest {
       // permission.
       URLPattern example_com(URLPattern::SCHEME_ALL, "https://example.com/*");
       URLPatternSet remaining_urls({example_com});
-      PermissionSet remaining_permissions(APIPermissionSet(),
-                                          ManifestPermissionSet(),
-                                          remaining_urls, URLPatternSet());
+      PermissionSet remaining_permissions(
+          APIPermissionSet(), ManifestPermissionSet(),
+          std::move(remaining_urls), URLPatternSet());
       std::unique_ptr<const PermissionSet> retrieved_permissions =
           prefs()->GetRuntimeGrantedPermissions(extension_a_->id());
       ASSERT_TRUE(retrieved_permissions);

chrome/browser/extensions/permissions_updater.cc

@@ -360,7 +360,7 @@ void PermissionsUpdater::RevokeRuntimePermissions(
         active_permissions_to_remove->apis().Clone(),
         active_permissions_to_remove->manifest_permissions().Clone(),
         URLPatternSet(removable_explicit_hosts),
-        active_permissions_to_remove->scriptable_hosts());
+        active_permissions_to_remove->scriptable_hosts().Clone());
   }
 
   CHECK(extension.permissions_data()->active_permissions().Contains(
@@ -537,7 +537,8 @@ void PermissionsUpdater::SetPermissions(
   std::unique_ptr<const PermissionSet> new_withheld =
       PermissionSet::CreateDifference(
           PermissionSet(APIPermissionSet(), ManifestPermissionSet(),
-                        required.explicit_hosts(), required.scriptable_hosts()),
+                        required.explicit_hosts().Clone(),
+                        required.scriptable_hosts().Clone()),
           *new_active);
 
   extension->permissions_data()->SetPermissions(std::move(new_active),

chrome/browser/extensions/permissions_updater_unittest.cc

@@ -173,8 +173,8 @@ TEST_F(PermissionsUpdaterTest, GrantAndRevokeOptionalPermissions) {
   URLPatternSet default_hosts;
   AddPattern(&default_hosts, "http://a.com/*");
   PermissionSet default_permissions(default_apis.Clone(),
-                                    ManifestPermissionSet(), default_hosts,
-                                    URLPatternSet());
+                                    ManifestPermissionSet(),
+                                    std::move(default_hosts), URLPatternSet());
 
   // Make sure it loaded properly.
   ASSERT_EQ(default_permissions,
@@ -191,7 +191,7 @@ TEST_F(PermissionsUpdaterTest, GrantAndRevokeOptionalPermissions) {
   AddPattern(&hosts, "http://*.c.com/*");
 
   {
-    PermissionSet delta(apis.Clone(), ManifestPermissionSet(), hosts,
+    PermissionSet delta(apis.Clone(), ManifestPermissionSet(), hosts.Clone(),
                         URLPatternSet());
 
     PermissionsUpdaterListener listener;
@@ -225,7 +225,7 @@ TEST_F(PermissionsUpdaterTest, GrantAndRevokeOptionalPermissions) {
     // In the second part of the test, we'll remove the permissions that we
     // just added except for 'notifications'.
     apis.erase(APIPermission::kNotifications);
-    PermissionSet delta(apis.Clone(), ManifestPermissionSet(), hosts,
+    PermissionSet delta(apis.Clone(), ManifestPermissionSet(), hosts.Clone(),
                         URLPatternSet());
 
     PermissionsUpdaterListener listener;
@@ -552,9 +552,9 @@ TEST_F(PermissionsUpdaterTest,
 
   URLPatternSet explicit_hosts({URLPattern(
       Extension::kValidHostPermissionSchemes, "https://example.com/*")});
-  PermissionSet runtime_granted_permissions(APIPermissionSet(),
-                                            ManifestPermissionSet(),
-                                            explicit_hosts, URLPatternSet());
+  PermissionSet runtime_granted_permissions(
+      APIPermissionSet(), ManifestPermissionSet(), std::move(explicit_hosts),
+      URLPatternSet());
 
   // Granting runtime-granted permissions should update the runtime granted
   // permissions store in preferences, but *not* granted permissions in
@@ -620,9 +620,9 @@ TEST_F(PermissionsUpdaterTest, RevokingPermissionsWithRuntimeHostPermissions) {
 
     URLPatternSet url_pattern_set;
     url_pattern_set.AddOrigin(URLPattern::SCHEME_ALL, kOrigin);
-    const PermissionSet permission_set(APIPermissionSet(),
-                                       ManifestPermissionSet(), url_pattern_set,
-                                       URLPatternSet());
+    const PermissionSet permission_set(
+        APIPermissionSet(), ManifestPermissionSet(), std::move(url_pattern_set),
+        URLPatternSet());
     // Give the extension access to the test site. Now, the test site permission
     // should be revokable.
     permissions_test_util::GrantRuntimePermissionsAndWaitForCompletion(