Permissions: Move common logic from DecidePermission to RequestPermission

chrome/browser/storage/durable_storage_permission_context.cc

Index: chrome/browser/storage/durable_storage_permission_context.cc
diff --git a/chrome/browser/storage/durable_storage_permission_context.cc b/chrome/browser/storage/durable_storage_permission_context.cc
index 4e520bb320accda943fa511a21696a37062fb87e..c937268a7f34f2571e911fb5e3ceee62f16b69f5 100644
--- a/chrome/browser/storage/durable_storage_permission_context.cc
+++ b/chrome/browser/storage/durable_storage_permission_context.cc
@@ -33,45 +33,8 @@ void DurableStoragePermissionContext::DecidePermission(
     const GURL& embedding_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
-  // TODO(dgrogan): Reuse the base class's implementation of everything from
-  // here to using bookmarks.
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  if (!requesting_origin.is_valid() || !embedding_origin.is_valid()) {
-    std::string type_name =
-        content_settings::WebsiteSettingsRegistry::GetInstance()
-            ->Get(CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)
-            ->name();
-
-    DVLOG(1) << "Attempt to use " << type_name
-             << " from an invalid URL: " << requesting_origin << ","
-             << embedding_origin << " (" << type_name
-             << " is not supported in popups)";
-    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                        false /* persist */, CONTENT_SETTING_BLOCK);
-    return;
-  }
-
-  if (IsRestrictedToSecureOrigins() &&
-      !content::IsOriginSecure(requesting_origin)) {
-    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                        false /* persist */, CONTENT_SETTING_BLOCK);
-    return;
-  }
-
-  ContentSetting content_setting =
-      HostContentSettingsMapFactory::GetForProfile(profile())
-          ->GetContentSettingAndMaybeUpdateLastUsage(
-              requesting_origin, embedding_origin,
-              CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, std::string());
-
-  DCHECK_NE(CONTENT_SETTING_BLOCK, content_setting);
-  if (content_setting == CONTENT_SETTING_ALLOW) {
-    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                        false /* persist */, content_setting);
-    return;
-  }
-
   // TODO(dgrogan): Remove bookmarks check in favor of site engagement. In the
   // meantime maybe grant permission to A2HS origins as well.
   BookmarkModel* model = BookmarkModelFactory::GetForProfileIfExists(profile());
@@ -95,8 +58,7 @@ void DurableStoragePermissionContext::UpdateContentSetting(
     ContentSetting content_setting) {
   DCHECK_EQ(requesting_origin, requesting_origin.GetOrigin());
   DCHECK_EQ(embedding_origin_ignored, embedding_origin_ignored.GetOrigin());
-  DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
-         content_setting == CONTENT_SETTING_BLOCK);
+  DCHECK(content_setting == CONTENT_SETTING_ALLOW);

[mlamouri (slow - plz ping), 2015/11/26 12:45:49]

Why are you doing this change? This method was overridden because they want the
permission to not be scoped by the embedder.

[johnme, 2015/11/26 18:28:48]

They had a `DCHECK_NE(CONTENT_SETTING_BLOCK, content_setting);` in the
DecidePermission, and I was trying to preserve the DCHECK for that invariant.
But I realise this is wrong, as earlier on DecidePermission does sometimes block
the permission. So I guess I'll just remove this (though unfortunately, that
means the invariant is no longer checked).

 
   HostContentSettingsMapFactory::GetForProfile(profile())->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(requesting_origin),