- Revision
- 224924
- Author
- [email protected]
- Date
- 2017-11-16 10:21:44 -0800 (Thu, 16 Nov 2017)
Log Message
Dispatching an event on a ServiceWorker may fail or crash due to GC
https://bugs.webkit.org/show_bug.cgi?id=179745
Reviewed by Geoffrey Garen.
Source/WebCore:
Dispatching an event on a ServiceWorker may fail or crash due to GC. We need to make sure
that a ServiceWorker's wrapper stays alive as long as we may dispatch events on it.
Keep the wrapper alive by making ServiceWorker an ActiveDOMObject and making sure the
implementation object keeps a PendingActivity alive while it may dispatch JS events.
The only event dispatched on ServiceWorker objects is the "statechange" one. We may
dispatch statechange events on a ServiceWorker until its state becomes "redundant".
We therefore take a PendingActivity when the ServiceWorker's state is or becomes
non-redundant (becoming non redundant can only happen when switching initially from
redundant to installing, at which point the ServiceWorker object is not exposed to
the JS yet). We release the PendingActivity when the ServiceWorker's state becomes
redundant or the ActiveDOMObject is stopped (to avoid leaks on navigation).
Test: http/tests/workers/service/service-worker-gc-event.html
* workers/service/ServiceWorker.cpp:
(WebCore::mutableAllWorkers):
(WebCore::ServiceWorker::removeFromAllWorkers):
(WebCore::ServiceWorker::getOrCreate):
(WebCore::ServiceWorker::ServiceWorker):
(WebCore::ServiceWorker::~ServiceWorker):
(WebCore::ServiceWorker::scheduleTaskToUpdateState):
(WebCore::ServiceWorker::activeDOMObjectName const):
(WebCore::ServiceWorker::canSuspendForDocumentSuspension const):
(WebCore::ServiceWorker::stop):
(WebCore::ServiceWorker::updatePendingActivityForEventDispatch):
* workers/service/ServiceWorker.h:
* workers/service/ServiceWorker.idl:
LayoutTests:
Add layout test coverage.
* http/tests/workers/service/resources/sw-test-pre.js:
* http/tests/workers/service/service-worker-gc-event.html: Added.
* http/tests/workers/service/service-worker-gc-event-expected.txt: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (224923 => 224924)
--- trunk/LayoutTests/ChangeLog 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/LayoutTests/ChangeLog 2017-11-16 18:21:44 UTC (rev 224924)
@@ -1,3 +1,16 @@
+2017-11-16 Chris Dumez <[email protected]>
+
+ Dispatching an event on a ServiceWorker may fail or crash due to GC
+ https://bugs.webkit.org/show_bug.cgi?id=179745
+
+ Reviewed by Geoffrey Garen.
+
+ Add layout test coverage.
+
+ * http/tests/workers/service/resources/sw-test-pre.js:
+ * http/tests/workers/service/service-worker-gc-event.html: Added.
+ * http/tests/workers/service/service-worker-gc-event-expected.txt: Added.
+
2017-11-16 Youenn Fablet <[email protected]>
LayoutTest imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html is a flaky failure
Modified: trunk/LayoutTests/http/tests/workers/service/resources/sw-test-pre.js (224923 => 224924)
--- trunk/LayoutTests/http/tests/workers/service/resources/sw-test-pre.js 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/LayoutTests/http/tests/workers/service/resources/sw-test-pre.js 2017-11-16 18:21:44 UTC (rev 224924)
@@ -61,3 +61,20 @@
document.body.appendChild(frame);
});
}
+
+function gc() {
+ if (typeof GCController !== "undefined")
+ GCController.collect();
+ else {
+ var gcRec = function (n) {
+ if (n < 1)
+ return {};
+ var temp = {i: "ab" + i + (i / 100000)};
+ temp += "foo";
+ gcRec(n-1);
+ };
+ for (var i = 0; i < 1000; i++)
+ gcRec(10);
+ }
+}
+
Added: trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event-expected.txt (0 => 224924)
--- trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event-expected.txt 2017-11-16 18:21:44 UTC (rev 224924)
@@ -0,0 +1,4 @@
+statechange to installed
+statechange to activating
+statechange to activated
+
Added: trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event.html (0 => 224924)
--- trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event.html (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-gc-event.html 2017-11-16 18:21:44 UTC (rev 224924)
@@ -0,0 +1,33 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+function getNewestWorker(registration)
+{
+ if (registration.installing)
+ return registration.installing;
+ if (registration.waiting)
+ return registration.waiting;
+ return registration.active;
+}
+
+navigator.serviceWorker.register("resources/empty.js", { }).then(function(_registration) {
+ registration = _registration;
+ registration.installing.addEventListener("statechange", function() {
+ gc();
+ let newState = getNewestWorker(registration).state;
+ gc();
+ log("statechange to " + newState);
+ if (newState == "activated")
+ finishSWTest();
+ });
+ gc();
+ setTimeout(function() {
+ gc();
+ }, 0);
+});
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (224923 => 224924)
--- trunk/Source/WebCore/ChangeLog 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/Source/WebCore/ChangeLog 2017-11-16 18:21:44 UTC (rev 224924)
@@ -1,3 +1,39 @@
+2017-11-16 Chris Dumez <[email protected]>
+
+ Dispatching an event on a ServiceWorker may fail or crash due to GC
+ https://bugs.webkit.org/show_bug.cgi?id=179745
+
+ Reviewed by Geoffrey Garen.
+
+ Dispatching an event on a ServiceWorker may fail or crash due to GC. We need to make sure
+ that a ServiceWorker's wrapper stays alive as long as we may dispatch events on it.
+
+ Keep the wrapper alive by making ServiceWorker an ActiveDOMObject and making sure the
+ implementation object keeps a PendingActivity alive while it may dispatch JS events.
+ The only event dispatched on ServiceWorker objects is the "statechange" one. We may
+ dispatch statechange events on a ServiceWorker until its state becomes "redundant".
+ We therefore take a PendingActivity when the ServiceWorker's state is or becomes
+ non-redundant (becoming non redundant can only happen when switching initially from
+ redundant to installing, at which point the ServiceWorker object is not exposed to
+ the JS yet). We release the PendingActivity when the ServiceWorker's state becomes
+ redundant or the ActiveDOMObject is stopped (to avoid leaks on navigation).
+
+ Test: http/tests/workers/service/service-worker-gc-event.html
+
+ * workers/service/ServiceWorker.cpp:
+ (WebCore::mutableAllWorkers):
+ (WebCore::ServiceWorker::removeFromAllWorkers):
+ (WebCore::ServiceWorker::getOrCreate):
+ (WebCore::ServiceWorker::ServiceWorker):
+ (WebCore::ServiceWorker::~ServiceWorker):
+ (WebCore::ServiceWorker::scheduleTaskToUpdateState):
+ (WebCore::ServiceWorker::activeDOMObjectName const):
+ (WebCore::ServiceWorker::canSuspendForDocumentSuspension const):
+ (WebCore::ServiceWorker::stop):
+ (WebCore::ServiceWorker::updatePendingActivityForEventDispatch):
+ * workers/service/ServiceWorker.h:
+ * workers/service/ServiceWorker.idl:
+
2017-11-16 Frederic Wang <[email protected]>
Consider non-main frames for frameViewRootLayerDidChange
Modified: trunk/Source/WebCore/workers/service/ServiceWorker.cpp (224923 => 224924)
--- trunk/Source/WebCore/workers/service/ServiceWorker.cpp 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/Source/WebCore/workers/service/ServiceWorker.cpp 2017-11-16 18:21:44 UTC (rev 224924)
@@ -59,8 +59,10 @@
auto it = allWorkers().find(data.identifier);
if (it != allWorkers().end()) {
for (auto& worker : it->value) {
- if (worker->scriptExecutionContext() == &context)
+ if (worker->scriptExecutionContext() == &context) {
+ ASSERT(!worker->m_isStopped);
return *worker;
+ }
}
}
return adoptRef(*new ServiceWorker(context, WTFMove(data)));
@@ -67,13 +69,18 @@
}
ServiceWorker::ServiceWorker(ScriptExecutionContext& context, ServiceWorkerData&& data)
- : ContextDestructionObserver(&context)
+ : ActiveDOMObject(&context)
, m_data(WTFMove(data))
{
+ suspendIfNeeded();
+
auto result = mutableAllWorkers().ensure(identifier(), [] {
return HashSet<ServiceWorker*>();
});
result.iterator->value.add(this);
+
+ relaxAdoptionRequirement();
+ updatePendingActivityForEventDispatch();
}
ServiceWorker::~ServiceWorker()
@@ -97,8 +104,15 @@
return;
context->postTask([this, protectedThis = makeRef(*this), state](ScriptExecutionContext&) {
+ ASSERT(this->state() != state);
+
m_data.state = state;
- dispatchEvent(Event::create(eventNames().statechangeEvent, false, false));
+ if (state != State::Installing && !m_isStopped) {
+ ASSERT(m_pendingActivityForEventDispatch);
+ dispatchEvent(Event::create(eventNames().statechangeEvent, false, false));
+ }
+
+ updatePendingActivityForEventDispatch();
});
}
@@ -149,6 +163,35 @@
return ContextDestructionObserver::scriptExecutionContext();
}
+const char* ServiceWorker::activeDOMObjectName() const
+{
+ return "ServiceWorker";
+}
+
+bool ServiceWorker::canSuspendForDocumentSuspension() const
+{
+ // FIXME: We should do better as this prevents the page from entering PageCache when there is a Service Worker.
+ return !hasPendingActivity();
+}
+
+void ServiceWorker::stop()
+{
+ m_isStopped = true;
+ updatePendingActivityForEventDispatch();
+}
+
+void ServiceWorker::updatePendingActivityForEventDispatch()
+{
+ // ServiceWorkers can dispatch events until they become redundant or they are stopped.
+ if (m_isStopped || state() == State::Redundant) {
+ m_pendingActivityForEventDispatch = nullptr;
+ return;
+ }
+ if (m_pendingActivityForEventDispatch)
+ return;
+ m_pendingActivityForEventDispatch = makePendingActivity(*this);
+}
+
} // namespace WebCore
#endif // ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebCore/workers/service/ServiceWorker.h (224923 => 224924)
--- trunk/Source/WebCore/workers/service/ServiceWorker.h 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/Source/WebCore/workers/service/ServiceWorker.h 2017-11-16 18:21:44 UTC (rev 224924)
@@ -27,6 +27,7 @@
#if ENABLE(SERVICE_WORKER)
+#include "ActiveDOMObject.h"
#include "ContextDestructionObserver.h"
#include "EventTarget.h"
#include "ServiceWorkerData.h"
@@ -42,7 +43,7 @@
class Frame;
-class ServiceWorker final : public RefCounted<ServiceWorker>, public EventTargetWithInlineData, public ContextDestructionObserver {
+class ServiceWorker final : public RefCounted<ServiceWorker>, public EventTargetWithInlineData, public ActiveDOMObject {
public:
using State = ServiceWorkerState;
static Ref<ServiceWorker> getOrCreate(ScriptExecutionContext&, ServiceWorkerData&&);
@@ -67,6 +68,7 @@
private:
ServiceWorker(ScriptExecutionContext&, ServiceWorkerData&&);
static HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>& mutableAllWorkers();
+ void updatePendingActivityForEventDispatch();
EventTargetInterface eventTargetInterface() const final;
ScriptExecutionContext* scriptExecutionContext() const final;
@@ -73,7 +75,14 @@
void refEventTarget() final { ref(); }
void derefEventTarget() final { deref(); }
+ // ActiveDOMObject.
+ const char* activeDOMObjectName() const final;
+ bool canSuspendForDocumentSuspension() const final;
+ void stop() final;
+
ServiceWorkerData m_data;
+ bool m_isStopped { false };
+ RefPtr<PendingActivity<ServiceWorker>> m_pendingActivityForEventDispatch;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/service/ServiceWorker.idl (224923 => 224924)
--- trunk/Source/WebCore/workers/service/ServiceWorker.idl 2017-11-16 18:05:29 UTC (rev 224923)
+++ trunk/Source/WebCore/workers/service/ServiceWorker.idl 2017-11-16 18:21:44 UTC (rev 224924)
@@ -27,6 +27,7 @@
// We don't currently support nested workers.
[
+ ActiveDOMObject,
SecureContext,
Exposed=(Window),
Conditional=SERVICE_WORKER,
