Update GrantObjectPermission to take url::Origin

This change updates ChooserContextBase's GrantObjectPermission() method
to take url::Origins instead of GURLs. In the process the internal
GetWebsiteSetting() and SetWebsiteSetting() methods have also been
modified to take url::Origins.

This is part of a migration effort to remove GURL from the interface of
ChooserContextBase and its subclasses.

Bug: 951785
Change-Id: I88878374ca4c8c1d56cb6f05614fb76f70ac4f0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1579477Reviewed-by: Raymes Khoury <raymes@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653495}

chrome/browser/permissions/chooser_context_base.cc

@@ -79,7 +79,7 @@ ChooserContextBase::GetGrantedObjects(const GURL& requesting_origin_url,
 
   content_settings::SettingInfo info;
   base::Value setting =
-      GetWebsiteSetting(requesting_origin_url, embedding_origin_url, &info);
+      GetWebsiteSetting(requesting_origin, embedding_origin, &info);
 
   base::Value* objects = setting.FindListKey(kObjectListKey);
   if (!objects)
@@ -116,7 +116,7 @@ ChooserContextBase::GetAllGrantedObjects() {
 
     content_settings::SettingInfo info;
     base::Value setting =
-        GetWebsiteSetting(requesting_origin_url, embedding_origin_url, &info);
+        GetWebsiteSetting(requesting_origin, embedding_origin, &info);
     base::Value* objects = setting.FindListKey(kObjectListKey);
     if (!objects)
       continue;
@@ -135,11 +135,10 @@ ChooserContextBase::GetAllGrantedObjects() {
   return results;
 }
 
-void ChooserContextBase::GrantObjectPermission(const GURL& requesting_origin,
-                                               const GURL& embedding_origin,
-                                               base::Value object) {
-  DCHECK_EQ(requesting_origin, requesting_origin.GetOrigin());
-  DCHECK_EQ(embedding_origin, embedding_origin.GetOrigin());
+void ChooserContextBase::GrantObjectPermission(
+    const url::Origin& requesting_origin,
+    const url::Origin& embedding_origin,
+    base::Value object) {
   DCHECK(IsValidObject(object));
 
   base::Value setting =
@@ -158,11 +157,14 @@ void ChooserContextBase::GrantObjectPermission(const GURL& requesting_origin,
   NotifyPermissionChanged();
 }
 
-void ChooserContextBase::RevokeObjectPermission(const GURL& requesting_origin,
-                                                const GURL& embedding_origin,
-                                                const base::Value& object) {
-  DCHECK_EQ(requesting_origin, requesting_origin.GetOrigin());
-  DCHECK_EQ(embedding_origin, embedding_origin.GetOrigin());
+void ChooserContextBase::RevokeObjectPermission(
+    const GURL& requesting_origin_url,
+    const GURL& embedding_origin_url,
+    const base::Value& object) {
+  const auto requesting_origin = url::Origin::Create(requesting_origin_url);
+  DCHECK_EQ(requesting_origin_url, requesting_origin.GetURL());
+  const auto embedding_origin = url::Origin::Create(embedding_origin_url);
+  DCHECK_EQ(embedding_origin_url, embedding_origin.GetURL());
   DCHECK(IsValidObject(object));
 
   base::Value setting =
@@ -177,7 +179,7 @@ void ChooserContextBase::RevokeObjectPermission(const GURL& requesting_origin,
     objects->GetList().erase(it);
 
   SetWebsiteSetting(requesting_origin, embedding_origin, std::move(setting));
-  NotifyPermissionRevoked(requesting_origin, embedding_origin);
+  NotifyPermissionRevoked(requesting_origin_url, embedding_origin_url);
 }
 
 void ChooserContextBase::NotifyPermissionChanged() {
@@ -197,22 +199,23 @@ void ChooserContextBase::NotifyPermissionRevoked(const GURL& requesting_origin,
 }
 
 base::Value ChooserContextBase::GetWebsiteSetting(
-    const GURL& requesting_origin,
-    const GURL& embedding_origin,
+    const url::Origin& requesting_origin,
+    const url::Origin& embedding_origin,
     content_settings::SettingInfo* info) {
   std::unique_ptr<base::Value> value =
       host_content_settings_map_->GetWebsiteSetting(
-          requesting_origin, embedding_origin, data_content_settings_type_,
-          std::string(), info);
+          requesting_origin.GetURL(), embedding_origin.GetURL(),
+          data_content_settings_type_, std::string(), info);
   if (value)
     return base::Value::FromUniquePtrValue(std::move(value));
   return base::Value(base::Value::Type::DICTIONARY);
 }
 
-void ChooserContextBase::SetWebsiteSetting(const GURL& requesting_origin,
-                                           const GURL& embedding_origin,
+void ChooserContextBase::SetWebsiteSetting(const url::Origin& requesting_origin,
+                                           const url::Origin& embedding_origin,
                                            base::Value value) {
   host_content_settings_map_->SetWebsiteSettingDefaultScope(
-      requesting_origin, embedding_origin, data_content_settings_type_,
-      std::string(), base::Value::ToUniquePtrValue(std::move(value)));
+      requesting_origin.GetURL(), embedding_origin.GetURL(),
+      data_content_settings_type_, std::string(),
+      base::Value::ToUniquePtrValue(std::move(value)));
 }

chrome/browser/permissions/chooser_context_base.h

@@ -93,8 +93,8 @@ class ChooserContextBase : public KeyedService {
 
   // Grants |requesting_origin| access to |object| when embedded within
   // |embedding_origin| by writing it into |host_content_settings_map_|.
-  void GrantObjectPermission(const GURL& requesting_origin,
-                             const GURL& embedding_origin,
+  void GrantObjectPermission(const url::Origin& requesting_origin,
+                             const url::Origin& embedding_origin,
                              base::Value object);
 
   // Revokes |requesting_origin|'s permission to access |object| when embedded
@@ -121,11 +121,11 @@ class ChooserContextBase : public KeyedService {
   base::ObserverList<PermissionObserver> permission_observer_list_;
 
  private:
-  base::Value GetWebsiteSetting(const GURL& requesting_origin,
-                                const GURL& embedding_origin,
+  base::Value GetWebsiteSetting(const url::Origin& requesting_origin,
+                                const url::Origin& embedding_origin,
                                 content_settings::SettingInfo* info);
-  void SetWebsiteSetting(const GURL& requesting_origin,
-                         const GURL& embedding_origin,
+  void SetWebsiteSetting(const url::Origin& requesting_origin,
+                         const url::Origin& embedding_origin,
                          base::Value value);
 
   HostContentSettingsMap* const host_content_settings_map_;

chrome/browser/permissions/chooser_context_base_unittest.cc

@@ -37,8 +37,10 @@ class TestChooserContext : public ChooserContextBase {
 class ChooserContextBaseTest : public testing::Test {
  public:
   ChooserContextBaseTest()
-      : origin1_("https://google.com"),
-        origin2_("https://chromium.org"),
+      : url1_("https://google.com"),
+        url2_("https://chromium.org"),
+        origin1_(url::Origin::Create(url1_)),
+        origin2_(url::Origin::Create(url2_)),
         object1_(base::Value::Type::DICTIONARY),
         object2_(base::Value::Type::DICTIONARY) {
     object1_.SetStringKey(kRequiredKey1, "value1");
@@ -56,8 +58,10 @@ class ChooserContextBaseTest : public testing::Test {
   TestingProfile profile_;
 
  protected:
-  GURL origin1_;
-  GURL origin2_;
+  const GURL url1_;
+  const GURL url2_;
+  const url::Origin origin1_;
+  const url::Origin origin2_;
   base::Value object1_;
   base::Value object2_;
 };
@@ -72,24 +76,24 @@ TEST_F(ChooserContextBaseTest, GrantAndRevokeObjectPermissions) {
   context.GrantObjectPermission(origin1_, origin1_, object2_.Clone());
 
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects =
-      context.GetGrantedObjects(origin1_, origin1_);
+      context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(2u, objects.size());
   EXPECT_EQ(object1_, objects[0]->value);
   EXPECT_EQ(object2_, objects[1]->value);
 
   // Granting permission to one origin should not grant them to another.
-  objects = context.GetGrantedObjects(origin2_, origin2_);
+  objects = context.GetGrantedObjects(url2_, url2_);
   EXPECT_EQ(0u, objects.size());
 
   // Nor when the original origin is embedded in another.
-  objects = context.GetGrantedObjects(origin1_, origin2_);
+  objects = context.GetGrantedObjects(url1_, url2_);
   EXPECT_EQ(0u, objects.size());
 
   EXPECT_CALL(mock_observer, OnChooserObjectPermissionChanged(_, _)).Times(2);
-  EXPECT_CALL(mock_observer, OnPermissionRevoked(origin1_, origin1_)).Times(2);
-  context.RevokeObjectPermission(origin1_, origin1_, object1_);
-  context.RevokeObjectPermission(origin1_, origin1_, object2_);
-  objects = context.GetGrantedObjects(origin1_, origin1_);
+  EXPECT_CALL(mock_observer, OnPermissionRevoked(url1_, url1_)).Times(2);
+  context.RevokeObjectPermission(url1_, url1_, object1_);
+  context.RevokeObjectPermission(url1_, url1_, object2_);
+  objects = context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(0u, objects.size());
 }
 
@@ -103,14 +107,14 @@ TEST_F(ChooserContextBaseTest, GrantObjectPermissionTwice) {
   context.GrantObjectPermission(origin1_, origin1_, object1_.Clone());
 
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects =
-      context.GetGrantedObjects(origin1_, origin1_);
+      context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(1u, objects.size());
   EXPECT_EQ(object1_, objects[0]->value);
 
   EXPECT_CALL(mock_observer, OnChooserObjectPermissionChanged(_, _));
-  EXPECT_CALL(mock_observer, OnPermissionRevoked(origin1_, origin1_));
-  context.RevokeObjectPermission(origin1_, origin1_, object1_);
-  objects = context.GetGrantedObjects(origin1_, origin1_);
+  EXPECT_CALL(mock_observer, OnPermissionRevoked(url1_, url1_));
+  context.RevokeObjectPermission(url1_, url1_, object1_);
+  objects = context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(0u, objects.size());
 }
 
@@ -123,16 +127,16 @@ TEST_F(ChooserContextBaseTest, GrantObjectPermissionEmbedded) {
   context.GrantObjectPermission(origin1_, origin2_, object1_.Clone());
 
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects =
-      context.GetGrantedObjects(origin1_, origin2_);
+      context.GetGrantedObjects(url1_, url2_);
   EXPECT_EQ(1u, objects.size());
   EXPECT_EQ(object1_, objects[0]->value);
 
   // The embedding origin still does not have permission.
-  objects = context.GetGrantedObjects(origin2_, origin2_);
+  objects = context.GetGrantedObjects(url2_, url2_);
   EXPECT_EQ(0u, objects.size());
 
   // The requesting origin also doesn't have permission when not embedded.
-  objects = context.GetGrantedObjects(origin1_, origin1_);
+  objects = context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(0u, objects.size());
 }
 
@@ -151,14 +155,14 @@ TEST_F(ChooserContextBaseTest, GetAllGrantedObjects) {
   bool found_one = false;
   bool found_two = false;
   for (const auto& object : objects) {
-    if (object->requesting_origin == origin1_) {
+    if (object->requesting_origin == url1_) {
       EXPECT_FALSE(found_one);
-      EXPECT_EQ(origin1_, object->embedding_origin);
+      EXPECT_EQ(url1_, object->embedding_origin);
       EXPECT_EQ(object1_, objects[0]->value);
       found_one = true;
-    } else if (object->requesting_origin == origin2_) {
+    } else if (object->requesting_origin == url2_) {
       EXPECT_FALSE(found_two);
-      EXPECT_EQ(origin2_, object->embedding_origin);
+      EXPECT_EQ(url2_, object->embedding_origin);
       EXPECT_EQ(object2_, objects[1]->value);
       found_two = true;
     } else {
@@ -171,7 +175,7 @@ TEST_F(ChooserContextBaseTest, GetAllGrantedObjects) {
 
 TEST_F(ChooserContextBaseTest, GetGrantedObjectsWithGuardBlocked) {
   auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
-  map->SetContentSettingDefaultScope(origin1_, origin1_,
+  map->SetContentSettingDefaultScope(url1_, url1_,
                                      CONTENT_SETTINGS_TYPE_USB_GUARD,
                                      std::string(), CONTENT_SETTING_BLOCK);
 
@@ -184,18 +188,18 @@ TEST_F(ChooserContextBaseTest, GetGrantedObjectsWithGuardBlocked) {
   context.GrantObjectPermission(origin2_, origin2_, object2_.Clone());
 
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects1 =
-      context.GetGrantedObjects(origin1_, origin1_);
+      context.GetGrantedObjects(url1_, url1_);
   EXPECT_EQ(0u, objects1.size());
 
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects2 =
-      context.GetGrantedObjects(origin2_, origin2_);
+      context.GetGrantedObjects(url2_, url2_);
   ASSERT_EQ(1u, objects2.size());
   EXPECT_EQ(object2_, objects2[0]->value);
 }
 
 TEST_F(ChooserContextBaseTest, GetAllGrantedObjectsWithGuardBlocked) {
   auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
-  map->SetContentSettingDefaultScope(origin1_, origin1_,
+  map->SetContentSettingDefaultScope(url1_, url1_,
                                      CONTENT_SETTINGS_TYPE_USB_GUARD,
                                      std::string(), CONTENT_SETTING_BLOCK);
 
@@ -210,7 +214,7 @@ TEST_F(ChooserContextBaseTest, GetAllGrantedObjectsWithGuardBlocked) {
   std::vector<std::unique_ptr<ChooserContextBase::Object>> objects =
       context.GetAllGrantedObjects();
   ASSERT_EQ(1u, objects.size());
-  EXPECT_EQ(origin2_, objects[0]->requesting_origin);
-  EXPECT_EQ(origin2_, objects[0]->embedding_origin);
+  EXPECT_EQ(url2_, objects[0]->requesting_origin);
+  EXPECT_EQ(url2_, objects[0]->embedding_origin);
   EXPECT_EQ(object2_, objects[0]->value);
 }

chrome/browser/usb/usb_chooser_context.cc

@@ -395,7 +395,7 @@ void UsbChooserContext::GrantDevicePermission(
     const url::Origin& embedding_origin,
     const device::mojom::UsbDeviceInfo& device_info) {
   if (CanStorePersistentEntry(device_info)) {
-    GrantObjectPermission(requesting_origin.GetURL(), embedding_origin.GetURL(),
+    GrantObjectPermission(requesting_origin, embedding_origin,
                           DeviceInfoToValue(device_info));
   } else {
     ephemeral_devices_[std::make_pair(requesting_origin, embedding_origin)]