Diff
Modified: trunk/LayoutTests/ChangeLog (278328 => 278329)
--- trunk/LayoutTests/ChangeLog 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/LayoutTests/ChangeLog 2021-06-01 22:03:35 UTC (rev 278329)
@@ -1,3 +1,19 @@
+2021-06-01 Chris Dumez <cdu...@apple.com>
+
+ Fix unsafe access to m_upload in XMLHttpRequest::virtualHasPendingActivity()
+ https://bugs.webkit.org/show_bug.cgi?id=226508
+
+ Reviewed by Geoffrey Garen.
+
+ Improve layout test coverage to make sure that XMLHttpRequest.upload always returns
+ the same object and that progress events on XMLHttpRequest.upload still get fired
+ after GC.
+
+ * fast/xmlhttprequest/xmlhttprequest-upload-sameobject-expected.txt: Added.
+ * fast/xmlhttprequest/xmlhttprequest-upload-sameobject.html: Added.
+ * http/tests/xmlhttprequest/upload-progress-events-gc-expected.txt: Added.
+ * http/tests/xmlhttprequest/upload-progress-events-gc.html: Added.
+
2021-06-01 Sam Weinig <wei...@apple.com>
Support calc() on components inside relative color syntax colors
Added: trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject-expected.txt (0 => 278329)
--- trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject-expected.txt 2021-06-01 22:03:35 UTC (rev 278329)
@@ -0,0 +1,10 @@
+Tests that XMLHttpRequest.upload always returns the same object.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS xhr.upload.foo is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject.html (0 => 278329)
--- trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject.html (rev 0)
+++ trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-upload-sameobject.html 2021-06-01 22:03:35 UTC (rev 278329)
@@ -0,0 +1,19 @@
+<DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that XMLHttpRequest.upload always returns the same object.");
+jsTestIsAsync = true;
+
+let xhr = new XMLHttpRequest();
+xhr.upload.foo = 1;
+gc();
+setTimeout(() => {
+ gc();
+ shouldBe("xhr.upload.foo", "1");
+ finishJSTest();
+}, 0);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc-expected.txt (0 => 278329)
--- trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc-expected.txt 2021-06-01 22:03:35 UTC (rev 278329)
@@ -0,0 +1,12 @@
+Makes sure that upload events are still fired after GC.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS loadstart event fired on XMLHttpRequestUpload: [object XMLHttpRequestProgressEvent]
+PASS onprogress event fired on XMLHttpRequestUpload: [object XMLHttpRequestProgressEvent] [Loaded: 4 Total: 4]
+PASS load event fired on XMLHttpRequestUpload: [object XMLHttpRequestProgressEvent]
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc.html (0 => 278329)
--- trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc.html (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/upload-progress-events-gc.html 2021-06-01 22:03:35 UTC (rev 278329)
@@ -0,0 +1,35 @@
+<html>
+<body>
+<script src=""
+<script>
+ description("Makes sure that upload events are still fired after GC.");
+ jsTestIsAsync = true;
+
+ function progressHandler(evt)
+ {
+ testPassed("onprogress event fired on XMLHttpRequestUpload: " + evt + " [Loaded: " + evt.loaded + " Total: " + evt.total + "]");
+ }
+
+ function loadstartHandler(evt)
+ {
+ testPassed("loadstart event fired on XMLHttpRequestUpload: " + evt);
+ }
+
+ function loadHandler(evt)
+ {
+ testPassed("load event fired on XMLHttpRequestUpload: " + evt);
+ finishJSTest();
+ }
+
+ let xhr = new XMLHttpRequest;
+ xhr.upload._onprogress_ = progressHandler;
+ xhr.upload._onloadstart_ = loadstartHandler;
+ xhr.upload._onload_ = loadHandler;
+ xhr.open("POST", "resources/post-echo.cgi", true);
+ xhr.send("data");
+ xhr = null;
+ gc();
+ setInterval(() => { gc(); }, 1);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (278328 => 278329)
--- trunk/Source/WebCore/ChangeLog 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/ChangeLog 2021-06-01 22:03:35 UTC (rev 278329)
@@ -1,3 +1,27 @@
+2021-06-01 Chris Dumez <cdu...@apple.com>
+
+ Fix unsafe access to m_upload in XMLHttpRequest::virtualHasPendingActivity()
+ https://bugs.webkit.org/show_bug.cgi?id=226508
+
+ Reviewed by Geoffrey Garen.
+
+ Fix unsafe access to m_upload in XMLHttpRequest::virtualHasPendingActivity() as virtualHasPendingActivity()
+ may get called off the main thread and m_upload gets initialized lazily on the main thread.
+
+ Tests: fast/xmlhttprequest/xmlhttprequest-upload-sameobject.html
+ http/tests/xmlhttprequest/upload-progress-events-gc.html
+
+ * xml/XMLHttpRequest.cpp:
+ (WebCore::XMLHttpRequest::updateHasRelevantEventListener):
+ (WebCore::XMLHttpRequest::eventListenersDidChange):
+ (WebCore::XMLHttpRequest::virtualHasPendingActivity const):
+ * xml/XMLHttpRequest.h:
+ * xml/XMLHttpRequest.idl:
+ * xml/XMLHttpRequestUpload.cpp:
+ (WebCore::XMLHttpRequestUpload::eventListenersDidChange):
+ (WebCore::XMLHttpRequestUpload::hasRelevantEventListener const):
+ * xml/XMLHttpRequestUpload.h:
+
2021-06-01 Fujii Hironori <hironori.fu...@sony.com>
[Win] Remove unused GraphicsContext::hdc()
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.cpp (278328 => 278329)
--- trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2021-06-01 22:03:35 UTC (rev 278329)
@@ -1161,7 +1161,7 @@
ActiveDOMObject::contextDestroyed();
}
-void XMLHttpRequest::eventListenersDidChange()
+void XMLHttpRequest::updateHasRelevantEventListener()
{
m_hasRelevantEventListener = hasEventListeners(eventNames().abortEvent)
|| hasEventListeners(eventNames().errorEvent)
@@ -1169,14 +1169,20 @@
|| hasEventListeners(eventNames().loadendEvent)
|| hasEventListeners(eventNames().progressEvent)
|| hasEventListeners(eventNames().readystatechangeEvent)
- || hasEventListeners(eventNames().timeoutEvent);
+ || hasEventListeners(eventNames().timeoutEvent)
+ || (m_upload && m_upload->hasRelevantEventListener());
}
+void XMLHttpRequest::eventListenersDidChange()
+{
+ updateHasRelevantEventListener();
+}
+
// An XMLHttpRequest object must not be garbage collected if its state is either opened with the send() flag set, headers received, or loading, and
// it has one or more event listeners registered whose type is one of readystatechange, progress, abort, error, load, timeout, and loadend.
bool XMLHttpRequest::virtualHasPendingActivity() const
{
- if (!m_hasRelevantEventListener && !(m_upload && m_upload->hasRelevantEventListener()))
+ if (!m_hasRelevantEventListener)
return false;
switch (readyState()) {
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.h (278328 => 278329)
--- trunk/Source/WebCore/xml/XMLHttpRequest.h 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.h 2021-06-01 22:03:35 UTC (rev 278329)
@@ -133,8 +133,11 @@
void dispatchEvent(Event&) override;
private:
+ friend class XMLHttpRequestUpload;
explicit XMLHttpRequest(ScriptExecutionContext&);
+ void updateHasRelevantEventListener();
+
// EventTarget.
void eventListenersDidChange() final;
@@ -247,7 +250,7 @@
std::optional<ExceptionCode> m_exceptionCode;
RefPtr<UserGestureToken> m_userGestureToken;
- bool m_hasRelevantEventListener { false };
+ std::atomic<bool> m_hasRelevantEventListener;
};
inline auto XMLHttpRequest::responseType() const -> ResponseType
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.idl (278328 => 278329)
--- trunk/Source/WebCore/xml/XMLHttpRequest.idl 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.idl 2021-06-01 22:03:35 UTC (rev 278329)
@@ -66,7 +66,7 @@
undefined setRequestHeader(ByteString header, ByteString value);
attribute unsigned long timeout;
attribute boolean withCredentials;
- readonly attribute XMLHttpRequestUpload upload;
+ [SameObject] readonly attribute XMLHttpRequestUpload upload;
undefined send(optional (Document or BodyInit)? body = null);
undefined abort();
Modified: trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp (278328 => 278329)
--- trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp 2021-06-01 22:03:35 UTC (rev 278329)
@@ -43,7 +43,12 @@
void XMLHttpRequestUpload::eventListenersDidChange()
{
- m_hasRelevantEventListener = hasEventListeners(eventNames().abortEvent)
+ m_request.updateHasRelevantEventListener();
+}
+
+bool XMLHttpRequestUpload::hasRelevantEventListener() const
+{
+ return hasEventListeners(eventNames().abortEvent)
|| hasEventListeners(eventNames().errorEvent)
|| hasEventListeners(eventNames().loadEvent)
|| hasEventListeners(eventNames().loadendEvent)
Modified: trunk/Source/WebCore/xml/XMLHttpRequestUpload.h (278328 => 278329)
--- trunk/Source/WebCore/xml/XMLHttpRequestUpload.h 2021-06-01 21:04:36 UTC (rev 278328)
+++ trunk/Source/WebCore/xml/XMLHttpRequestUpload.h 2021-06-01 22:03:35 UTC (rev 278329)
@@ -40,7 +40,7 @@
void dispatchProgressEvent(const AtomString& type, unsigned long long loaded, unsigned long long total);
- bool hasRelevantEventListener() const { return m_hasRelevantEventListener; }
+ bool hasRelevantEventListener() const;
private:
// EventTarget.
@@ -52,7 +52,6 @@
ScriptExecutionContext* scriptExecutionContext() const final { return m_request.scriptExecutionContext(); }
XMLHttpRequest& m_request;
- bool m_hasRelevantEventListener { false };
};
} // namespace WebCore
