Attachment 339840 Details for Bug 185368 – Patch

[patch] Patch

bug-185368-20180508111415.patch (text/plain), 17.47 KB, created by Alex Christensen on 2018-05-08 11:14:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-05-08 11:14:16 PDT

Size: 17.47 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 231494)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Youenn Fablet  <youenn@apple.com>
> 
>         Allow WebResourceLoader to cancel a load served from a service worker
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 231494)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -204,6 +204,10 @@
> #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(Stri
> 
> 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: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp	(revision 231494)
>+++ Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp	(working copy)
>@@ -355,7 +355,7 @@ void WebResourceLoadStatisticsStore::has
>     });
> }
> 
>-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,42 @@ void WebResourceLoadStatisticsStore::req
>     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);
>+                callback(StorageAccessStatus::CannotRequestAccess);
>             });
>             return;
>         }
>         
>         if (!shouldPartitionCookies(subFrameStatistic)) {
>             callOnMainThread([callback = WTFMove(callback)] {
>-                callback(true);
>+                callback(StorageAccessStatus::HasAccess);
>+            });
>+            return;
>+        }
>+
>+        if (promptEnabled) {
>+            callOnMainThread([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 {
>+            callOnMainThread([callback = WTFMove(callback), wasGrantedAccess] () mutable {
>+                if (wasGrantedAccess)
>+                    callback(StorageAccessStatus::HasAccess);
>+                else
>+                    callback(StorageAccessStatus::CannotRequestAccess);
>             });
>         });
>     });
>@@ -413,12 +423,38 @@ void WebResourceLoadStatisticsStore::req
>     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 {
>+            callOnMainThread([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)
>+{
>+    UNUSED_PARAM(userWasPrompted);
>+    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: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
>===================================================================
>--- Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	(revision 231494)
>+++ Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	(working copy)
>@@ -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: Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
>===================================================================
>--- Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in	(revision 231494)
>+++ Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in	(working copy)
>@@ -47,7 +47,7 @@ messages -> NetworkProcessProxy LegacyRe
> #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: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(revision 231494)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(working copy)
>@@ -1240,14 +1240,24 @@ void WebsiteDataStore::hasStorageAccess(
>     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: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>===================================================================
>--- Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	(revision 231494)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	(working copy)
>@@ -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();

Flags:

achristensen: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185368: 339696 | 339786 | 339790 | 339792 | 339800 | 339836 | 339840