Title: [279004] trunk/Source
Revision
279004
Author
katherine_che...@apple.com
Date
2021-06-17 13:03:07 -0700 (Thu, 17 Jun 2021)

Log Message

Storage Access quirks should prompt up to twice if a user does not allow storage access
https://bugs.webkit.org/show_bug.cgi?id=227099
<rdar://problem/79409843>

Reviewed by John Wilander.

Source/WebCore:

Remove hasDeniedCrossPageStorageAccess functions. They are not needed
now that we are aligning storage access quirks with non-quirks by
using maxNumberOfTimesExplicitlyDeniedStorageAccess.

* dom/DocumentStorageAccess.cpp:
(WebCore::DocumentStorageAccess::requestStorageAccessQuickCheck):
(WebCore::DocumentStorageAccess::requestStorageAccessForNonDocumentQuirk):
* dom/DocumentStorageAccess.h:
* dom/Element.cpp:
(WebCore::Element::dispatchMouseEvent):
* dom/Element.h:
* loader/ResourceLoadObserver.h:
(WebCore::ResourceLoadObserver::hasHadUserInteraction const):
(WebCore::ResourceLoadObserver::setHasDeniedCrossPageStorageAccess): Deleted.
(WebCore::ResourceLoadObserver::hasDeniedCrossPageStorageAccess const): Deleted.
* page/Quirks.cpp:
(WebCore::isStorageAccessQuirkDomainAndElement):
(WebCore::Quirks::requestStorageAccessAndHandleClick const):
(WebCore::Quirks::triggerOptionalStorageAccessQuirk const):
(WebCore::hasDeniedCrossPageStorageAccess): Deleted.
* page/Quirks.h:

Source/WebKit:

We should allow storage access prompts twice per document for quirks
just like for non-quirk cases. Previously we were only presenting
the quirk prompt once per web content process if the user denied. This
patch removes this code and utilizes maxNumberOfTimesExplicitlyDeniedStorageAccess
instead.

* WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:
(WebKit::WebResourceLoadObserver::hasDeniedCrossPageStorageAccess const): Deleted.
(WebKit::WebResourceLoadObserver::setHasDeniedCrossPageStorageAccess): Deleted.
* WebProcess/WebCoreSupport/WebResourceLoadObserver.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (279003 => 279004)


--- trunk/Source/WebCore/ChangeLog	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/ChangeLog	2021-06-17 20:03:07 UTC (rev 279004)
@@ -1,3 +1,33 @@
+2021-06-17  Kate Cheney  <katherine_che...@apple.com>
+
+        Storage Access quirks should prompt up to twice if a user does not allow storage access
+        https://bugs.webkit.org/show_bug.cgi?id=227099
+        <rdar://problem/79409843>
+
+        Reviewed by John Wilander.
+
+        Remove hasDeniedCrossPageStorageAccess functions. They are not needed
+        now that we are aligning storage access quirks with non-quirks by
+        using maxNumberOfTimesExplicitlyDeniedStorageAccess. 
+
+        * dom/DocumentStorageAccess.cpp:
+        (WebCore::DocumentStorageAccess::requestStorageAccessQuickCheck):
+        (WebCore::DocumentStorageAccess::requestStorageAccessForNonDocumentQuirk):
+        * dom/DocumentStorageAccess.h:
+        * dom/Element.cpp:
+        (WebCore::Element::dispatchMouseEvent):
+        * dom/Element.h:
+        * loader/ResourceLoadObserver.h:
+        (WebCore::ResourceLoadObserver::hasHadUserInteraction const):
+        (WebCore::ResourceLoadObserver::setHasDeniedCrossPageStorageAccess): Deleted.
+        (WebCore::ResourceLoadObserver::hasDeniedCrossPageStorageAccess const): Deleted.
+        * page/Quirks.cpp:
+        (WebCore::isStorageAccessQuirkDomainAndElement):
+        (WebCore::Quirks::requestStorageAccessAndHandleClick const):
+        (WebCore::Quirks::triggerOptionalStorageAccessQuirk const):
+        (WebCore::hasDeniedCrossPageStorageAccess): Deleted.
+        * page/Quirks.h:
+
 2021-06-17  Aditya Keerthi  <akeer...@apple.com>
 
         REGRESSION (r277067): Incorrect text color for default-button appearance

Modified: trunk/Source/WebCore/dom/DocumentStorageAccess.cpp (279003 => 279004)


--- trunk/Source/WebCore/dom/DocumentStorageAccess.cpp	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/dom/DocumentStorageAccess.cpp	2021-06-17 20:03:07 UTC (rev 279004)
@@ -153,7 +153,7 @@
     if (frame && hasFrameSpecificStorageAccess())
         return StorageAccessQuickResult::Grant;
 
-    if (!frame || m_document.securityOrigin().isUnique() || !isAllowedToRequestFrameSpecificStorageAccess())
+    if (!frame || m_document.securityOrigin().isUnique() || !isAllowedToRequestStorageAccess())
         return StorageAccessQuickResult::Reject;
 
     if (frame->isMainFrame())
@@ -255,7 +255,7 @@
 
 void DocumentStorageAccess::requestStorageAccessForNonDocumentQuirk(RegistrableDomain&& requestingDomain, CompletionHandler<void(StorageAccessWasGranted)>&& completionHandler)
 {
-    if (!m_document.frame() || !m_document.frame()->page()) {
+    if (!m_document.frame() || !m_document.frame()->page() || !isAllowedToRequestStorageAccess()) {
         completionHandler(StorageAccessWasGranted::No);
         return;
     }

Modified: trunk/Source/WebCore/dom/DocumentStorageAccess.h (279003 => 279004)


--- trunk/Source/WebCore/dom/DocumentStorageAccess.h	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/dom/DocumentStorageAccess.h	2021-06-17 20:03:07 UTC (rev 279004)
@@ -68,7 +68,7 @@
     template<class Decoder> static std::optional<RequestStorageAccessResult> decode(Decoder&);
 };
 
-const unsigned maxNumberOfTimesExplicitlyDeniedFrameSpecificStorageAccess = 2;
+const unsigned maxNumberOfTimesExplicitlyDeniedStorageAccess = 2;
 
 class DocumentStorageAccess final : public Supplement<Document>, public CanMakeWeakPtr<DocumentStorageAccess> {
     WTF_MAKE_FAST_ALLOCATED;
@@ -98,7 +98,7 @@
     static const char* supplementName();
     bool hasFrameSpecificStorageAccess() const;
     void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; };
-    bool isAllowedToRequestFrameSpecificStorageAccess() { return m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess < maxNumberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; };
+    bool isAllowedToRequestStorageAccess() { return m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess < maxNumberOfTimesExplicitlyDeniedStorageAccess; };
     void enableTemporaryTimeUserGesture();
     void consumeTemporaryTimeUserGesture();
 

Modified: trunk/Source/WebCore/dom/Element.cpp (279003 => 279004)


--- trunk/Source/WebCore/dom/Element.cpp	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/dom/Element.cpp	2021-06-17 20:03:07 UTC (rev 279004)
@@ -373,7 +373,7 @@
     return ShouldIgnoreMouseEvent::No;
 }
 
-bool Element::dispatchMouseEvent(const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget)
+bool Element::dispatchMouseEvent(const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget, IsSyntheticClick isSyntheticClick)
 {
     if (isDisabledFormControl())
         return false;
@@ -399,7 +399,7 @@
 #elif PLATFORM(MAC)
     isParentProcessAFullWebBrowser = MacApplication::isSafari();
 #endif
-    if (Quirks::StorageAccessResult::ShouldCancelEvent == document().quirks().triggerOptionalStorageAccessQuirk(*this, platformEvent, eventType, detail, relatedTarget, isParentProcessAFullWebBrowser))
+    if (Quirks::StorageAccessResult::ShouldCancelEvent == document().quirks().triggerOptionalStorageAccessQuirk(*this, platformEvent, eventType, detail, relatedTarget, isParentProcessAFullWebBrowser, isSyntheticClick))
         return false;
 
     ASSERT(!mouseEvent->target() || mouseEvent->target() != relatedTarget);

Modified: trunk/Source/WebCore/dom/Element.h (279003 => 279004)


--- trunk/Source/WebCore/dom/Element.h	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/dom/Element.h	2021-06-17 20:03:07 UTC (rev 279004)
@@ -65,6 +65,7 @@
 
 enum class AnimationImpact;
 enum class EventProcessing : uint8_t;
+enum class IsSyntheticClick : bool { No, Yes };
 enum class SelectionRestorationMode : uint8_t;
 
 struct GetAnimationsOptions;
@@ -533,7 +534,7 @@
     IntPoint savedLayerScrollPosition() const;
     void setSavedLayerScrollPosition(const IntPoint&);
 
-    bool dispatchMouseEvent(const PlatformMouseEvent&, const AtomString& eventType, int clickCount = 0, Element* relatedTarget = nullptr);
+    bool dispatchMouseEvent(const PlatformMouseEvent&, const AtomString& eventType, int clickCount = 0, Element* relatedTarget = nullptr, IsSyntheticClick isSyntethicClick = IsSyntheticClick::No);
     bool dispatchWheelEvent(const PlatformWheelEvent&, OptionSet<EventHandling>&, Event::IsCancelable = Event::IsCancelable::Yes);
     bool dispatchKeyEvent(const PlatformKeyboardEvent&);
     bool dispatchSimulatedClick(Event* underlyingEvent, SimulatedClickMouseEventOptions = SendNoEvents, SimulatedClickVisualOptions = ShowPressedLook);

Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.h (279003 => 279004)


--- trunk/Source/WebCore/loader/ResourceLoadObserver.h	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.h	2021-06-17 20:03:07 UTC (rev 279004)
@@ -71,9 +71,6 @@
     virtual void setDomainsWithCrossPageStorageAccess(HashMap<TopFrameDomain, SubResourceDomain>&&, CompletionHandler<void()>&& completionHandler) { completionHandler(); }
     virtual bool hasCrossPageStorageAccess(const SubResourceDomain&, const TopFrameDomain&) const { return false; }
     virtual bool hasHadUserInteraction(const RegistrableDomain&) const { return false; }
-    
-    virtual void setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubResourceDomain>&&, CompletionHandler<void()>&& completionHandler) { completionHandler(); }
-    virtual bool hasDeniedCrossPageStorageAccess(const SubResourceDomain&, const TopFrameDomain&) const { return false; }
 };
     
 } // namespace WebCore

Modified: trunk/Source/WebCore/page/Quirks.cpp (279003 => 279004)


--- trunk/Source/WebCore/page/Quirks.cpp	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/page/Quirks.cpp	2021-06-17 20:03:07 UTC (rev 279004)
@@ -1068,6 +1068,7 @@
     if (url.host() == "www.playstation.com"_s || url.host() == "my.playstation.com"_s) {
         return element.hasClass()
         && (element.classNames().contains("web-toolbar__signin-button")
+        || element.classNames().contains("web-toolbar__signin-button-label")
         || element.classNames().contains("sb-signin-button"));
     }
 
@@ -1083,15 +1084,6 @@
     return true;
 }
 
-static bool hasDeniedCrossPageStorageAccess(const HashSet<RegistrableDomain>& loginDomains, const RegistrableDomain& topFrameDomain)
-{
-    for (auto& loginDomain : loginDomains) {
-        if (ResourceLoadObserver::shared().hasDeniedCrossPageStorageAccess(loginDomain, topFrameDomain))
-            return true;
-    }
-    return false;
-}
-
 const String& Quirks::BBCRadioPlayerURLString()
 {
     static NeverDestroyed<String> BBCRadioPlayerURLString = "https://www.bbc.co.uk/sounds/player/bbc_world_service"_s;
@@ -1127,8 +1119,8 @@
         completionHandler(ShouldDispatchClick::No);
         return Quirks::StorageAccessResult::ShouldNotCancelEvent;
     }
-    if (hasStorageAccessForAllLoginDomains(*domainsInNeedOfStorageAccess, firstPartyDomain)
-        || hasDeniedCrossPageStorageAccess(*domainsInNeedOfStorageAccess, firstPartyDomain)) {
+
+    if (hasStorageAccessForAllLoginDomains(*domainsInNeedOfStorageAccess, firstPartyDomain)) {
         completionHandler(ShouldDispatchClick::No);
         return Quirks::StorageAccessResult::ShouldNotCancelEvent;
     }
@@ -1142,9 +1134,7 @@
 
     DocumentStorageAccess::requestStorageAccessForNonDocumentQuirk(*m_document, WTFMove(domainInNeedOfStorageAccess), [firstPartyDomain, domainInNeedOfStorageAccess, completionHandler = WTFMove(completionHandler)](StorageAccessWasGranted storageAccessGranted) mutable {
         if (storageAccessGranted == StorageAccessWasGranted::No) {
-            ResourceLoadObserver::shared().setHasDeniedCrossPageStorageAccess({{ firstPartyDomain, domainInNeedOfStorageAccess }}, [completionHandler = WTFMove(completionHandler)] () mutable {
-                completionHandler(ShouldDispatchClick::Yes);
-            });
+            completionHandler(ShouldDispatchClick::Yes);
             return;
         }
 
@@ -1156,7 +1146,7 @@
 }
 #endif
 
-Quirks::StorageAccessResult Quirks::triggerOptionalStorageAccessQuirk(Element& element, const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget, bool isParentProcessAFullWebBrowser) const
+Quirks::StorageAccessResult Quirks::triggerOptionalStorageAccessQuirk(Element& element, const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget, bool isParentProcessAFullWebBrowser, IsSyntheticClick isSyntheticClick) const
 {
     if (!DeprecatedGlobalSettings::resourceLoadStatisticsEnabled() || !isParentProcessAFullWebBrowser)
         return Quirks::StorageAccessResult::ShouldNotCancelEvent;
@@ -1232,7 +1222,8 @@
             }
         }
 
-        if (isStorageAccessQuirkDomainAndElement(m_document->url(), element)) {
+        // If the click is synthetic, the user has already gone through the storage access flow and we should not request again.
+        if (isStorageAccessQuirkDomainAndElement(m_document->url(), element) && isSyntheticClick == IsSyntheticClick::No) {
             return requestStorageAccessAndHandleClick([element = makeWeakPtr(element), platformEvent, eventType, detail, relatedTarget] (ShouldDispatchClick shouldDispatchClick) mutable {
                 RefPtr protectedElement { element.get() };
                 if (!protectedElement)
@@ -1239,7 +1230,7 @@
                     return;
 
                 if (shouldDispatchClick == ShouldDispatchClick::Yes)
-                    protectedElement->dispatchMouseEvent(platformEvent, eventType, detail, relatedTarget);
+                    protectedElement->dispatchMouseEvent(platformEvent, eventType, detail, relatedTarget, IsSyntheticClick::Yes);
             });
         }
 

Modified: trunk/Source/WebCore/page/Quirks.h (279003 => 279004)


--- trunk/Source/WebCore/page/Quirks.h	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebCore/page/Quirks.h	2021-06-17 20:03:07 UTC (rev 279004)
@@ -44,6 +44,8 @@
 enum class StorageAccessWasGranted : bool;
 #endif
 
+enum class IsSyntheticClick : bool;
+
 class Quirks {
     WTF_MAKE_NONCOPYABLE(Quirks); WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -122,7 +124,7 @@
 
     enum StorageAccessResult : bool { ShouldNotCancelEvent, ShouldCancelEvent };
     enum ShouldDispatchClick : bool { No, Yes };
-    StorageAccessResult triggerOptionalStorageAccessQuirk(Element&, const PlatformMouseEvent&, const AtomString& eventType, int, Element*, bool isParentProcessAFullWebBrowser) const;
+    StorageAccessResult triggerOptionalStorageAccessQuirk(Element&, const PlatformMouseEvent&, const AtomString& eventType, int, Element*, bool isParentProcessAFullWebBrowser, IsSyntheticClick) const;
 
     bool needsVP9FullRangeFlagQuirk() const;
     bool needsHDRPixelDepthQuirk() const;

Modified: trunk/Source/WebKit/ChangeLog (279003 => 279004)


--- trunk/Source/WebKit/ChangeLog	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebKit/ChangeLog	2021-06-17 20:03:07 UTC (rev 279004)
@@ -1,3 +1,22 @@
+2021-06-17  Kate Cheney  <katherine_che...@apple.com>
+
+        Storage Access quirks should prompt up to twice if a user does not allow storage access
+        https://bugs.webkit.org/show_bug.cgi?id=227099
+        <rdar://problem/79409843>
+
+        Reviewed by John Wilander.
+
+        We should allow storage access prompts twice per document for quirks
+        just like for non-quirk cases. Previously we were only presenting
+        the quirk prompt once per web content process if the user denied. This
+        patch removes this code and utilizes maxNumberOfTimesExplicitlyDeniedStorageAccess 
+        instead.
+
+        * WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:
+        (WebKit::WebResourceLoadObserver::hasDeniedCrossPageStorageAccess const): Deleted.
+        (WebKit::WebResourceLoadObserver::setHasDeniedCrossPageStorageAccess): Deleted.
+        * WebProcess/WebCoreSupport/WebResourceLoadObserver.h:
+
 2021-06-17  Tim Horton  <timothy_hor...@apple.com>
 
         Adopt WKHoverGestureRecognizer

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp (279003 => 279004)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp	2021-06-17 20:03:07 UTC (rev 279004)
@@ -443,25 +443,6 @@
     completionHandler();
 }
 
-bool WebResourceLoadObserver::hasDeniedCrossPageStorageAccess(const SubFrameDomain& subDomain, const TopFrameDomain& topDomain) const
-{
-    auto it = m_domainsWithDeniedStorageAccess.find(topDomain);
-
-    if (it != m_domainsWithDeniedStorageAccess.end())
-        return it->value.contains(subDomain);
-
-    return false;
-}
-
-void WebResourceLoadObserver::setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&& domains, CompletionHandler<void()>&& completionHandler)
-{
-    for (auto& topDomain : domains.keys()) {
-        m_domainsWithDeniedStorageAccess.ensure(topDomain, [] { return HashSet<RegistrableDomain> { };
-            }).iterator->value.add(domains.get(topDomain));
-    }
-    completionHandler();
-}
-
 } // namespace WebKit
 
 #endif // ENABLE(RESOURCE_LOAD_STATISTICS)

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h (279003 => 279004)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h	2021-06-17 20:00:49 UTC (rev 279003)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h	2021-06-17 20:03:07 UTC (rev 279004)
@@ -69,8 +69,6 @@
     void setDomainsWithCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&&, CompletionHandler<void()>&&) final;
     bool hasHadUserInteraction(const WebCore::RegistrableDomain&) const final;
     bool hasCrossPageStorageAccess(const SubFrameDomain&, const TopFrameDomain&) const final;
-    void setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&&, CompletionHandler<void()>&&) final;
-    bool hasDeniedCrossPageStorageAccess(const SubFrameDomain&, const TopFrameDomain&) const final;
 
 private:
     WebCore::ResourceLoadStatistics& ensureResourceStatisticsForRegistrableDomain(const WebCore::RegistrableDomain&);
@@ -90,7 +88,6 @@
 
     HashSet<WebCore::RegistrableDomain> m_domainsWithUserInteraction;
     HashMap<TopFrameDomain, HashSet<SubFrameDomain>> m_domainsWithCrossPageStorageAccess;
-    HashMap<TopFrameDomain, HashSet<SubFrameDomain>> m_domainsWithDeniedStorageAccess;
 #if !RELEASE_LOG_DISABLED
     uint64_t m_loggingCounter { 0 };
     static bool shouldLogUserInteraction;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to