"serviceworker.js" is fetched several times in a row
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 01:57:36 +0000 (01:57 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 01:57:36 +0000 (01:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187435
<rdar://problem/41940569>

Reviewed by Youenn Fablet.

Soft updates happen every time a fetch event is sent to a service worker for a main resource request.
This can happen many times during a page load and will cause us to spam the HTTP server with update
requests, especially considering that the default behavior is to bypass the HTTP cache. To address
the issue, we now do soft updates on a 1 second delay and we keep rescheduling this timer was long as
soft update requests keep coming. Based on my understanding of the Chromium code, this seems to be
what they are doing so this should align our behavior with them.

* workers/service/ServiceWorkerRegistration.cpp:
(WebCore::ServiceWorkerRegistration::ServiceWorkerRegistration):
(WebCore::ServiceWorkerRegistration::scheduleSoftUpdate):
* workers/service/ServiceWorkerRegistration.h:
* workers/service/context/ServiceWorkerFetch.cpp:
(WebCore::ServiceWorkerFetch::dispatchFetchEvent):

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

Source/WebCore/ChangeLog
Source/WebCore/workers/service/ServiceWorkerRegistration.cpp
Source/WebCore/workers/service/ServiceWorkerRegistration.h
Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp

index 60fe4bf..a73bcad 100644 (file)
@@ -1,3 +1,25 @@
+2018-07-10  Chris Dumez  <cdumez@apple.com>
+
+        "serviceworker.js" is fetched several times in a row
+        https://bugs.webkit.org/show_bug.cgi?id=187435
+        <rdar://problem/41940569>
+
+        Reviewed by Youenn Fablet.
+
+        Soft updates happen every time a fetch event is sent to a service worker for a main resource request.
+        This can happen many times during a page load and will cause us to spam the HTTP server with update
+        requests, especially considering that the default behavior is to bypass the HTTP cache. To address
+        the issue, we now do soft updates on a 1 second delay and we keep rescheduling this timer was long as
+        soft update requests keep coming. Based on my understanding of the Chromium code, this seems to be
+        what they are doing so this should align our behavior with them.
+
+        * workers/service/ServiceWorkerRegistration.cpp:
+        (WebCore::ServiceWorkerRegistration::ServiceWorkerRegistration):
+        (WebCore::ServiceWorkerRegistration::scheduleSoftUpdate):
+        * workers/service/ServiceWorkerRegistration.h:
+        * workers/service/context/ServiceWorkerFetch.cpp:
+        (WebCore::ServiceWorkerFetch::dispatchFetchEvent):
+
 2018-07-10  Ross Kirsling  <ross.kirsling@sony.com>
 
         [WinCairo] MIME type registry doesn't explicitly recognize *.css
index 515f860..8fea629 100644 (file)
@@ -42,6 +42,8 @@
 
 namespace WebCore {
 
+const Seconds softUpdateDelay { 1_s };
+
 Ref<ServiceWorkerRegistration> ServiceWorkerRegistration::getOrCreate(ScriptExecutionContext& context, Ref<ServiceWorkerContainer>&& container, ServiceWorkerRegistrationData&& data)
 {
     if (auto* registration = container->registration(data.identifier)) {
@@ -56,6 +58,7 @@ ServiceWorkerRegistration::ServiceWorkerRegistration(ScriptExecutionContext& con
     : ActiveDOMObject(&context)
     , m_registrationData(WTFMove(registrationData))
     , m_container(WTFMove(container))
+    , m_softUpdateTimer([this] { softUpdate(); })
 {
     LOG(ServiceWorker, "Creating registration %p for registration key %s", this, m_registrationData.key.loggingString().utf8().data());
     suspendIfNeeded();
@@ -149,6 +152,15 @@ void ServiceWorkerRegistration::update(Ref<DeferredPromise>&& promise)
     m_container->updateRegistration(m_registrationData.scopeURL, newestWorker->scriptURL(), WorkerType::Classic, WTFMove(promise));
 }
 
+// To avoid scheduling many updates during a single page load, we do soft updates on a 1 second delay and keep delaying
+// as long as soft update requests keep coming. This seems to match Chrome's behavior.
+void ServiceWorkerRegistration::scheduleSoftUpdate()
+{
+    if (m_softUpdateTimer.isActive())
+        m_softUpdateTimer.stop();
+    m_softUpdateTimer.startOneShot(softUpdateDelay);
+}
+
 void ServiceWorkerRegistration::softUpdate()
 {
     if (m_isStopped)
index 4372887..d75fbc4 100644 (file)
@@ -32,6 +32,7 @@
 #include "JSDOMPromiseDeferred.h"
 #include "SWClientConnection.h"
 #include "ServiceWorkerRegistrationData.h"
+#include "Timer.h"
 
 namespace WebCore {
 
@@ -66,7 +67,7 @@ public:
     void update(Ref<DeferredPromise>&&);
     void unregister(Ref<DeferredPromise>&&);
 
-    void softUpdate();
+    void scheduleSoftUpdate();
 
     using RefCounted::ref;
     using RefCounted::deref;
@@ -85,6 +86,8 @@ private:
     void refEventTarget() final { ref(); }
     void derefEventTarget() final { deref(); }
 
+    void softUpdate();
+
     // ActiveDOMObject.
     const char* activeDOMObjectName() const final;
     bool canSuspendForDocumentSuspension() const final;
@@ -99,6 +102,7 @@ private:
 
     bool m_isStopped { false };
     RefPtr<PendingActivity<ServiceWorkerRegistration>> m_pendingActivityForEventDispatch;
+    Timer m_softUpdateTimer;
 };
 
 } // namespace WebCore
index 416b05f..7e5a60d 100644 (file)
@@ -140,7 +140,7 @@ void dispatchFetchEvent(Ref<Client>&& client, ServiceWorkerGlobalScope& globalSc
 
     auto& registration = globalScope.registration();
     if (isNonSubresourceRequest || registration.needsUpdate())
-        registration.softUpdate();
+        registration.scheduleSoftUpdate();
 }
 
 } // namespace ServiceWorkerFetch