⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 277376 in webkit


Ignore:
Timestamp:
May 12, 2021, 10:52:38 AM (5 years ago)
Author:
Chris Dumez
Message:

Queue notification permission requests for the same origin on WebKit side
​https://bugs.webkit.org/show_bug.cgi?id=225701
<rdar://76804977>

Reviewed by Geoffrey Garen.

Source/WebCore:

Remove some dead code.

  • Modules/notifications/NotificationClient.h:

Source/WebKit:

If there are parallel notification permission requests for the same origin, we now queue them on WebKit
side and only ask the client once for the origin. Once we've received the permission from the client,
we respond to all JS requests at this point.

This patch also removes some dead code to facilitate refactoring the code to support this.
In a follow-up I am planning to use sendWithAsyncReply() and refactor this code further.

  • WebProcess/Notifications/NotificationPermissionRequestManager.cpp:

(WebKit::NotificationPermissionRequestManager::startRequest):
(WebKit::NotificationPermissionRequestManager::permissionLevel):
(WebKit::NotificationPermissionRequestManager::didReceiveNotificationPermissionDecision):

  • WebProcess/Notifications/NotificationPermissionRequestManager.h:
  • WebProcess/Notifications/WebNotificationManager.cpp:

(WebKit::WebNotificationManager::policyForOrigin const):

  • WebProcess/Notifications/WebNotificationManager.h:
  • WebProcess/WebCoreSupport/WebNotificationClient.cpp:

(WebKit::WebNotificationClient::requestPermission):
(WebKit::WebNotificationClient::checkPermission):

  • WebProcess/WebCoreSupport/WebNotificationClient.h:

Source/WebKitLegacy/mac:

Remove some dead code.

  • WebCoreSupport/WebNotificationClient.h:
  • WebCoreSupport/WebNotificationClient.mm:

Source/WebKitLegacy/win:

Remove some dead code.

  • WebCoreSupport/WebDesktopNotificationsDelegate.cpp:
  • WebCoreSupport/WebDesktopNotificationsDelegate.h:

Tools:

Add API test coverage.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebKitCocoa/NotificationAPI.mm: Added.

(-[NotificationPermissionMessageHandler userContentController:didReceiveScriptMessage:]):
(-[NotificationPermissionUIDelegate initWithHandler:]):
(-[NotificationPermissionUIDelegate _webView:requestNotificationPermissionForSecurityOrigin:decisionHandler:]):
(TestWebKitAPI::runRequestPermissionTest):
(TestWebKitAPI::TEST):
(TestWebKitAPI::runParallelPermissionRequestsTest):

Location:
trunk
Files:
1 added
17 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r277373 r277376  
     12021-05-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Queue notification permission requests for the same origin on WebKit side
     4        https://bugs.webkit.org/show_bug.cgi?id=225701
     5        <rdar://76804977>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Remove some dead code.
     10
     11        * Modules/notifications/NotificationClient.h:
     12
    1132021-05-12  Ryosuke Niwa  <rniwa@webkit.org>
    214

  • trunk/Source/WebCore/Modules/notifications/NotificationClient.h

    r223728 r277376  
    6969    virtual void requestPermission(ScriptExecutionContext*, RefPtr<NotificationPermissionCallback>&&) = 0;
    7070
    71     virtual bool hasPendingPermissionRequests(ScriptExecutionContext*) const = 0;
    72 
    73     // Cancel all outstanding requests for the ScriptExecutionContext
    74     virtual void cancelRequestsForPermission(ScriptExecutionContext*) = 0;
    75 
    7671    // Checks the current level of permission.
    7772    virtual Permission checkPermission(ScriptExecutionContext*) = 0;

  • trunk/Source/WebKit/ChangeLog

    r277375 r277376  
     12021-05-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Queue notification permission requests for the same origin on WebKit side
     4        https://bugs.webkit.org/show_bug.cgi?id=225701
     5        <rdar://76804977>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        If there are parallel notification permission requests for the same origin, we now queue them on WebKit
     10        side and only ask the client once for the origin. Once we've received the permission from the client,
     11        we respond to all JS requests at this point.
     12
     13        This patch also removes some dead code to facilitate refactoring the code to support this.
     14        In a follow-up I am planning to use sendWithAsyncReply() and refactor this code further.
     15
     16        * WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
     17        (WebKit::NotificationPermissionRequestManager::startRequest):
     18        (WebKit::NotificationPermissionRequestManager::permissionLevel):
     19        (WebKit::NotificationPermissionRequestManager::didReceiveNotificationPermissionDecision):
     20        * WebProcess/Notifications/NotificationPermissionRequestManager.h:
     21        * WebProcess/Notifications/WebNotificationManager.cpp:
     22        (WebKit::WebNotificationManager::policyForOrigin const):
     23        * WebProcess/Notifications/WebNotificationManager.h:
     24        * WebProcess/WebCoreSupport/WebNotificationClient.cpp:
     25        (WebKit::WebNotificationClient::requestPermission):
     26        (WebKit::WebNotificationClient::checkPermission):
     27        * WebProcess/WebCoreSupport/WebNotificationClient.h:
     28
    1292021-05-12  Julian Gonzalez  <julian_a_gonzalez@apple.com>
    230

  • trunk/Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.cpp

    r235205 r277376  
    6969
    7070#if ENABLE(NOTIFICATIONS)
    71 void NotificationPermissionRequestManager::startRequest(SecurityOrigin* origin, RefPtr<NotificationPermissionCallback>&& callback)
     71void NotificationPermissionRequestManager::startRequest(const SecurityOriginData& securityOrigin, RefPtr<NotificationPermissionCallback>&& callback)
    7272{
    73     auto permission = permissionLevel(origin);
     73    auto permission = permissionLevel(securityOrigin);
    7474    if (permission != NotificationClient::Permission::Default) {
    7575        if (callback)
    … …  
    7878    }
    7979
     80    auto addResult = m_requestsPerOrigin.add(securityOrigin, Vector<RefPtr<WebCore::NotificationPermissionCallback>> { });
     81    addResult.iterator->value.append(WTFMove(callback));
     82    if (!addResult.isNewEntry)
     83        return;
     84
    8085    uint64_t requestID = generateRequestID();
    81     m_originToIDMap.set(origin, requestID);
    82     m_idToOriginMap.set(requestID, origin);
    83     m_idToCallbackMap.set(requestID, WTFMove(callback));
    84     m_page->send(Messages::WebPageProxy::RequestNotificationPermission(requestID, origin->toString()));
     86    m_idToOriginMap.set(requestID, securityOrigin);
     87
     88    // FIXME: This should use sendWithAsyncReply().
     89    m_page->send(Messages::WebPageProxy::RequestNotificationPermission(requestID, securityOrigin.toString()));
    8590}
    8691#endif
    8792
    88 void NotificationPermissionRequestManager::cancelRequest(SecurityOrigin* origin)
    89 {
    90 #if ENABLE(NOTIFICATIONS)
    91     uint64_t id = m_originToIDMap.take(origin);
    92     if (!id)
    93         return;
    94    
    95     m_idToOriginMap.remove(id);
    96     m_idToCallbackMap.remove(id);
    97 #else
    98     UNUSED_PARAM(origin);
    99 #endif
    100 }
    101 
    102 bool NotificationPermissionRequestManager::hasPendingPermissionRequests(SecurityOrigin* origin) const
    103 {
    104 #if ENABLE(NOTIFICATIONS)
    105     return m_originToIDMap.contains(origin);
    106 #else
    107     UNUSED_PARAM(origin);
    108     return false;
    109 #endif
    110 }
    111 
    112 NotificationClient::Permission NotificationPermissionRequestManager::permissionLevel(SecurityOrigin* securityOrigin)
     93NotificationClient::Permission NotificationPermissionRequestManager::permissionLevel(const SecurityOriginData& securityOrigin)
    11394{
    11495#if ENABLE(NOTIFICATIONS)
    … …  
    11697        return NotificationClient::Permission::Denied;
    11798   
    118     return WebProcess::singleton().supplement<WebNotificationManager>()->policyForOrigin(securityOrigin);
     99    return WebProcess::singleton().supplement<WebNotificationManager>()->policyForOrigin(securityOrigin.toString());
    119100#else
    120101    UNUSED_PARAM(securityOrigin);
    … …  
    146127        return;
    147128
    148     RefPtr<WebCore::SecurityOrigin> origin = m_idToOriginMap.take(requestID);
    149     if (!origin)
     129    auto origin = m_idToOriginMap.take(requestID);
     130    if (origin.isEmpty())
    150131        return;
    151132
    152     m_originToIDMap.remove(origin);
     133    WebProcess::singleton().supplement<WebNotificationManager>()->didUpdateNotificationDecision(origin.toString(), allowed);
    153134
    154     WebProcess::singleton().supplement<WebNotificationManager>()->didUpdateNotificationDecision(origin->toString(), allowed);
    155 
    156     RefPtr<NotificationPermissionCallback> callback = m_idToCallbackMap.take(requestID);
    157     if (!callback)
    158         return;
    159    
    160     callback->handleEvent(allowed ? NotificationClient::Permission::Granted : NotificationClient::Permission::Denied);
     135    auto callbacks = m_requestsPerOrigin.take(origin);
     136    for (auto& callback : callbacks) {
     137        if (!callback)
     138            return;
     139        callback->handleEvent(allowed ? NotificationClient::Permission::Granted : NotificationClient::Permission::Denied);
     140    }
    161141#else
    162142    UNUSED_PARAM(requestID);

  • trunk/Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.h

    r272784 r277376  
    2929#include <WebCore/NotificationClient.h>
    3030#include <WebCore/NotificationPermissionCallback.h>
    31 #include <WebCore/SecurityOriginHash.h>
     31#include <WebCore/SecurityOriginData.h>
    3232#include <wtf/HashMap.h>
    3333#include <wtf/RefCounted.h>
    … …  
    3636
    3737namespace WebCore {
    38 class Notification;   
    39 class SecurityOrigin;
     38class Notification;
    4039}
    4140
    … …  
    5049
    5150#if ENABLE(NOTIFICATIONS)
    52     void startRequest(WebCore::SecurityOrigin*, RefPtr<WebCore::NotificationPermissionCallback>&&);
     51    void startRequest(const WebCore::SecurityOriginData&, RefPtr<WebCore::NotificationPermissionCallback>&&);
    5352#endif
    54     void cancelRequest(WebCore::SecurityOrigin*);
    55     bool hasPendingPermissionRequests(WebCore::SecurityOrigin*) const;
    5653   
    57     WebCore::NotificationClient::Permission permissionLevel(WebCore::SecurityOrigin*);
     54    WebCore::NotificationClient::Permission permissionLevel(const WebCore::SecurityOriginData&);
    5855
    5956    // For testing purposes only.
    … …  
    6764
    6865#if ENABLE(NOTIFICATIONS)
    69     HashMap<uint64_t, RefPtr<WebCore::NotificationPermissionCallback>> m_idToCallbackMap;
    70 #endif
    71     HashMap<RefPtr<WebCore::SecurityOrigin>, uint64_t> m_originToIDMap;
    72     HashMap<uint64_t, RefPtr<WebCore::SecurityOrigin>> m_idToOriginMap;
    73 
    74 #if ENABLE(NOTIFICATIONS)
     66    HashMap<WebCore::SecurityOriginData, Vector<RefPtr<WebCore::NotificationPermissionCallback>>> m_requestsPerOrigin;
     67    HashMap<uint64_t, WebCore::SecurityOriginData> m_idToOriginMap;
    7568    WebPage* m_page;
    7669#endif

  • trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.cpp

    r276108 r277376  
    102102}
    103103
    104 NotificationClient::Permission WebNotificationManager::policyForOrigin(WebCore::SecurityOrigin* origin) const
    105 {
    106 #if ENABLE(NOTIFICATIONS)
    107     if (!origin)
    108         return NotificationClient::Permission::Default;
    109 
    110     ASSERT(!origin->isUnique());
    111 
    112     auto originString = origin->toRawString();
    113     if (!decltype(m_permissionsMap)::isValidKey(originString))
     104NotificationClient::Permission WebNotificationManager::policyForOrigin(const String& originString) const
     105{
     106#if ENABLE(NOTIFICATIONS)
     107    if (!originString)
    114108        return NotificationClient::Permission::Default;
    115109
    … …  
    118112        return it->value ? NotificationClient::Permission::Granted : NotificationClient::Permission::Denied;
    119113#else
    120     UNUSED_PARAM(origin);
     114    UNUSED_PARAM(originString);
    121115#endif
    122116   

  • trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.h

    r248762 r277376  
    6363
    6464    // Looks in local cache for permission. If not found, returns DefaultDenied.
    65     WebCore::NotificationClient::Permission policyForOrigin(WebCore::SecurityOrigin*) const;
     65    WebCore::NotificationClient::Permission policyForOrigin(const String& originString) const;
    6666
    6767    void removeAllPermissionsForTesting();

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp

    r235205 r277376  
    7474void WebNotificationClient::requestPermission(ScriptExecutionContext* context, RefPtr<NotificationPermissionCallback>&& callback)
    7575{
    76     m_page->notificationPermissionRequestManager()->startRequest(context->securityOrigin(), WTFMove(callback));
    77 }
    78 
    79 bool WebNotificationClient::hasPendingPermissionRequests(ScriptExecutionContext* context) const
    80 {
    81     return m_page->notificationPermissionRequestManager()->hasPendingPermissionRequests(context->securityOrigin());
    82 }
    83 
    84 void WebNotificationClient::cancelRequestsForPermission(ScriptExecutionContext* context)
    85 {
    86     m_page->notificationPermissionRequestManager()->cancelRequest(context->securityOrigin());
     76    auto* securityOrigin = context->securityOrigin();
     77    if (!securityOrigin) {
     78        if (callback)
     79            callback->handleEvent(NotificationClient::Permission::Denied);
     80        return;
     81    }
     82    m_page->notificationPermissionRequestManager()->startRequest(securityOrigin->data(), WTFMove(callback));
    8783}
    8884
    8985NotificationClient::Permission WebNotificationClient::checkPermission(ScriptExecutionContext* context)
    9086{
    91     if (!context || !context->isDocument())
     87    if (!context || !context->isDocument() || !context->securityOrigin())
    9288        return NotificationClient::Permission::Denied;
    93     return m_page->notificationPermissionRequestManager()->permissionLevel(context->securityOrigin());
     89    return m_page->notificationPermissionRequestManager()->permissionLevel(context->securityOrigin()->data());
    9490}
    9591

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.h

    r272784 r277376  
    5353    void notificationControllerDestroyed() override;
    5454    void requestPermission(WebCore::ScriptExecutionContext*, RefPtr<WebCore::NotificationPermissionCallback>&&) override;
    55     void cancelRequestsForPermission(WebCore::ScriptExecutionContext*) override;
    56     bool hasPendingPermissionRequests(WebCore::ScriptExecutionContext*) const override;
    5755    WebCore::NotificationClient::Permission checkPermission(WebCore::ScriptExecutionContext*) override;
    5856   

  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r277295 r277376  
     12021-05-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Queue notification permission requests for the same origin on WebKit side
     4        https://bugs.webkit.org/show_bug.cgi?id=225701
     5        <rdar://76804977>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Remove some dead code.
     10
     11        * WebCoreSupport/WebNotificationClient.h:
     12        * WebCoreSupport/WebNotificationClient.mm:
     13
    1142021-05-10  Wenson Hsieh  <wenson_hsieh@apple.com>
    215

  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebNotificationClient.h

    r248762 r277376  
    5252    void notificationControllerDestroyed() override;
    5353    void requestPermission(WebCore::ScriptExecutionContext*, RefPtr<WebCore::NotificationPermissionCallback>&&) override;
    54     void cancelRequestsForPermission(WebCore::ScriptExecutionContext*) override { }
    55     bool hasPendingPermissionRequests(WebCore::ScriptExecutionContext*) const override;
    5654    WebCore::NotificationClient::Permission checkPermission(WebCore::ScriptExecutionContext*) override;
    5755

  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebNotificationClient.mm

    r272789 r277376  
    136136}
    137137
    138 bool WebNotificationClient::hasPendingPermissionRequests(ScriptExecutionContext*) const
    139 {
    140     // We know permission was requested but we don't know if the client responded. In this case, we play it
    141     // safe and presume there is one pending so that ActiveDOMObjects don't get suspended.
    142     return m_everRequestedPermission;
    143 }
    144 
    145138void WebNotificationClient::requestPermission(ScriptExecutionContext* context, RefPtr<NotificationPermissionCallback>&& callback)
    146139{

  • trunk/Source/WebKitLegacy/win/ChangeLog

    r277357 r277376  
     12021-05-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Queue notification permission requests for the same origin on WebKit side
     4        https://bugs.webkit.org/show_bug.cgi?id=225701
     5        <rdar://76804977>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Remove some dead code.
     10
     11        * WebCoreSupport/WebDesktopNotificationsDelegate.cpp:
     12        * WebCoreSupport/WebDesktopNotificationsDelegate.h:
     13
    1142021-05-11  Chris Dumez  <cdumez@apple.com>
    215

  • trunk/Source/WebKitLegacy/win/WebCoreSupport/WebDesktopNotificationsDelegate.cpp

    r238771 r277376  
    182182}
    183183
    184 void WebDesktopNotificationsDelegate::cancelRequestsForPermission(ScriptExecutionContext* context)
    185 {
    186 }
    187 
    188 bool hasPendingPermissionRequests(ScriptExecutionContext*) const
    189 {
    190     // We can safely return false here because our implementation for requestPermission() never calls
    191     // the completion callback.
    192     return false;
    193 }
    194 
    195184NotificationClient::Permission WebDesktopNotificationsDelegate::checkPermission(const URL& url)
    196185{

  • trunk/Source/WebKitLegacy/win/WebCoreSupport/WebDesktopNotificationsDelegate.h

    r254836 r277376  
    5555    virtual void notificationControllerDestroyed();
    5656    virtual void requestPermission(WebCore::SecurityOrigin*, RefPtr<WebCore::NotificationPermissionCallback>&&);
    57     bool hasPendingPermissionRequests(WebCore::ScriptExecutionContext*) const override;
    58     virtual void cancelRequestsForPermission(WebCore::ScriptExecutionContext*);
    5957    virtual WebCore::NotificationClient::Permission checkPermission(const URL&);
    6058

  • trunk/Tools/ChangeLog

    r277357 r277376  
     12021-05-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Queue notification permission requests for the same origin on WebKit side
     4        https://bugs.webkit.org/show_bug.cgi?id=225701
     5        <rdar://76804977>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Add API test coverage.
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/WebKitCocoa/NotificationAPI.mm: Added.
     13        (-[NotificationPermissionMessageHandler userContentController:didReceiveScriptMessage:]):
     14        (-[NotificationPermissionUIDelegate initWithHandler:]):
     15        (-[NotificationPermissionUIDelegate _webView:requestNotificationPermissionForSecurityOrigin:decisionHandler:]):
     16        (TestWebKitAPI::runRequestPermissionTest):
     17        (TestWebKitAPI::TEST):
     18        (TestWebKitAPI::runParallelPermissionRequestsTest):
     19
    1202021-05-11  Chris Dumez  <cdumez@apple.com>
    221

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r277356 r277376  
    273273                46A44A5425A7830300F61E16 /* webaudio-createMediaElementSource.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46A44A5325A782DD00F61E16 /* webaudio-createMediaElementSource.html */; };
    274274                46A46A1A2575645600A1B118 /* SessionStorage.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A46A192575645600A1B118 /* SessionStorage.mm */; };
     275                46A80F26264C29D400EEF20D /* NotificationAPI.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A80F25264C29D400EEF20D /* NotificationAPI.mm */; };
    275276                46A911592108E6780078D40D /* CustomUserAgent.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A911582108E66B0078D40D /* CustomUserAgent.mm */; };
    276277                46BBEA1B25F9835700D4987A /* WorkQueue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7AA6A1511AAC0B31002B2ED3 /* WorkQueue.cpp */; };
    … …  
    20812082                46A44A5325A782DD00F61E16 /* webaudio-createMediaElementSource.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "webaudio-createMediaElementSource.html"; sourceTree = "<group>"; };
    20822083                46A46A192575645600A1B118 /* SessionStorage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SessionStorage.mm; sourceTree = "<group>"; };
     2084                46A80F25264C29D400EEF20D /* NotificationAPI.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NotificationAPI.mm; sourceTree = "<group>"; };
    20832085                46A911582108E66B0078D40D /* CustomUserAgent.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CustomUserAgent.mm; sourceTree = "<group>"; };
    20842086                46C1EA9725758805005E409E /* alert.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = alert.html; sourceTree = "<group>"; };
    … …  
    34823484                                5C8BC798218CF3E900813886 /* NetworkProcess.mm */,
    34833485                                5CAE4637201937CD0051610F /* NetworkProcessCrashNonPersistentDataStore.mm */,
     3486                                46A80F25264C29D400EEF20D /* NotificationAPI.mm */,
    34843487                                CDCFFEC022E268D500DF4223 /* NoPauseWhenSwitchingTabs.mm */,
    34853488                                CD2D0D19213465560018C784 /* NowPlaying.mm */,
    … …  
    54505453                                CDE77D2525A6591C00D4115E /* FullscreenPointerLeave.mm in Sources */,
    54515454                                CDDC7C6925FFF6D000224278 /* FullscreenRemoveNodeBeforeEnter.mm in Sources */,
     5455                                46A80F26264C29D400EEF20D /* NotificationAPI.mm in Sources */,
    54525456                                CDBFCC451A9FF45300A7B691 /* FullscreenZoomInitialFrame.mm in Sources */,
    54535457                                83DB79691EF63B3C00BFA5E5 /* Function.cpp in Sources */,
Note: See TracChangeset for help on using the changeset viewer.