[DurableStorage] Don't grant durable if origin cannot write cookies.

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 7eac71f09f7bd38afc7452a8ef6e209229900b31..7f6a7602a8149c98c6eea0e0cb66831955d6ae45 100644
--- a/chrome/browser/storage/durable_storage_permission_context.cc
+++ b/chrome/browser/storage/durable_storage_permission_context.cc
@@ -8,11 +8,13 @@
 
 #include "base/logging.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/browser/website_settings_registry.h"
 #include "content/public/browser/browser_thread.h"
@@ -38,8 +40,31 @@ void DurableStoragePermissionContext::DecidePermission(
     const BrowserPermissionCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

[michaeln, 2016/10/05 00:22:40]

A comment about how this is only expected to be called when the permission has
not yet be granted might be good. Otherwise it looks possible for a top level
frame and a nested cross origin iframe to get different answers for the same
origin and the same time, which wouldn't make sense.

Better yet, DCHECKs would speak even louder than a comment.

DCHECK_NEQ(ALLOWED, GetPermissionStatus(requesting_origin, embedding_origin));
DCHECK_NEQ(BLOCKED, GetPermissionStatus(requesting_origin, embedding_origin));

[dmurph, 2016/10/05 03:22:36]

Done.

 
-  // TODO(dgrogan): Remove bookmarks check in favor of site engagement. In the
-  // meantime maybe grant permission to A2HS origins as well.
+  // Don't grant durable if we're off the record.
+  if (profile()->IsOffTheRecord()) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);
+    return;
+  }
+
+  // Durable is only allowed to be granted on top-level origins.
+  if (requesting_origin != embedding_origin) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);
+    return;
+  }
+
+  // Don't grant durable if we can't write cookies.
+  scoped_refptr<content_settings::CookieSettings> cookie_settings =
+      CookieSettingsFactory::GetForProfile(profile());
+  if (!cookie_settings->IsSettingCookieAllowed(requesting_origin,
+                                               requesting_origin)) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);

[michaeln, 2016/10/05 00:22:40]

Yup, DEFAULT seems right here, as a sticky BLOCK setting would otherwise prevent
future cookie setting changes from allowing durability.

[dmurph, 2016/10/05 03:22:36]

Acknowledged.

+    return;
+  }
+
+  // TODO(dmurph): Remove bookmarks check in favor of important sites.
   BookmarkModel* model =
       BookmarkModelFactory::GetForBrowserContextIfExists(profile());
   if (model) {