REGRESSION (r240498): Storage Access API prompts result is not remembered
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 Jan 2019 22:34:22 +0000 (22:34 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 Jan 2019 22:34:22 +0000 (22:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193922
<rdar://problem/47608767>

Reviewed by Chris Dumez.

The refactoring in r240498 bypassed bookkeeping in ResourceLoadStatisticsMemoryStore
that kept track of whether a successful user prompt had been encountered. This
patch corrects this codepath so the user is not prompted repeatedly after approving
use of the Storage Access API.

* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::grantStorageAccess): Switch from Move operator to
const reference to allow the method to be called by the NetworkProcess.
* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::grantStorageAccess): Request access through the WebResourceLoadStatistics
object, rather than jumping directly to the NetworkStorageSession.

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
Source/WebKit/NetworkProcess/NetworkProcess.cpp

index 5d0f5d9..fc48ef7 100644 (file)
@@ -1,3 +1,24 @@
+2019-01-28  Brent Fulgham  <bfulgham@apple.com>
+
+        REGRESSION (r240498): Storage Access API prompts result is not remembered
+        https://bugs.webkit.org/show_bug.cgi?id=193922
+        <rdar://problem/47608767>
+
+        Reviewed by Chris Dumez.
+
+        The refactoring in r240498 bypassed bookkeeping in ResourceLoadStatisticsMemoryStore
+        that kept track of whether a successful user prompt had been encountered. This
+        patch corrects this codepath so the user is not prompted repeatedly after approving
+        use of the Storage Access API.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::grantStorageAccess): Switch from Move operator to
+        const reference to allow the method to be called by the NetworkProcess.
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::grantStorageAccess): Request access through the WebResourceLoadStatistics
+        object, rather than jumping directly to the NetworkStorageSession.
+
 2019-01-28  Chris Dumez  <cdumez@apple.com>
 
         Regression(PSON) Crash under WebPageProxy::didStartProgress()
 2019-01-28  Chris Dumez  <cdumez@apple.com>
 
         Regression(PSON) Crash under WebPageProxy::didStartProgress()
index 33f5f0d..5cdf830 100644 (file)
@@ -339,7 +339,7 @@ void WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener(String&& pr
     });
 }
 
     });
 }
 
-void WebResourceLoadStatisticsStore::grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPromptedNow, CompletionHandler<void(bool)>&& completionHandler)
+void WebResourceLoadStatisticsStore::grantStorageAccess(const String& subFrameHost, const String& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPromptedNow, CompletionHandler<void(bool)>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
     postTask([this, subFrameHost = crossThreadCopy(subFrameHost), topFrameHost = crossThreadCopy(topFrameHost), frameID, pageID, userWasPromptedNow, completionHandler = WTFMove(completionHandler)]() mutable {
 {
     ASSERT(RunLoop::isMain());
     postTask([this, subFrameHost = crossThreadCopy(subFrameHost), topFrameHost = crossThreadCopy(topFrameHost), frameID, pageID, userWasPromptedNow, completionHandler = WTFMove(completionHandler)]() mutable {
index dfd065a..d28890e 100644 (file)
@@ -76,7 +76,7 @@ public:
     void setShouldSubmitTelemetry(bool);
 
     void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool promptEnabled, CompletionHandler<void(StorageAccessStatus)>&&);
     void setShouldSubmitTelemetry(bool);
 
     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 userWasPromptedNow, CompletionHandler<void(bool)>&&);
+    void grantStorageAccess(const String& subFrameHost, const String& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPromptedNow, CompletionHandler<void(bool)>&&);
     void requestStorageAccessCallback(bool wasGranted, uint64_t contextId);
 
     void applicationWillTerminate();
     void requestStorageAccessCallback(bool wasGranted, uint64_t contextId);
 
     void applicationWillTerminate();
index cdcb2ab..9542879 100644 (file)
@@ -980,15 +980,15 @@ void NetworkProcess::requestStorageAccess(PAL::SessionID sessionID, const String
 
 void NetworkProcess::grantStorageAccess(PAL::SessionID sessionID, const String& resourceDomain, const String& firstPartyDomain, Optional<uint64_t> frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&& completionHandler)
 {
 
 void NetworkProcess::grantStorageAccess(PAL::SessionID sessionID, const String& resourceDomain, const String& firstPartyDomain, Optional<uint64_t> frameID, uint64_t pageID, bool userWasPrompted, CompletionHandler<void(bool)>&& completionHandler)
 {
-    bool isStorageGranted = false;
-    if (auto* networkStorageSession = storageSession(sessionID)) {
-        networkStorageSession->grantStorageAccess(resourceDomain, firstPartyDomain, frameID, pageID);
-        ASSERT(networkStorageSession->hasStorageAccess(resourceDomain, firstPartyDomain, frameID, pageID));
-        isStorageGranted = true;
-    } else
+    if (auto* networkSession = this->networkSession(sessionID)) {
+        if (auto* resourceLoadStatistics = networkSession->resourceLoadStatistics())
+            resourceLoadStatistics->grantStorageAccess(resourceDomain, firstPartyDomain, frameID.value(), pageID, userWasPrompted, WTFMove(completionHandler));
+        else
+            completionHandler(false);
+    } else {
         ASSERT_NOT_REACHED();
         ASSERT_NOT_REACHED();
-
-    completionHandler(isStorageGranted);
+        completionHandler(false);
+    }
 }
 
 void NetworkProcess::logFrameNavigation(PAL::SessionID sessionID, const String& targetPrimaryDomain, const String& mainFramePrimaryDomain, const String& sourcePrimaryDomain, const String& targetHost, const String& mainFrameHost, bool isRedirect, bool isMainFrame)
 }
 
 void NetworkProcess::logFrameNavigation(PAL::SessionID sessionID, const String& targetPrimaryDomain, const String& mainFramePrimaryDomain, const String& sourcePrimaryDomain, const String& targetHost, const String& mainFrameHost, bool isRedirect, bool isMainFrame)