Attachment 339790 Details for Bug 185368 – Patch

[patch] Patch

bug-185368-20180507194026.patch (text/plain), 17.18 KB, created by John Wilander on 2018-05-07 19:40:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: John Wilander

Created: 2018-05-07 19:40:27 PDT

Size: 17.18 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 231470)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,43 @@
>+2018-05-07  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::DoesNotHaveAccess.
>+            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, this
>+            access was already granted, or access was granted now.
>+
>+        If the first call to WebResourceLoadStatisticsStore::requestStorageAccess()
>+        comes back as WebKit::StorageAccessStatus::DoesNotHaveAccess, the WebPageProxy
>+        may prompt the user and request access again, if the user said yes.
>+
>+        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.
>+        * UIProcess/WebResourceLoadStatisticsStore.h:
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::requestStorageAccess):
>+            Piping through WebKit::StorageAccessStatus from
>+            WebResourceLoadStatisticsStore to WebPageProxy.
>+        * UIProcess/WebsiteData/WebsiteDataStore.h:
>+
> 2018-05-07  Alex Christensen  <achristensen@webkit.org>
> 
>         WebResourceLoadStatisticsStore::requestStorageAccess should call its completion handler on the main thread
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 231470)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -114,6 +114,7 @@
> #include "WebProcessPool.h"
> #include "WebProcessProxy.h"
> #include "WebProtectionSpace.h"
>+#include "WebResourceLoadStatisticsStore.h"
> #include "WebURLSchemeHandler.h"
> #include "WebUserContentControllerProxy.h"
> #include "WebsiteDataStore.h"
>@@ -7484,16 +7485,48 @@ void WebPageProxy::requestStorageAccess(
>     auto 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));
>+    auto promptStatus = promptEnabled ? StorageAccessPromptStatus::UserNotPrompted : StorageAccessPromptStatus::UserGrantedAccess;
>+    String subFrameHostCopy = subFrameHost;
>+    String topFrameHostCopy = topFrameHost;
>+    m_websiteDataStore->requestStorageAccess(WTFMove(subFrameHostCopy), WTFMove(topFrameHostCopy), frameID, m_pageID, promptStatus, [this, protectedThis = makeRef(*this), subFrameHost = WTFMove(subFrameHost), topFrameHost = WTFMove(topFrameHost), promptEnabled, frameID, completionHandler = WTFMove(completionHandler)] (StorageAccessStatus status) mutable {
>+        switch (status) {
>+        case StorageAccessStatus::CannotRequestAccess:
>+            completionHandler(false);
>+            return;
>+        case StorageAccessStatus::DoesNotHaveAccess: {
>+            if (!promptEnabled) {
>+                ASSERT_NOT_REACHED();
>+                completionHandler(false);
>+                return;
>+            }
>+            String subFrameHostCopy = subFrameHost;
>+            String topFrameHostCopy = topFrameHost;
>+            m_uiClient->requestStorageAccessConfirm(*this, m_process->webFrame(frameID), WTFMove(subFrameHostCopy), WTFMove(topFrameHostCopy), [this, protectedThis = makeRef(*this), subFrameHost = WTFMove(subFrameHost), topFrameHost = WTFMove(topFrameHost), frameID, completionHandler = WTFMove(completionHandler)] (bool access) mutable {
>+                if (access) {
>+                    m_websiteDataStore->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, m_pageID, StorageAccessPromptStatus::UserGrantedAccess, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (StorageAccessStatus status) mutable {
>+                        switch (status) {
>+                        case StorageAccessStatus::CannotRequestAccess:
>+                        case StorageAccessStatus::DoesNotHaveAccess:
>+                            ASSERT_NOT_REACHED();
>+                            completionHandler(false);
>+                            return;
>+                        case StorageAccessStatus::HasAccess:
>+                            completionHandler(true);
>+                            return;
>+                        }
>+                        RELEASE_ASSERT_NOT_REACHED();
>+                    });
>+                } else
>+                    completionHandler(false);
>+            });
>+            return;
>+        }
>+        case StorageAccessStatus::HasAccess:
>+            completionHandler(true);
>+            return;
>+        }
>+        RELEASE_ASSERT_NOT_REACHED();
>     });
> }
> #endif
>Index: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp	(revision 231470)
>+++ 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, StorageAccessPromptStatus promptStatus, WTF::CompletionHandler<void(StorageAccessStatus)>&& callback)
> {
>     ASSERT(subFrameHost != topFrameHost);
>     ASSERT(RunLoop::isMain());
>@@ -363,33 +363,46 @@ 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, promptStatus, 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 (promptStatus == StorageAccessPromptStatus::UserNotPrompted) {
>+            callOnMainThread([callback = WTFMove(callback)] {
>+                callback(StorageAccessStatus::DoesNotHaveAccess);
>             });
>             return;
>         }
> 
>         subFrameStatistic.timesAccessedAsFirstPartyDueToStorageAccessAPI++;
> 
>-        m_grantStorageAccessHandler(subFramePrimaryDomain, topFramePrimaryDomain, frameID, pageID, [callback = WTFMove(callback)] (bool value) mutable {
>-            callOnMainThread([value, callback = WTFMove(callback)] {
>-                callback(value);
>-            });
>+        m_grantStorageAccessHandler(subFramePrimaryDomain, topFramePrimaryDomain, frameID, pageID, [callback = WTFMove(callback)] (bool wasGranted) mutable {
>+            if (wasGranted) {
>+                callOnMainThread([callback = WTFMove(callback)] {
>+                    callback(StorageAccessStatus::HasAccess);
>+                });
>+            } else {
>+                callOnMainThread([callback = WTFMove(callback)] {
>+                    callback(StorageAccessStatus::CannotRequestAccess);
>+                });
>+            }
>         });
>     });
> }
>Index: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
>===================================================================
>--- Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	(revision 231470)
>+++ Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	(working copy)
>@@ -58,6 +58,15 @@ class OperatingDate;
> class WebProcessProxy;
> 
> enum class ShouldClearFirst;
>+enum class StorageAccessStatus {
>+    CannotRequestAccess,
>+    DoesNotHaveAccess,
>+    HasAccess
>+};
>+enum class StorageAccessPromptStatus {
>+    UserNotPrompted,
>+    UserGrantedAccess
>+};
> 
> class WebResourceLoadStatisticsStore final : public IPC::Connection::WorkQueueMessageReceiver {
> public:
>@@ -66,7 +75,7 @@ public:
>     using GrantStorageAccessHandler = WTF::Function<void(const String& resourceDomain, const String& firstPartyDomain, std::optional<uint64_t> frameID, uint64_t pageID, WTF::Function<void(bool wasGranted)>&& callback)>;
>     using RemoveAllStorageAccessHandler = WTF::Function<void()>;
>     using RemovePrevalentDomainsHandler = WTF::Function<void (const Vector<String>&)>;
>-    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, bool isEphemeral, UpdatePrevalentDomainsToPartitionOrBlockCookiesHandler&& updatePrevalentDomainsToPartitionOrBlockCookiesHandler = [](const WTF::Vector<String>&, const WTF::Vector<String>&, const WTF::Vector<String>&, ShouldClearFirst) { }, HasStorageAccessForFrameHandler&& hasStorageAccessForFrameHandler = [](const String&, const String&, uint64_t, uint64_t, WTF::Function<void(bool)>&&) { }, GrantStorageAccessHandler&& grantStorageAccessHandler = [](const String&, const String&, std::optional<uint64_t>, uint64_t, WTF::Function<void(bool)>&&) { }, RemoveAllStorageAccessHandler&& removeAllStorageAccessHandler = []() { }, RemovePrevalentDomainsHandler&& removeDomainsHandler = [] (const WTF::Vector<String>&) { })
>+    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, bool isEphemeral, UpdatePrevalentDomainsToPartitionOrBlockCookiesHandler&& updatePrevalentDomainsToPartitionOrBlockCookiesHandler = [](const WTF::Vector<String>&, const WTF::Vector<String>&, const WTF::Vector<String>&, ShouldClearFirst) { }, HasStorageAccessForFrameHandler&& hasStorageAccessForFrameHandler = [](const String&, const String&, uint64_t, uint64_t, WTF::Function<void(bool)>&&) { }, GrantStorageAccessHandler&& grantStorageAccessHandler = [](const String&, const String&, std::optional<uint64_t>, uint64_t, WTF::Function<void(StorageAccessStatus)>&&) { }, RemoveAllStorageAccessHandler&& removeAllStorageAccessHandler = []() { }, RemovePrevalentDomainsHandler&& removeDomainsHandler = [] (const WTF::Vector<String>&) { })
>     {
>         return adoptRef(*new WebResourceLoadStatisticsStore(resourceLoadStatisticsDirectory, WTFMove(testingCallback), isEphemeral, WTFMove(updatePrevalentDomainsToPartitionOrBlockCookiesHandler), WTFMove(hasStorageAccessForFrameHandler), WTFMove(grantStorageAccessHandler), WTFMove(removeAllStorageAccessHandler), WTFMove(removeDomainsHandler)));
>     }
>@@ -85,7 +94,7 @@ 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, StorageAccessPromptStatus, WTF::CompletionHandler<void(StorageAccessStatus)>&& callback);
>     void requestStorageAccessUnderOpener(String&& domainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerDomain, bool isTriggeredByUserGesture);
>     void requestStorageAccessCallback(bool wasGranted, uint64_t contextId);
> 
>Index: Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
>===================================================================
>--- Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in	(revision 231470)
>+++ 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 231470)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(working copy)
>@@ -1240,14 +1240,14 @@ 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, StorageAccessPromptStatus promptStatus, CompletionHandler<void(StorageAccessStatus)>&& callback)
> {
>     if (!resourceLoadStatisticsEnabled()) {
>-        callback(false);
>+        callback(StorageAccessStatus::CannotRequestAccess);
>         return;
>     }
>     
>-    m_resourceLoadStatistics->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, WTFMove(callback));
>+    m_resourceLoadStatistics->requestStorageAccess(WTFMove(subFrameHost), WTFMove(topFrameHost), frameID, pageID, promptStatus, WTFMove(callback));
> }
> #endif
> 
>Index: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>===================================================================
>--- Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	(revision 231470)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	(working copy)
>@@ -59,6 +59,8 @@ enum class WebsiteDataType;
> struct StorageProcessCreationParameters;
> struct WebsiteDataRecord;
> struct WebsiteDataStoreParameters;
>+enum class StorageAccessStatus;
>+enum class StorageAccessPromptStatus;
> 
> #if ENABLE(NETSCAPE_PLUGIN_API)
> struct PluginModuleInfo;
>@@ -130,7 +132,7 @@ 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, StorageAccessPromptStatus, CompletionHandler<void(StorageAccessStatus)>&&);
> #endif
>     void networkProcessDidCrash();
>     void resolveDirectoriesIfNecessary();

Actions: View | Formatted Diff | Diff

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