Changeset 256900 in webkit


Ignore:
Timestamp:
Feb 18, 2020 7:26:28 PM (4 years ago)
Author:
youenn@apple.com
Message:

SWServer::claim should check for the service worker to be active
​https://bugs.webkit.org/show_bug.cgi?id=207739
<rdar://problem/45441129>

Reviewed by Alex Christensen.

Source/WebCore:

claim is only working for service workers that are active.
But there might be a time when a service worker is active in its web process but redundant in networking process.
Thus, we need to move the check from WebProcess to NetworkProcess.

  • workers/service/ServiceWorkerClients.cpp:

(WebCore::ServiceWorkerClients::claim):

  • workers/service/context/SWContextManager.h:
  • workers/service/server/SWServer.cpp:

(WebCore::SWServer::claim):

  • workers/service/server/SWServer.h:
  • workers/service/server/SWServerToContextConnection.cpp:

(WebCore::SWServerToContextConnection::claim):

  • workers/service/server/SWServerToContextConnection.h:
  • workers/service/server/SWServerWorker.cpp:

(WebCore::SWServerWorker::claim): Deleted.

  • workers/service/server/SWServerWorker.h:

(WebCore::SWServerWorker::isActive const):

Source/WebKit:

Use Async Reply to remove the need for a map and passing integers around.

  • NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:

(WebKit::WebSWServerToContextConnection::claimCompleted): Deleted.

  • NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
  • NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in:
  • WebProcess/Storage/WebSWContextManagerConnection.cpp:

(WebKit::WebSWContextManagerConnection::claim):
(WebKit::WebSWContextManagerConnection::claimCompleted): Deleted.

  • WebProcess/Storage/WebSWContextManagerConnection.h:
  • WebProcess/Storage/WebSWContextManagerConnection.messages.in:
Location:
trunk/Source
Files:
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r256895 r256900  
     12020-02-18  Youenn Fablet  <youenn@apple.com>
     2
     3        SWServer::claim should check for the service worker to be active
     4        https://bugs.webkit.org/show_bug.cgi?id=207739
     5        <rdar://problem/45441129>
     6
     7        Reviewed by Alex Christensen.
     8
     9        claim is only working for service workers that are active.
     10        But there might be a time when a service worker is active in its web process but redundant in networking process.
     11        Thus, we need to move the check from WebProcess to NetworkProcess.
     12
     13        * workers/service/ServiceWorkerClients.cpp:
     14        (WebCore::ServiceWorkerClients::claim):
     15        * workers/service/context/SWContextManager.h:
     16        * workers/service/server/SWServer.cpp:
     17        (WebCore::SWServer::claim):
     18        * workers/service/server/SWServer.h:
     19        * workers/service/server/SWServerToContextConnection.cpp:
     20        (WebCore::SWServerToContextConnection::claim):
     21        * workers/service/server/SWServerToContextConnection.h:
     22        * workers/service/server/SWServerWorker.cpp:
     23        (WebCore::SWServerWorker::claim): Deleted.
     24        * workers/service/server/SWServerWorker.h:
     25        (WebCore::SWServerWorker::isActive const):
     26
    1272020-02-18  Zalan Bujtas  <zalan@apple.com>
    228

  • trunk/Source/WebCore/workers/service/ServiceWorkerClients.cpp

    r244115 r256900  
    117117    auto serviceWorkerIdentifier = serviceWorkerGlobalScope.thread().identifier();
    118118
    119     if (!serviceWorkerGlobalScope.registration().active() || serviceWorkerGlobalScope.registration().active()->identifier() != serviceWorkerIdentifier) {
    120         promise->reject(Exception { InvalidStateError, "Service worker is not active"_s });
    121         return;
    122     }
    123 
    124119    auto promisePointer = promise.ptr();
    125120    m_pendingPromises.add(promisePointer, WTFMove(promise));
    … …  
    127122    callOnMainThread([promisePointer, serviceWorkerIdentifier] () mutable {
    128123        auto connection = SWContextManager::singleton().connection();
    129         connection->claim(serviceWorkerIdentifier, [promisePointer, serviceWorkerIdentifier] () mutable {
    130             SWContextManager::singleton().postTaskToServiceWorker(serviceWorkerIdentifier, [promisePointer] (auto& scope) mutable {
    131                 if (auto promise = scope.clients().m_pendingPromises.take(promisePointer))
    132                     promise.value()->resolve();
     124        connection->claim(serviceWorkerIdentifier, [promisePointer, serviceWorkerIdentifier](auto&& result) mutable {
     125            SWContextManager::singleton().postTaskToServiceWorker(serviceWorkerIdentifier, [promisePointer, result = isolatedCopy(WTFMove(result))](auto& scope) mutable {
     126                if (auto promise = scope.clients().m_pendingPromises.take(promisePointer)) {
     127                    DOMPromiseDeferred<void> pendingPromise { WTFMove(promise.value()) };
     128                    pendingPromise.settle(WTFMove(result));
     129                }
    133130            });
    134131        });

  • trunk/Source/WebCore/workers/service/context/SWContextManager.h

    r255681 r256900  
    6464        virtual void findClientByIdentifier(ServiceWorkerIdentifier, ServiceWorkerClientIdentifier, FindClientByIdentifierCallback&&) = 0;
    6565        virtual void matchAll(ServiceWorkerIdentifier, const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&) = 0;
    66         virtual void claim(ServiceWorkerIdentifier, CompletionHandler<void()>&&) = 0;
     66        virtual void claim(ServiceWorkerIdentifier, CompletionHandler<void(ExceptionOr<void>&&)>&&) = 0;
    6767
    6868        virtual void didFailHeartBeatCheck(ServiceWorkerIdentifier) = 0;

  • trunk/Source/WebCore/workers/service/server/SWServer.cpp

    r256664 r256900  
    560560}
    561561
    562 void SWServer::claim(SWServerWorker& worker)
    563 {
     562Optional<ExceptionData> SWServer::claim(SWServerWorker& worker)
     563{
     564    if (!worker.isActive())
     565        return ExceptionData { InvalidStateError, "Service worker is not active"_s };
     566
    564567    auto& origin = worker.origin();
    565568    forEachClientForOrigin(origin, [&](auto& clientData) {
    … …  
    579582        registration->controlClient(clientData.identifier);
    580583    });
     584    return { };
    581585}
    582586

  • trunk/Source/WebCore/workers/service/server/SWServer.h

    r256749 r256900  
    3030#include "ClientOrigin.h"
    3131#include "DocumentIdentifier.h"
     32#include "ExceptionOr.h"
    3233#include "SWServerWorker.h"
    3334#include "SecurityOriginData.h"
    … …  
    179180    void workerContextTerminated(SWServerWorker&);
    180181    void matchAll(SWServerWorker&, const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
    181     void claim(SWServerWorker&);
     182    Optional<ExceptionData> claim(SWServerWorker&);
    182183
    183184    WEBCORE_EXPORT static HashSet<SWServer*>& allServers();

  • trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp

    r253898 r256900  
    102102}
    103103
    104 void SWServerToContextConnection::claim(uint64_t requestIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier)
     104void SWServerToContextConnection::claim(ServiceWorkerIdentifier serviceWorkerIdentifier, CompletionHandler<void(Optional<ExceptionData>&&)>&& callback)
    105105{
    106     if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier)) {
    107         worker->claim();
    108         worker->contextConnection()->claimCompleted(requestIdentifier);
    109     }
     106    auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier);
     107    auto* server = worker ? worker->server() : nullptr;
     108    callback(server ? server->claim(*worker) : WTF::nullopt);
    110109}
    111110

  • trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h

    r253898 r256900  
    2828#if ENABLE(SERVICE_WORKER)
    2929
     30#include "ExceptionData.h"
    3031#include "RegistrableDomain.h"
    3132#include "ServiceWorkerClientQueryOptions.h"
    … …  
    5758    virtual void findClientByIdentifierCompleted(uint64_t requestIdentifier, const Optional<ServiceWorkerClientData>&, bool hasSecurityError) = 0;
    5859    virtual void matchAllCompleted(uint64_t requestIdentifier, const Vector<ServiceWorkerClientData>&) = 0;
    59     virtual void claimCompleted(uint64_t requestIdentifier) = 0;
    6060
    6161    // Messages back from the SW host process
    … …  
    6969    WEBCORE_EXPORT void findClientByIdentifier(uint64_t clientIdRequestIdentifier, ServiceWorkerIdentifier, ServiceWorkerClientIdentifier);
    7070    WEBCORE_EXPORT void matchAll(uint64_t requestIdentifier, ServiceWorkerIdentifier, const ServiceWorkerClientQueryOptions&);
    71     WEBCORE_EXPORT void claim(uint64_t requestIdentifier, ServiceWorkerIdentifier);
     71    WEBCORE_EXPORT void claim(ServiceWorkerIdentifier, CompletionHandler<void(Optional<ExceptionData>&&)>&&);
    7272    WEBCORE_EXPORT void setScriptResource(ServiceWorkerIdentifier, URL&& scriptURL, String&& script, URL&& responseURL, String&& mimeType);
    7373    WEBCORE_EXPORT void didFailHeartBeatCheck(ServiceWorkerIdentifier);

  • trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp

    r255058 r256900  
    173173}
    174174
    175 void SWServerWorker::claim()
    176 {
    177     ASSERT(m_server);
    178     if (m_server)
    179         m_server->claim(*this);
    180 }
    181 
    182175void SWServerWorker::setScriptResource(URL&& url, ServiceWorkerContextData::ImportedScript&& script)
    183176{

  • trunk/Source/WebCore/workers/service/server/SWServerWorker.h

    r255058 r256900  
    8383
    8484    ServiceWorkerState state() const { return m_data.state; }
     85    bool isActive() const { return m_data.state == ServiceWorkerState::Activated || m_data.state == ServiceWorkerState::Activating; }
     86
    8587    void setState(ServiceWorkerState);
    8688
    … …  
    9597    WEBCORE_EXPORT Optional<ServiceWorkerClientData> findClientByIdentifier(const ServiceWorkerClientIdentifier&) const;
    9698    void matchAll(const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
    97     void claim();
    9899    void setScriptResource(URL&&, ServiceWorkerContextData::ImportedScript&&);
    99100

  • trunk/Source/WebKit/ChangeLog

    r256892 r256900  
     12020-02-18  Youenn Fablet  <youenn@apple.com>
     2
     3        SWServer::claim should check for the service worker to be active
     4        https://bugs.webkit.org/show_bug.cgi?id=207739
     5        <rdar://problem/45441129>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Use Async Reply to remove the need for a map and passing integers around.
     10
     11        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
     12        (WebKit::WebSWServerToContextConnection::claimCompleted): Deleted.
     13        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
     14        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in:
     15        * WebProcess/Storage/WebSWContextManagerConnection.cpp:
     16        (WebKit::WebSWContextManagerConnection::claim):
     17        (WebKit::WebSWContextManagerConnection::claimCompleted): Deleted.
     18        * WebProcess/Storage/WebSWContextManagerConnection.h:
     19        * WebProcess/Storage/WebSWContextManagerConnection.messages.in:
     20
    1212020-02-18  Said Abou-Hallawa  <sabouhallawa@apple.com>
    222

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp

    r255058 r256900  
    123123}
    124124
    125 void WebSWServerToContextConnection::claimCompleted(uint64_t requestIdentifier)
    126 {
    127     send(Messages::WebSWContextManagerConnection::ClaimCompleted { requestIdentifier });
    128 }
    129 
    130125void WebSWServerToContextConnection::connectionIsNoLongerNeeded()
    131126{

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h

    r255058 r256900  
    9191    void findClientByIdentifierCompleted(uint64_t requestIdentifier, const Optional<WebCore::ServiceWorkerClientData>&, bool hasSecurityError) final;
    9292    void matchAllCompleted(uint64_t requestIdentifier, const Vector<WebCore::ServiceWorkerClientData>&) final;
    93     void claimCompleted(uint64_t requestIdentifier) final;
    9493
    9594    void connectionIsNoLongerNeeded() final;

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in

    r253898 r256900  
    2626    # When possible, these messages can be implemented directly by WebCore::SWServerToContextConnection
    2727
    28     ScriptContextFailedToStart(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, String message);
    29     ScriptContextStarted(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, bool doesHandleFetch);
    30     DidFinishInstall(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, bool wasSuccessful);
    31     DidFinishActivation(WebCore::ServiceWorkerIdentifier identifier);
    32     SetServiceWorkerHasPendingEvents(WebCore::ServiceWorkerIdentifier identifier, bool hasPendingEvents);
     28    ScriptContextFailedToStart(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, String message)
     29    ScriptContextStarted(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, bool doesHandleFetch)
     30    DidFinishInstall(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, bool wasSuccessful)
     31    DidFinishActivation(WebCore::ServiceWorkerIdentifier identifier)
     32    SetServiceWorkerHasPendingEvents(WebCore::ServiceWorkerIdentifier identifier, bool hasPendingEvents)
    3333    SkipWaiting(WebCore::ServiceWorkerIdentifier identifier) -> () Async
    34     WorkerTerminated(WebCore::ServiceWorkerIdentifier identifier);
    35     FindClientByIdentifier(uint64_t requestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, struct WebCore::ServiceWorkerClientIdentifier clientIdentifier);
     34    WorkerTerminated(WebCore::ServiceWorkerIdentifier identifier)
     35    FindClientByIdentifier(uint64_t requestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, struct WebCore::ServiceWorkerClientIdentifier clientIdentifier)
    3636    MatchAll(uint64_t matchAllRequestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, struct WebCore::ServiceWorkerClientQueryOptions options);
    37     Claim(uint64_t claimRequestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier);
    38     SetScriptResource(WebCore::ServiceWorkerIdentifier identifier, URL scriptURL, String script, URL responseURL, String mimeType);
     37    Claim(WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier) -> (Optional<WebCore::ExceptionData> result) Async
     38    SetScriptResource(WebCore::ServiceWorkerIdentifier identifier, URL scriptURL, String script, URL responseURL, String mimeType)
    3939    PostMessageToServiceWorkerClient(struct WebCore::ServiceWorkerClientIdentifier destination, struct WebCore::MessageWithMessagePorts message, WebCore::ServiceWorkerIdentifier source, String sourceOrigin)
    40     DidFailHeartBeatCheck(WebCore::ServiceWorkerIdentifier identifier);
     40    DidFailHeartBeatCheck(WebCore::ServiceWorkerIdentifier identifier)
    4141}
    4242

  • trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp

    r255681 r256900  
    338338}
    339339
    340 void WebSWContextManagerConnection::claim(WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, CompletionHandler<void()>&& callback)
    341 {
    342     auto requestIdentifier = ++m_previousRequestIdentifier;
    343     m_claimRequests.add(requestIdentifier, WTFMove(callback));
    344     m_connectionToNetworkProcess->send(Messages::WebSWServerToContextConnection::Claim { requestIdentifier, serviceWorkerIdentifier }, 0);
    345 }
    346 
    347 void WebSWContextManagerConnection::claimCompleted(uint64_t claimRequestIdentifier)
    348 {
    349     if (auto callback = m_claimRequests.take(claimRequestIdentifier))
    350         callback();
     340void WebSWContextManagerConnection::claim(WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, CompletionHandler<void(ExceptionOr<void>&&)>&& callback)
     341{
     342    m_connectionToNetworkProcess->sendWithAsyncReply(Messages::WebSWServerToContextConnection::Claim { serviceWorkerIdentifier }, [callback = WTFMove(callback)](auto&& result) mutable {
     343        callback(result ? result->toException() : ExceptionOr<void> { });
     344    });
    351345}
    352346

  • trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h

    r255681 r256900  
    7777    void findClientByIdentifier(WebCore::ServiceWorkerIdentifier, WebCore::ServiceWorkerClientIdentifier, FindClientByIdentifierCallback&&) final;
    7878    void matchAll(WebCore::ServiceWorkerIdentifier, const WebCore::ServiceWorkerClientQueryOptions&, WebCore::ServiceWorkerClientsMatchAllCallback&&) final;
    79     void claim(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&) final;
     79    void claim(WebCore::ServiceWorkerIdentifier, CompletionHandler<void(WebCore::ExceptionOr<void>&&)>&&) final;
    8080    void skipWaiting(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&) final;
    8181    void setScriptResource(WebCore::ServiceWorkerIdentifier, const URL&, const WebCore::ServiceWorkerContextData::ImportedScript&) final;
    … …  
    9797    void findClientByIdentifierCompleted(uint64_t requestIdentifier, Optional<WebCore::ServiceWorkerClientData>&&, bool hasSecurityError);
    9898    void matchAllCompleted(uint64_t matchAllRequestIdentifier, Vector<WebCore::ServiceWorkerClientData>&&);
    99     void claimCompleted(uint64_t claimRequestIdentifier);
    10099    void setUserAgent(String&& userAgent);
    101100    void close();
    … …  
    114113    HashMap<uint64_t, FindClientByIdentifierCallback> m_findClientByIdentifierRequests;
    115114    HashMap<uint64_t, WebCore::ServiceWorkerClientsMatchAllCallback> m_matchAllRequests;
    116     HashMap<uint64_t, WTF::CompletionHandler<void()>> m_claimRequests;
    117115    uint64_t m_previousRequestIdentifier { 0 };
    118116    String m_userAgent;

  • trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in

    r253752 r256900  
    3535    FindClientByIdentifierCompleted(uint64_t clientIdRequestIdentifier, Optional<WebCore::ServiceWorkerClientData> data, bool hasSecurityError)
    3636    MatchAllCompleted(uint64_t matchAllRequestIdentifier, Vector<WebCore::ServiceWorkerClientData> clientsData)
    37     ClaimCompleted(uint64_t claimRequestIdentifier)
    3837    SetUserAgent(String userAgent)
    3938    UpdatePreferencesStore(struct WebKit::WebPreferencesStore store)
Note: See TracChangeset for help on using the changeset viewer.