Changeset 229183 in webkit


Ignore:
Timestamp:
Mar 2, 2018 11:17:02 AM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Clients should register to StorageProcess with their service worker registration identifier
​https://bugs.webkit.org/show_bug.cgi?id=182313
<rdar://problem/38044403>

Patch by Youenn Fablet <​youenn@apple.com> on 2018-03-02
Reviewed by Chris Dumez.

Source/WebCore:

Relanding with fixing matchAll for uncontrolled clients.

No observable change of behavior in regular conditions.
When service worker process crashes, the service worker identifiers sent by the WebProcess might be wrong
and we will not be able to retrieve the registration from these identifiers.
The storage process will be able to still process correctly messages coming from the WebProcess to register clients of the registration.
Otherwise, there is a chance that WebProcess clients will not be added to the SWServerRegistration.m_clientsUsingRegistration maps.

  • dom/Document.cpp:

(WebCore::Document::setServiceWorkerConnection):

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

(WebCore::SWServer::matchAll):
(WebCore::SWServer::claim):
(WebCore::SWServer::registerServiceWorkerClient):
(WebCore::SWServer::unregisterServiceWorkerClient):
(WebCore::SWServer::setClientActiveWorker): Deleted.

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

(WebCore::SWServerRegistration::activate):

Source/WebKit:

Relanding.

  • StorageProcess/ServiceWorker/WebSWServerConnection.cpp:

(WebKit::WebSWServerConnection::registerServiceWorkerClient):

  • StorageProcess/ServiceWorker/WebSWServerConnection.h:
  • StorageProcess/ServiceWorker/WebSWServerConnection.messages.in:
  • WebProcess/Storage/WebSWClientConnection.cpp:

(WebKit::WebSWClientConnection::registerServiceWorkerClient):

  • WebProcess/Storage/WebSWClientConnection.h:
Location:
trunk/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r229181 r229183  
     12018-03-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Clients should register to StorageProcess with their service worker registration identifier
     4        https://bugs.webkit.org/show_bug.cgi?id=182313
     5        <rdar://problem/38044403>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Relanding with fixing matchAll for uncontrolled clients.
     10
     11        No observable change of behavior in regular conditions.
     12        When service worker process crashes, the service worker identifiers sent by the WebProcess might be wrong
     13        and we will not be able to retrieve the registration from these identifiers.
     14        The storage process will be able to still process correctly messages coming from the WebProcess to register clients of the registration.
     15        Otherwise, there is a chance that WebProcess clients will not be added to the SWServerRegistration.m_clientsUsingRegistration maps.
     16
     17        * dom/Document.cpp:
     18        (WebCore::Document::setServiceWorkerConnection):
     19        * workers/service/SWClientConnection.h:
     20        * workers/service/server/SWServer.cpp:
     21        (WebCore::SWServer::matchAll):
     22        (WebCore::SWServer::claim):
     23        (WebCore::SWServer::registerServiceWorkerClient):
     24        (WebCore::SWServer::unregisterServiceWorkerClient):
     25        (WebCore::SWServer::setClientActiveWorker): Deleted.
     26        * workers/service/server/SWServer.h:
     27        * workers/service/server/SWServerRegistration.cpp:
     28        (WebCore::SWServerRegistration::activate):
     29
    1302018-03-02  Youenn Fablet  <youenn@apple.com>
    231

  • trunk/Source/WebCore/dom/Document.cpp

    r229163 r229183  
    77447744        return;
    77457745
    7746     auto controllingServiceWorkerIdentifier = activeServiceWorker() ? std::make_optional<ServiceWorkerIdentifier>(activeServiceWorker()->identifier()) : std::nullopt;
    7747     m_serviceWorkerConnection->registerServiceWorkerClient(topOrigin(), ServiceWorkerClientData::from(*this, *serviceWorkerConnection), controllingServiceWorkerIdentifier);
     7746    auto controllingServiceWorkerRegistrationIdentifier = activeServiceWorker() ? std::make_optional<ServiceWorkerRegistrationIdentifier>(activeServiceWorker()->registrationIdentifier()) : std::nullopt;
     7747    m_serviceWorkerConnection->registerServiceWorkerClient(topOrigin(), ServiceWorkerClientData::from(*this, *serviceWorkerConnection), controllingServiceWorkerRegistrationIdentifier);
    77487748}
    77497749#endif

  • trunk/Source/WebCore/workers/service/SWClientConnection.h

    r229163 r229183  
    8181    virtual void syncTerminateWorker(ServiceWorkerIdentifier) = 0;
    8282
    83     virtual void registerServiceWorkerClient(const SecurityOrigin& topOrigin, const ServiceWorkerClientData&, const std::optional<ServiceWorkerIdentifier>&) = 0;
     83    virtual void registerServiceWorkerClient(const SecurityOrigin& topOrigin, const ServiceWorkerClientData&, const std::optional<ServiceWorkerRegistrationIdentifier>&) = 0;
    8484    virtual void unregisterServiceWorkerClient(DocumentIdentifier) = 0;
    8585

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

    r229163 r229183  
    418418    Vector<ServiceWorkerClientData> matchingClients;
    419419    forEachClientForOrigin(worker.origin(), [&](auto& clientData) {
    420         if (!options.includeUncontrolled && worker.identifier() != m_clientToControllingWorker.get(clientData.identifier))
    421             return;
     420        if (!options.includeUncontrolled) {
     421            auto registrationIdentifier = m_clientToControllingRegistration.get(clientData.identifier);
     422            if (worker.data().registrationIdentifier != registrationIdentifier)
     423                return;
     424            if (&worker != activeWorkerFromRegistrationID(registrationIdentifier))
     425                return;
     426        }
    422427        if (options.type != ServiceWorkerClientType::All && options.type != clientData.type)
    423428            return;
    … …  
    448453            return;
    449454
    450         auto result = m_clientToControllingWorker.add(clientData.identifier, worker.identifier());
     455        auto result = m_clientToControllingRegistration.add(clientData.identifier, registration->identifier());
    451456        if (!result.isNewEntry) {
    452457            auto previousIdentifier = result.iterator->value;
    453             if (previousIdentifier == worker.identifier())
     458            if (previousIdentifier == registration->identifier())
    454459                return;
    455             result.iterator->value = worker.identifier();
    456             if (auto* controllingRegistration = this->registrationFromServiceWorkerIdentifier(previousIdentifier))
     460            result.iterator->value = registration->identifier();
     461            if (auto* controllingRegistration = m_registrationsByID.get(previousIdentifier))
    457462                controllingRegistration->removeClientUsingRegistration(clientData.identifier);
    458463        }
    … …  
    727732}
    728733
    729 void SWServer::setClientActiveWorker(ServiceWorkerClientIdentifier clientIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier)
    730 {
    731     m_clientToControllingWorker.set(clientIdentifier, serviceWorkerIdentifier);
    732 }
    733 
    734734SWServerRegistration* SWServer::registrationFromServiceWorkerIdentifier(ServiceWorkerIdentifier identifier)
    735735{
    … …  
    741741}
    742742
    743 void SWServer::registerServiceWorkerClient(ClientOrigin&& clientOrigin, ServiceWorkerClientData&& data, const std::optional<ServiceWorkerIdentifier>& controllingServiceWorkerIdentifier)
     743void SWServer::registerServiceWorkerClient(ClientOrigin&& clientOrigin, ServiceWorkerClientData&& data, const std::optional<ServiceWorkerRegistrationIdentifier>& controllingServiceWorkerRegistrationIdentifier)
    744744{
    745745    auto clientIdentifier = data.identifier;
    … …  
    753753    clientIdentifiersForOrigin.terminateServiceWorkersTimer = nullptr;
    754754
    755     if (!controllingServiceWorkerIdentifier)
    756         return;
    757 
    758     if (auto* controllingRegistration = registrationFromServiceWorkerIdentifier(*controllingServiceWorkerIdentifier)) {
    759         controllingRegistration->addClientUsingRegistration(clientIdentifier);
    760         auto result = m_clientToControllingWorker.add(clientIdentifier, *controllingServiceWorkerIdentifier);
    761         ASSERT_UNUSED(result, result.isNewEntry);
    762     }
     755    if (!controllingServiceWorkerRegistrationIdentifier)
     756        return;
     757
     758    auto* controllingRegistration = m_registrationsByID.get(*controllingServiceWorkerRegistrationIdentifier);
     759    if (!controllingRegistration || !controllingRegistration->activeWorker())
     760        return;
     761
     762    controllingRegistration->addClientUsingRegistration(clientIdentifier);
     763    auto result = m_clientToControllingRegistration.add(clientIdentifier, *controllingServiceWorkerRegistrationIdentifier);
     764    ASSERT_UNUSED(result, result.isNewEntry);
    763765}
    764766
    … …  
    787789    }
    788790
    789     auto workerIterator = m_clientToControllingWorker.find(clientIdentifier);
    790     if (workerIterator == m_clientToControllingWorker.end())
    791         return;
    792 
    793     if (auto* controllingRegistration = registrationFromServiceWorkerIdentifier(workerIterator->value))
    794         controllingRegistration->removeClientUsingRegistration(clientIdentifier);
    795 
    796     m_clientToControllingWorker.remove(workerIterator);
     791    auto registrationIterator = m_clientToControllingRegistration.find(clientIdentifier);
     792    if (registrationIterator == m_clientToControllingRegistration.end())
     793        return;
     794
     795    if (auto* registration = m_registrationsByID.get(registrationIterator->value))
     796        registration->removeClientUsingRegistration(clientIdentifier);
     797
     798    m_clientToControllingRegistration.remove(registrationIterator);
    797799}
    798800

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

    r229163 r229183  
    162162    WEBCORE_EXPORT static HashSet<SWServer*>& allServers();
    163163
    164     WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientData&&, const std::optional<ServiceWorkerIdentifier>& controllingServiceWorkerIdentifier);
     164    WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientData&&, const std::optional<ServiceWorkerRegistrationIdentifier>&);
    165165    WEBCORE_EXPORT void unregisterServiceWorkerClient(const ClientOrigin&, ServiceWorkerClientIdentifier);
    166166
    … …  
    168168    WEBCORE_EXPORT void runServiceWorkerIfNecessary(ServiceWorkerIdentifier, RunServiceWorkerCallback&&);
    169169
    170     void setClientActiveWorker(ServiceWorkerClientIdentifier, ServiceWorkerIdentifier);
    171170    void resolveRegistrationReadyRequests(SWServerRegistration&);
    172171
    … …  
    222221    HashMap<ClientOrigin, Clients> m_clientIdentifiersPerOrigin;
    223222    HashMap<ServiceWorkerClientIdentifier, ServiceWorkerClientData> m_clientsById;
    224     HashMap<ServiceWorkerClientIdentifier, ServiceWorkerIdentifier> m_clientToControllingWorker;
     223    HashMap<ServiceWorkerClientIdentifier, ServiceWorkerRegistrationIdentifier> m_clientToControllingRegistration;
    225224
    226225    UniqueRef<SWOriginStore> m_originStore;

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

    r229163 r229183  
    308308    // For each service worker client who is using registration:
    309309    // - Set client's active worker to registration's active worker.
    310     for (auto keyValue : m_clientsUsingRegistration) {
    311         for (auto& clientIdentifier : keyValue.value)
    312             m_server.setClientActiveWorker(ServiceWorkerClientIdentifier { keyValue.key, clientIdentifier }, activeWorker()->identifier());
    313     }
     310
    314311    // - Invoke Notify Controller Change algorithm with client as the argument.
    315312    notifyClientsOfControllerChange();

  • trunk/Source/WebKit/ChangeLog

    r229182 r229183  
     12018-03-02  Youenn Fablet  <youenn@apple.com>
     2
     3        Clients should register to StorageProcess with their service worker registration identifier
     4        https://bugs.webkit.org/show_bug.cgi?id=182313
     5        <rdar://problem/38044403>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Relanding.
     10
     11        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
     12        (WebKit::WebSWServerConnection::registerServiceWorkerClient):
     13        * StorageProcess/ServiceWorker/WebSWServerConnection.h:
     14        * StorageProcess/ServiceWorker/WebSWServerConnection.messages.in:
     15        * WebProcess/Storage/WebSWClientConnection.cpp:
     16        (WebKit::WebSWClientConnection::registerServiceWorkerClient):
     17        * WebProcess/Storage/WebSWClientConnection.h:
     18
    1192018-03-02  Tim Horton  <timothy_horton@apple.com>
    220

  • trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp

    r229163 r229183  
    279279}
    280280
    281 void WebSWServerConnection::registerServiceWorkerClient(SecurityOriginData&& topOrigin, ServiceWorkerClientData&& data, const std::optional<ServiceWorkerIdentifier>& controllingServiceWorkerIdentifier)
     281void WebSWServerConnection::registerServiceWorkerClient(SecurityOriginData&& topOrigin, ServiceWorkerClientData&& data, const std::optional<ServiceWorkerRegistrationIdentifier>& controllingServiceWorkerRegistrationIdentifier)
    282282{
    283283    auto clientOrigin = ClientOrigin { WTFMove(topOrigin), SecurityOriginData::fromSecurityOrigin(SecurityOrigin::create(data.url)) };
    284284    m_clientOrigins.add(data.identifier, clientOrigin);
    285     server().registerServiceWorkerClient(WTFMove(clientOrigin), WTFMove(data), controllingServiceWorkerIdentifier);
     285    server().registerServiceWorkerClient(WTFMove(clientOrigin), WTFMove(data), controllingServiceWorkerRegistrationIdentifier);
    286286}
    287287

  • trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h

    r229163 r229183  
    9393    void getRegistrations(uint64_t registrationMatchRequestIdentifier, const WebCore::SecurityOriginData& topOrigin, const WebCore::URL& clientURL);
    9494
    95     void registerServiceWorkerClient(WebCore::SecurityOriginData&& topOrigin, WebCore::ServiceWorkerClientData&&, const std::optional<WebCore::ServiceWorkerIdentifier>&);
     95    void registerServiceWorkerClient(WebCore::SecurityOriginData&& topOrigin, WebCore::ServiceWorkerClientData&&, const std::optional<WebCore::ServiceWorkerRegistrationIdentifier>&);
    9696    void unregisterServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier&);
    9797

  • trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.messages.in

    r229163 r229183  
    3939    WhenRegistrationReady(uint64_t serviceRegistrationMatchRequestIdentifier, struct WebCore::SecurityOriginData topOrigin, WebCore::URL clientURL)
    4040    GetRegistrations(uint64_t serviceRegistrationMatchRequestIdentifier, struct WebCore::SecurityOriginData topOrigin, WebCore::URL clientURL)
    41     RegisterServiceWorkerClient(struct WebCore::SecurityOriginData topOrigin, struct WebCore::ServiceWorkerClientData data, std::optional<WebCore::ServiceWorkerIdentifier> controllingServiceWorkerIdentifier)
     41    RegisterServiceWorkerClient(struct WebCore::SecurityOriginData topOrigin, struct WebCore::ServiceWorkerClientData data, std::optional<WebCore::ServiceWorkerRegistrationIdentifier> controllingServiceWorkerRegistrationIdentifier)
    4242    UnregisterServiceWorkerClient(struct WebCore::ServiceWorkerClientIdentifier identifier)
    4343

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

    r229163 r229183  
    9494}
    9595
    96 void WebSWClientConnection::registerServiceWorkerClient(const SecurityOrigin& topOrigin, const WebCore::ServiceWorkerClientData& data, const std::optional<WebCore::ServiceWorkerIdentifier>& controllingServiceWorkerIdentifier)
    97 {
    98     send(Messages::WebSWServerConnection::RegisterServiceWorkerClient { SecurityOriginData::fromSecurityOrigin(topOrigin), data, controllingServiceWorkerIdentifier });
     96void WebSWClientConnection::registerServiceWorkerClient(const SecurityOrigin& topOrigin, const WebCore::ServiceWorkerClientData& data, const std::optional<WebCore::ServiceWorkerRegistrationIdentifier>& controllingServiceWorkerRegistrationIdentifier)
     97{
     98    send(Messages::WebSWServerConnection::RegisterServiceWorkerClient { SecurityOriginData::fromSecurityOrigin(topOrigin), data, controllingServiceWorkerRegistrationIdentifier });
    9999}
    100100

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

    r229163 r229183  
    7676    void finishFetchingScriptInServer(const WebCore::ServiceWorkerFetchResult&) final;
    7777    void postMessageToServiceWorker(WebCore::ServiceWorkerIdentifier destinationIdentifier, WebCore::MessageWithMessagePorts&&, const WebCore::ServiceWorkerOrClientIdentifier& source) final;
    78     void registerServiceWorkerClient(const WebCore::SecurityOrigin& topOrigin, const WebCore::ServiceWorkerClientData&, const std::optional<WebCore::ServiceWorkerIdentifier>&) final;
     78    void registerServiceWorkerClient(const WebCore::SecurityOrigin& topOrigin, const WebCore::ServiceWorkerClientData&, const std::optional<WebCore::ServiceWorkerRegistrationIdentifier>&) final;
    7979    void unregisterServiceWorkerClient(WebCore::DocumentIdentifier) final;
    8080
Note: See TracChangeset for help on using the changeset viewer.