Use [SecureContext] for navigator.storage

third_party/WebKit/Source/modules/quota/StorageManager.cpp

Index: third_party/WebKit/Source/modules/quota/StorageManager.cpp
diff --git a/third_party/WebKit/Source/modules/quota/StorageManager.cpp b/third_party/WebKit/Source/modules/quota/StorageManager.cpp
index 2830ea3e66798d6857bea3b36829d6824a5ff90e..11bf37797461b0eb18e2d78808a5c157b111312a 100644
--- a/third_party/WebKit/Source/modules/quota/StorageManager.cpp
+++ b/third_party/WebKit/Source/modules/quota/StorageManager.cpp
@@ -60,19 +60,15 @@ ScriptPromise StorageManager::persist(ScriptState* scriptState) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
   ExecutionContext* executionContext = scriptState->getExecutionContext();
+  DCHECK(executionContext->isSecureContext());  // [SecureContext] in IDL
   SecurityOrigin* securityOrigin = executionContext->getSecurityOrigin();
-  // TODO(dgrogan): Is the isUnique() check covered by the later
-  // isSecureContext() check? If so, maybe remove it. Write a test if it
-  // stays.
+  // TODO(jsbell): Is the isUnique() check covered by [SecureContext]? If so,

[jsbell, 2017/02/11 01:01:43]

I'm keeping this TODO but maybe mkwst@ has an answer? 

Tracing through the code, the isPotentiallyTrustworthy() check would allow
unique origins that have had setUniqueOriginIsPotentiallyTrustworthy() called,
which apparently happens sometimes (e.g. Document::initSecurityContext). If I'm
reading the code (and [HTML]) correctly, you can get a secure context for a
sandboxed iframe.

The Storage spec says nothing about unique origins, but they make no sense as
far as storage-related APIs are concerned. Our existing storage APIs block them
as this code does. Maybe we want another attribute to restrict this from being
used in unique origins?

+  // maybe remove it. Write a test if it stays.
   if (securityOrigin->isUnique()) {
     resolver->reject(DOMException::create(NotSupportedError));
     return promise;
   }
-  String errorMessage;
-  if (!executionContext->isSecureContext(errorMessage)) {
-    resolver->reject(DOMException::create(SecurityError, errorMessage));
-    return promise;
-  }
+
   ASSERT(executionContext->isDocument());
   PermissionService* permissionService =
       getPermissionService(scriptState->getExecutionContext());
@@ -96,6 +92,16 @@ ScriptPromise StorageManager::persist(ScriptState* scriptState) {
 ScriptPromise StorageManager::persisted(ScriptState* scriptState) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
+  ExecutionContext* executionContext = scriptState->getExecutionContext();
+  DCHECK(executionContext->isSecureContext());  // [SecureContext] in IDL
+  SecurityOrigin* securityOrigin = executionContext->getSecurityOrigin();
+  // TODO(jsbell): Is the isUnique() check covered by [SecureContext]? If so,
+  // maybe remove it. Write a test if it stays.
+  if (securityOrigin->isUnique()) {
+    resolver->reject(DOMException::create(NotSupportedError));
+    return promise;
+  }
+
   PermissionService* permissionService =
       getPermissionService(scriptState->getExecutionContext());
   if (!permissionService) {
@@ -117,17 +123,14 @@ ScriptPromise StorageManager::estimate(ScriptState* scriptState) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
   ExecutionContext* executionContext = scriptState->getExecutionContext();
+  DCHECK(executionContext->isSecureContext());  // [SecureContext] in IDL
   SecurityOrigin* securityOrigin = executionContext->getSecurityOrigin();
+  // TODO(jsbell): Is the isUnique() check covered by [SecureContext]? If so,
+  // maybe remove it. Write a test if it stays.
   if (securityOrigin->isUnique()) {
     resolver->reject(DOMException::create(NotSupportedError));
     return promise;
   }
-  // IDL has: [SecureContext]
-  String errorMessage;
-  if (!executionContext->isSecureContext(errorMessage)) {
-    resolver->reject(DOMException::create(SecurityError, errorMessage));
-    return promise;
-  }
 
   KURL storagePartition = KURL(KURL(), securityOrigin->toString());
   Platform::current()->queryStorageUsageAndQuota(