Storage Access API: Add a request roundtrip to check whether prompting is needed
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 May 2018 19:36:01 +0000 (19:36 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 May 2018 19:36:01 +0000 (19:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185368
<rdar://problem/40011556>

Reviewed by Alex Christensen and Youenn Fablet.

This patch adds an enum WebKit::StorageAccessStatus to handle our three access
states:
- WebKit::StorageAccessStatus::CannotRequestAccess.
    This means the domain is blocked from cookie access.
- WebKit::StorageAccessStatus::RequiresUserPrompt.
    This means that access has not been granted yet and a prompt is required.
- WebKit::StorageAccessStatus::HasAccess.
    This either means that this domain does not need to ask for access,
    access was already granted, or access was granted now.

If the call to WebResourceLoadStatisticsStore::requestStorageAccess() comes
back as WebKit::StorageAccessStatus::RequiresUserPrompt, the WebPageProxy
prompts the user and if the user said yes, calls a direct
WebResourceLoadStatisticsStore::grantStorageAccess().

Existing test cases pass because requestStorageAccessConfirm in WKPage.cpp
does not have m_client.requestStorageAccessConfirm and thus returns true.

* UIProcess/Network/NetworkProcessProxy.messages.in:
    Added a missing #endif.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::requestStorageAccess):
    Here we now handle the various cases encoded in WebKit::StorageAccessStatus.
* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::requestStorageAccess):
    Now covers the optional prompt case.
(WebKit::WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener):
(WebKit::WebResourceLoadStatisticsStore::grantStorageAccess):
(WebKit::WebResourceLoadStatisticsStore::grantStorageAccessInternal):
    Granting access is broken out to allow WebKit::WebPageProxy to call it
    directly.
* UIProcess/WebResourceLoadStatisticsStore.h:
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::requestStorageAccess):
(WebKit::WebsiteDataStore::grantStorageAccess):
    Piping through calls from from WebKit::WebResourceLoadStatisticsStore
    to WebKit::WebPageProxy.
* UIProcess/WebsiteData/WebsiteDataStore.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231501 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h

index 2cab276..5e74e28 100644 (file)
@@ -1,3 +1,50 @@
+2018-05-08  John Wilander  <wilander@apple.com>
+
+        Storage Access API: Add a request roundtrip to check whether prompting is needed
+        https://bugs.webkit.org/show_bug.cgi?id=185368
+        <rdar://problem/40011556>
+
+        Reviewed by Alex Christensen and Youenn Fablet.
+
+        This patch adds an enum WebKit::StorageAccessStatus to handle our three access
+        states:
+        - WebKit::StorageAccessStatus::CannotRequestAccess.
+            This means the domain is blocked from cookie access.
+        - WebKit::StorageAccessStatus::RequiresUserPrompt.
+            This means that access has not been granted yet and a prompt is required.
+        - WebKit::StorageAccessStatus::HasAccess.
+            This either means that this domain does not need to ask for access,
+            access was already granted, or access was granted now.
+
+        If the call to WebResourceLoadStatisticsStore::requestStorageAccess() comes
+        back as WebKit::StorageAccessStatus::RequiresUserPrompt, the WebPageProxy
+        prompts the user and if the user said yes, calls a direct
+        WebResourceLoadStatisticsStore::grantStorageAccess().
+
+        Existing test cases pass because requestStorageAccessConfirm in WKPage.cpp
+        does not have m_client.requestStorageAccessConfirm and thus returns true.
+
+        * UIProcess/Network/NetworkProcessProxy.messages.in:
+            Added a missing #endif.
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::requestStorageAccess):
+            Here we now handle the various cases encoded in WebKit::StorageAccessStatus.
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::requestStorageAccess):
+            Now covers the optional prompt case.
+        (WebKit::WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener):
+        (WebKit::WebResourceLoadStatisticsStore::grantStorageAccess):
+        (WebKit::WebResourceLoadStatisticsStore::grantStorageAccessInternal):
+            Granting access is broken out to allow WebKit::WebPageProxy to call it
+            directly.
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::requestStorageAccess):
+        (WebKit::WebsiteDataStore::grantStorageAccess):
+            Piping through calls from from WebKit::WebResourceLoadStatisticsStore
+            to WebKit::WebPageProxy.
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2018-05-08  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, rolling out r231376 and r231458.
index 184c2fd..468f7a4 100644 (file)
@@ -47,7 +47,7 @@ messages -> NetworkProcessProxy LegacyReceiver {
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
     StorageAccessRequestResult(bool wasGranted, uint64_t contextId)
     AllStorageAccessEntriesResult(Vector<String> domains, uint64_t contextId)
-
+#endif
 #if ENABLE(CONTENT_EXTENSIONS)
     ContentExtensionRules(WebKit::UserContentControllerIdentifier identifier)
 #endif
index 4bca575..3583224 100644 (file)
 #include "WebCredentialsMessengerProxy.h"
 #endif
 
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+#include "WebResourceLoadStatisticsStore.h"
+#endif
+
 // This controls what strategy we use for mouse wheel coalescing.
 #define MERGE_WHEEL_EVENTS 1
 
@@ -7481,19 +7485,28 @@ void WebPageProxy::hasStorageAccess(String&& subFrameHost, String&& topFrameHost
 
 void WebPageProxy::requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t webProcessContextId, bool promptEnabled)
 {
-    auto completionHandler = [this, protectedThis = makeRef(*this), webProcessContextId] (bool access) {
+    CompletionHandler<void(bool)> completionHandler = [this, protectedThis = makeRef(*this), webProcessContextId] (bool access) {
         m_process->send(Messages::WebPage::StorageAccessResponse(access, webProcessContextId), m_pageID);
     };
-    String requestingDomain = subFrameHost;
-    String currentDomain = topFrameHost;
-    m_websiteDataStore->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, m_pageID, [this, protectedThis = makeRef(*this), requestingDomain = WTFMove(requestingDomain), currentDomain = WTFMove(currentDomain), promptEnabled, frameID, completionHandler = WTFMove(completionHandler)] (bool wasGranted) mutable {
-        if (!wasGranted)
-            return completionHandler(false);
-        
-        if (!promptEnabled)
-            return completionHandler(true);
-
-        m_uiClient->requestStorageAccessConfirm(*this, m_process->webFrame(frameID), requestingDomain, currentDomain, WTFMove(completionHandler));
+
+    m_websiteDataStore->requestStorageAccess(String(subFrameHost), String(topFrameHost), frameID, m_pageID, promptEnabled, [this, protectedThis = makeRef(*this), subFrameHost, topFrameHost, promptEnabled, frameID, completionHandler = WTFMove(completionHandler)] (StorageAccessStatus status) mutable {
+        switch (status) {
+        case StorageAccessStatus::CannotRequestAccess:
+            completionHandler(false);
+            return;
+        case StorageAccessStatus::RequiresUserPrompt:
+            ASSERT_UNUSED(promptEnabled, promptEnabled);
+            m_uiClient->requestStorageAccessConfirm(*this, m_process->webFrame(frameID), String(subFrameHost), String(topFrameHost), [this, protectedThis = makeRef(*this), subFrameHost, topFrameHost, frameID, completionHandler = WTFMove(completionHandler)] (bool userDidGrantAccess) mutable {
+                if (userDidGrantAccess)
+                    m_websiteDataStore->grantStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, m_pageID, userDidGrantAccess, WTFMove(completionHandler));
+                else
+                    completionHandler(false);
+            });
+            return;
+        case StorageAccessStatus::HasAccess:
+            completionHandler(true);
+            return;
+        }
     });
 }
 #endif
index de7c4b1..8d05117 100644 (file)
@@ -355,7 +355,7 @@ void WebResourceLoadStatisticsStore::hasStorageAccess(String&& subFrameHost, Str
     });
 }
 
-void WebResourceLoadStatisticsStore::requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback)
+void WebResourceLoadStatisticsStore::requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool promptEnabled, CompletionHandler<void(StorageAccessStatus)>&& callback)
 {
     ASSERT(subFrameHost != topFrameHost);
     ASSERT(RunLoop::isMain());
@@ -363,32 +363,39 @@ void WebResourceLoadStatisticsStore::requestStorageAccess(String&& subFrameHost,
     auto subFramePrimaryDomain = isolatedPrimaryDomain(subFrameHost);
     auto topFramePrimaryDomain = isolatedPrimaryDomain(topFrameHost);
     if (subFramePrimaryDomain == topFramePrimaryDomain) {
-        callback(true);
+        callback(StorageAccessStatus::HasAccess);
         return;
     }
 
-    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), subFramePrimaryDomain = crossThreadCopy(subFramePrimaryDomain), topFramePrimaryDomain = crossThreadCopy(topFramePrimaryDomain), frameID, pageID, callback = WTFMove(callback)] () mutable {
+    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), subFramePrimaryDomain = crossThreadCopy(subFramePrimaryDomain), topFramePrimaryDomain = crossThreadCopy(topFramePrimaryDomain), frameID, pageID, promptEnabled, callback = WTFMove(callback)] () mutable {
 
         auto& subFrameStatistic = ensureResourceStatisticsForPrimaryDomain(subFramePrimaryDomain);
         if (shouldBlockCookies(subFrameStatistic)) {
-            callOnMainThread([callback = WTFMove(callback)] {
-                callback(false);
+            RunLoop::main().dispatch([callback = WTFMove(callback)] {
+                callback(StorageAccessStatus::CannotRequestAccess);
             });
             return;
         }
         
         if (!shouldPartitionCookies(subFrameStatistic)) {
-            callOnMainThread([callback = WTFMove(callback)] {
-                callback(true);
+            RunLoop::main().dispatch([callback = WTFMove(callback)] {
+                callback(StorageAccessStatus::HasAccess);
+            });
+            return;
+        }
+
+        if (promptEnabled) {
+            RunLoop::main().dispatch([callback = WTFMove(callback)] {
+                callback(StorageAccessStatus::RequiresUserPrompt);
             });
             return;
         }
 
         subFrameStatistic.timesAccessedAsFirstPartyDueToStorageAccessAPI++;
 
-        m_grantStorageAccessHandler(subFramePrimaryDomain, topFramePrimaryDomain, frameID, pageID, [callback = WTFMove(callback)] (bool value) mutable {
-            callOnMainThread([value, callback = WTFMove(callback)] {
-                callback(value);
+        grantStorageAccessInternal(WTFMove(subFramePrimaryDomain), WTFMove(topFramePrimaryDomain), frameID, pageID, false, [callback = WTFMove(callback)] (bool wasGrantedAccess) mutable {
+            RunLoop::main().dispatch([callback = WTFMove(callback), wasGrantedAccess] () mutable {
+                callback(wasGrantedAccess ? StorageAccessStatus::HasAccess : StorageAccessStatus::CannotRequestAccess);
             });
         });
     });
@@ -413,12 +420,37 @@ void WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener(String&& do
     if (!cookiesBlocked && !shouldPartitionCookies(domainInNeedOfStorageAccessStatistic))
         return;
 
-    m_grantStorageAccessHandler(WTFMove(domainInNeedOfStorageAccess), WTFMove(openerDomain), std::nullopt, openerPageID, [](bool) { });
+    grantStorageAccessInternal(WTFMove(domainInNeedOfStorageAccess), WTFMove(openerDomain), std::nullopt, openerPageID, false, [](bool) { });
 #if !RELEASE_LOG_DISABLED
     RELEASE_LOG_INFO_IF(m_debugLoggingEnabled, ResourceLoadStatisticsDebug, "Grant storage access for %{public}s under opener %{public}s, %{public}s user interaction.", domainInNeedOfStorageAccess.utf8().data(), openerDomain.utf8().data(), (isTriggeredByUserGesture ? "with" : "without"));
 #endif
 }
 
+void WebResourceLoadStatisticsStore::grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&& callback)
+{
+    ASSERT(RunLoop::isMain());
+    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), subFrameHost = crossThreadCopy(subFrameHost), topFrameHost = crossThreadCopy(topFrameHost), frameID, pageID, userWasPrompted, callback = WTFMove(callback)] () mutable {
+        grantStorageAccessInternal(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, userWasPrompted, [callback = WTFMove(callback)] (bool wasGrantedAccess) mutable {
+            RunLoop::main().dispatch([callback = WTFMove(callback), wasGrantedAccess] () mutable {
+                callback(wasGrantedAccess);
+            });
+        });
+    });
+}
+
+void WebResourceLoadStatisticsStore::grantStorageAccessInternal(String&& subFrameHost, String&& topFrameHost, std::optional<uint64_t> frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&& callback)
+{
+    ASSERT(!RunLoop::isMain());
+    auto subFramePrimaryDomain = isolatedPrimaryDomain(subFrameHost);
+    auto topFramePrimaryDomain = isolatedPrimaryDomain(topFrameHost);
+    if (subFramePrimaryDomain == topFramePrimaryDomain) {
+        callback(true);
+        return;
+    }
+    
+    m_grantStorageAccessHandler(subFramePrimaryDomain, topFramePrimaryDomain, frameID, pageID, WTFMove(callback));
+}
+
 void WebResourceLoadStatisticsStore::removeAllStorageAccess()
 {
     ASSERT(!RunLoop::isMain());
index f67b628..17a54e9 100644 (file)
@@ -58,6 +58,11 @@ class OperatingDate;
 class WebProcessProxy;
 
 enum class ShouldClearFirst;
+enum class StorageAccessStatus {
+    CannotRequestAccess,
+    RequiresUserPrompt,
+    HasAccess
+};
 
 class WebResourceLoadStatisticsStore final : public IPC::Connection::WorkQueueMessageReceiver {
 public:
@@ -85,8 +90,9 @@ public:
     void resourceLoadStatisticsUpdated(Vector<WebCore::ResourceLoadStatistics>&& origins);
 
     void hasStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
-    void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
+    void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool promptEnabled, CompletionHandler<void(StorageAccessStatus)>&&);
     void requestStorageAccessUnderOpener(String&& domainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerDomain, bool isTriggeredByUserGesture);
+    void grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&&);
     void requestStorageAccessCallback(bool wasGranted, uint64_t contextId);
 
     void processWillOpenConnection(WebProcessProxy&, IPC::Connection&);
@@ -179,6 +185,7 @@ private:
     void processStatisticsAndDataRecords();
 
     void resetCookiePartitioningState();
+    void grantStorageAccessInternal(String&& subFrameHost, String&& topFrameHost, std::optional<uint64_t> frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&&);
     void removeAllStorageAccess();
 
     void setDebugLogggingEnabled(bool enabled) { m_debugLoggingEnabled  = enabled; }
index b754521..e07472f 100644 (file)
@@ -1240,14 +1240,24 @@ void WebsiteDataStore::hasStorageAccess(String&& subFrameHost, String&& topFrame
     m_resourceLoadStatistics->hasStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, WTFMove(callback));
 }
 
-void WebsiteDataStore::requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback)
+void WebsiteDataStore::requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool promptEnabled, CompletionHandler<void(StorageAccessStatus)>&& callback)
+{
+    if (!resourceLoadStatisticsEnabled()) {
+        callback(StorageAccessStatus::CannotRequestAccess);
+        return;
+    }
+    
+    m_resourceLoadStatistics->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, promptEnabled, WTFMove(callback));
+}
+
+void WebsiteDataStore::grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&& callback)
 {
     if (!resourceLoadStatisticsEnabled()) {
         callback(false);
         return;
     }
     
-    m_resourceLoadStatistics->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, WTFMove(callback));
+    m_resourceLoadStatistics->grantStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, userWasPrompted, WTFMove(callback));
 }
 #endif
 
index 4dfd9d9..231b786 100644 (file)
@@ -60,6 +60,11 @@ struct StorageProcessCreationParameters;
 struct WebsiteDataRecord;
 struct WebsiteDataStoreParameters;
 
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+enum class StorageAccessStatus;
+enum class StorageAccessPromptStatus;
+#endif
+
 #if ENABLE(NETSCAPE_PLUGIN_API)
 struct PluginModuleInfo;
 #endif
@@ -130,7 +135,8 @@ public:
     void removeAllStorageAccessHandler();
     void removePrevalentDomains(const Vector<String>& domains);
     void hasStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
-    void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&& callback);
+    void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool promptEnabled, CompletionHandler<void(StorageAccessStatus)>&&);
+    void grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&&);
 #endif
     void networkProcessDidCrash();
     void resolveDirectoriesIfNecessary();