Implement unlimitedStorage content capability

chrome/browser/extensions/extension_special_storage_policy.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest_handlers/app_isolation_info.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/url_constants.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
+#include "extensions/common/manifest_handlers/content_capabilities_handler.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "storage/browser/quota/quota_manager.h"
 #include "storage/common/quota/quota_status_code.h"
 #include "storage/common/quota/quota_types.h"
 
 using content::BrowserThread;
 using extensions::APIPermission;
 using extensions::Extension;
 using storage::SpecialStoragePolicy;
 
@@ skipping 51 matching lines @@
 
 bool ExtensionSpecialStoragePolicy::IsStorageUnlimited(const GURL& origin) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUnlimitedStorage))
     return true;
 
   if (origin.SchemeIs(content::kChromeDevToolsScheme) &&
       origin.host() == chrome::kChromeUIDevToolsHost)
     return true;
 
   base::AutoLock locker(lock_);
-  return unlimited_extensions_.Contains(origin);
+  return unlimited_extensions_.Contains(origin) ||
+         content_capabilities_unlimited_extensions_.GrantsCapabilitiesTo(
+             origin);
 }
 
 bool ExtensionSpecialStoragePolicy::IsStorageSessionOnly(const GURL& origin) {
   if (cookie_settings_.get() == NULL)
     return false;
   return cookie_settings_->IsCookieSessionOnly(origin);
 }
 
 bool ExtensionSpecialStoragePolicy::CanQueryDiskSize(const GURL& origin) {
   base::AutoLock locker(lock_);
@@ skipping 35 matching lines @@
 ExtensionSpecialStoragePolicy::ExtensionsProtectingOrigin(
     const GURL& origin) {
   base::AutoLock locker(lock_);
   return protected_apps_.ExtensionsContaining(origin);
 }
 
 void ExtensionSpecialStoragePolicy::GrantRightsForExtension(
     const extensions::Extension* extension,
     content::BrowserContext* browser_context) {
   DCHECK(extension);
-  if (!(NeedsProtection(extension) ||
-        extension->permissions_data()->HasAPIPermission(
-            APIPermission::kUnlimitedStorage) ||
-        extension->permissions_data()->HasAPIPermission(
-            APIPermission::kFileBrowserHandler) ||
-        extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
-        extension->is_app())) {
-    return;
+
+  int change_flags = 0;
+  if (extensions::ContentCapabilitiesInfo::Get(extension)
+          .permissions.count(APIPermission::kUnlimitedStorage) > 0) {
+    content_capabilities_unlimited_extensions_.Add(extension);
+    change_flags |= SpecialStoragePolicy::STORAGE_UNLIMITED;
   }
 
-  int change_flags = 0;
-  {
+  if (NeedsProtection(extension) ||
+      extension->permissions_data()->HasAPIPermission(
+          APIPermission::kUnlimitedStorage) ||
+      extension->permissions_data()->HasAPIPermission(
+          APIPermission::kFileBrowserHandler) ||
+      extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
+      extension->is_app()) {

[not at google - send to devlin, 2014/12/12 21:03:06]

Bleh, I don't like this code. It's basically:

if (A || B || C || D || E) {
  if (A && A') ...;
  if (B) ...;
  if (C) ...;
  if (D) ...;
  if (E) ...;
}

The only reason for the highly redundant looking outer check is so that you only
need to Lock under slightly tighter conditions, but those "conditions" include
it being an app. I really don't see the point.

Which makes me realise that the change you made:

content_capabilities_unlimited_extensions_.Add(extension);

is, in theory, not thread safe. However, neither are some of the other methods
in this class (like HasSessionOnlyOrigins). How strange.

I vote:
1. Put the AutoLock(lock_) at the top of this method.
2. Delete the if (A || B || C || D || E) outer check.

[Ken Rockot(use gerrit already), 2014/12/12 21:10:17]

Doh, you're right. Moving the lock.

     base::AutoLock locker(lock_);
     if (NeedsProtection(extension) && protected_apps_.Add(extension))
       change_flags |= SpecialStoragePolicy::STORAGE_PROTECTED;
     // FIXME: Does GrantRightsForExtension imply |extension| is installed?
     if (extension->is_app())
       installed_apps_.Add(extension);
 
     if (extension->permissions_data()->HasAPIPermission(
             APIPermission::kUnlimitedStorage) &&
         unlimited_extensions_.Add(extension)) {
       if (extension->is_hosted_app())
         LogHostedAppUnlimitedStorageUsage(extension, browser_context);
-
       change_flags |= SpecialStoragePolicy::STORAGE_UNLIMITED;
     }
 
     if (extension->permissions_data()->HasAPIPermission(
             APIPermission::kFileBrowserHandler))
       file_handler_extensions_.Add(extension);
 
     if (extensions::AppIsolationInfo::HasIsolatedStorage(extension))
       isolated_extensions_.Add(extension);
   }
 
   if (change_flags) {
     NotifyGranted(Extension::GetBaseURLFromExtensionId(extension->id()),
                   change_flags);
   }
 }
 
 void ExtensionSpecialStoragePolicy::RevokeRightsForExtension(
     const extensions::Extension* extension) {
   DCHECK(extension);
-  if (!(NeedsProtection(extension) ||
-        extension->permissions_data()->HasAPIPermission(
-            APIPermission::kUnlimitedStorage) ||
-        extension->permissions_data()->HasAPIPermission(
-            APIPermission::kFileBrowserHandler) ||
-        extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
-        extension->is_app())) {
-    return;
+
+  int change_flags = 0;
+  if (extensions::ContentCapabilitiesInfo::Get(extension)
+          .permissions.count(APIPermission::kUnlimitedStorage) > 0) {
+    content_capabilities_unlimited_extensions_.Remove(extension);
+    change_flags |= SpecialStoragePolicy::STORAGE_UNLIMITED;
   }
-  int change_flags = 0;
-  {
+
+  if (NeedsProtection(extension) ||
+      extension->permissions_data()->HasAPIPermission(
+          APIPermission::kUnlimitedStorage) ||
+      extension->permissions_data()->HasAPIPermission(
+          APIPermission::kFileBrowserHandler) ||
+      extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
+      extension->is_app()) {

[not at google - send to devlin, 2014/12/12 21:03:06]

Here too.

     base::AutoLock locker(lock_);
     if (NeedsProtection(extension) && protected_apps_.Remove(extension))
       change_flags |= SpecialStoragePolicy::STORAGE_PROTECTED;
 
     if (extension->is_app())
       installed_apps_.Remove(extension);
 
     if (extension->permissions_data()->HasAPIPermission(
             APIPermission::kUnlimitedStorage) &&
         unlimited_extensions_.Remove(extension))
@@ skipping 14 matching lines @@
 }
 
 void ExtensionSpecialStoragePolicy::RevokeRightsForAllExtensions() {
   {
     base::AutoLock locker(lock_);
     protected_apps_.Clear();
     installed_apps_.Clear();
     unlimited_extensions_.Clear();
     file_handler_extensions_.Clear();
     isolated_extensions_.Clear();
+    content_capabilities_unlimited_extensions_.Clear();
   }
 
   NotifyCleared();
 }
 
 void ExtensionSpecialStoragePolicy::NotifyGranted(
     const GURL& origin,
     int change_flags) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     BrowserThread::PostTask(
@@ skipping 36 matching lines @@
 
 ExtensionSpecialStoragePolicy::SpecialCollection::~SpecialCollection() {
   STLDeleteValues(&cached_results_);
 }
 
 bool ExtensionSpecialStoragePolicy::SpecialCollection::Contains(
     const GURL& origin) {
   return !ExtensionsContaining(origin)->is_empty();
 }
 
+bool ExtensionSpecialStoragePolicy::SpecialCollection::GrantsCapabilitiesTo(
+    const GURL& origin) {
+  for (scoped_refptr<const Extension> extension : extensions_) {

[not at google - send to devlin, 2014/12/12 21:03:06]

Should this be content_capabilities_unlimited_extensions_?

(Also same thread safety question).

[Ken Rockot(use gerrit already), 2014/12/12 21:10:17]

No, we're in a different data structure here. There is only extensions_, and
this operation is meant to be called from within a held lock scope (which it
is).

+    if (extensions::ContentCapabilitiesInfo::Get(extension.get())
+            .url_patterns.MatchesURL(origin)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 const extensions::ExtensionSet*
 ExtensionSpecialStoragePolicy::SpecialCollection::ExtensionsContaining(
     const GURL& origin) {
   CachedResults::const_iterator found = cached_results_.find(origin);
   if (found != cached_results_.end())
     return found->second;
 
   extensions::ExtensionSet* result = new extensions::ExtensionSet();
   for (extensions::ExtensionSet::const_iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
@@ skipping 23 matching lines @@
 
 void ExtensionSpecialStoragePolicy::SpecialCollection::Clear() {
   ClearCache();
   extensions_.Clear();
 }
 
 void ExtensionSpecialStoragePolicy::SpecialCollection::ClearCache() {
   STLDeleteValues(&cached_results_);
   cached_results_.clear();
 }