Title: [259009] trunk
Revision
259009
Author
cdu...@apple.com
Date
2020-03-25 14:16:13 -0700 (Wed, 25 Mar 2020)

Log Message

Event listeners registered with 'once' option may get garbage collected too soon
https://bugs.webkit.org/show_bug.cgi?id=209504
<rdar://problem/60541567>

Reviewed by Yusuke Suzuki.

Source/_javascript_Core:

Add EnsureStillAliveScope RAII object for ensureStillAliveHere().

* runtime/JSCJSValue.h:
(JSC::EnsureStillAliveScope::EnsureStillAliveScope):
(JSC::EnsureStillAliveScope::~EnsureStillAliveScope):

Source/WebCore:

In EventTarget::innerInvokeEventListeners, if the listener we're about to call is a one-time
listener (has 'once' flag set), we would first unregister the event listener and then call
it, as per the DOM specification. However, once unregistered, the event listener is no longer
visited for GC purposes and its internal JS Function may get garbage collected before we get
a chance to call it.

To address the issue, we now make sure the JS Function (and its wrapper) stay alive for the
duration of the scope using ensureStillAliveHere().

Test: http/tests/inspector/network/har/har-page-aggressive-gc.html

* bindings/js/JSEventListener.h:
* dom/EventListener.h:
(WebCore::EventListener::jsFunction const):
(WebCore::EventListener::wrapper const):
* dom/EventTarget.cpp:
(WebCore::EventTarget::innerInvokeEventListeners):

LayoutTests:

Add layout test coverage.

* http/tests/inspector/network/har/har-page-aggressive-gc-expected.txt: Added.
* http/tests/inspector/network/har/har-page-aggressive-gc.html: Added.
* platform/gtk/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/mac-wk2/TestExpectations:
* platform/win/TestExpectations:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (259008 => 259009)


--- trunk/LayoutTests/ChangeLog	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/LayoutTests/ChangeLog	2020-03-25 21:16:13 UTC (rev 259009)
@@ -1,3 +1,20 @@
+2020-03-25  Chris Dumez  <cdu...@apple.com>
+
+        Event listeners registered with 'once' option may get garbage collected too soon
+        https://bugs.webkit.org/show_bug.cgi?id=209504
+        <rdar://problem/60541567>
+
+        Reviewed by Yusuke Suzuki.
+
+        Add layout test coverage.
+
+        * http/tests/inspector/network/har/har-page-aggressive-gc-expected.txt: Added.
+        * http/tests/inspector/network/har/har-page-aggressive-gc.html: Added.
+        * platform/gtk/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+        * platform/win/TestExpectations:
+
 2020-03-25  Jason Lawrence  <lawrenc...@apple.com>
 
         [ Mac wk1] ASSERTION FAILED: m_wrapper under WebCore::XMLHttpRequestUpload::dispatchProgressEvent

Added: trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc-expected.txt (0 => 259009)


--- trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc-expected.txt	2020-03-25 21:16:13 UTC (rev 259009)
@@ -0,0 +1,769 @@
+HAR Page Test.
+
+
+== Running test suite: HAR.Page
+-- Running test case: HAR.Basic.Page
+{
+  "log": {
+    "version": "1.2",
+    "creator": {
+      "name": "WebKit Web Inspector",
+      "version": "<filtered>"
+    },
+    "pages": [
+      {
+        "startedDateTime": "<filtered>",
+        "id": "page_0",
+        "title": "http://127.0.0.1:8000/inspector/network/har/har-page.html",
+        "pageTimings": {
+          "onContentLoad": "<filtered>",
+          "onLoad": "<filtered>"
+        }
+      }
+    ],
+    "entries": [
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/inspector/network/har/har-page.html",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/html",
+            "text": "<filtered>"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "high"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/inspector/resources/inspector-test.js",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "application/x-_javascript_",
+            "text": "<filtered>"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "high"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/cookie-utilities.js",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "application/x-_javascript_",
+            "text": "<filtered>"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "high"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "path",
+              "value": "path",
+              "path": "/A/B/C",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "secure",
+              "value": "secure",
+              "expires": "",
+              "httpOnly": false,
+              "secure": true
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "secure-and-http-only",
+              "value": "secure-and-http-only",
+              "expires": "",
+              "httpOnly": true,
+              "secure": true
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "with-expiration",
+              "value": "with-expiration",
+              "expires": "5000-01-04T08:00:00.000Z",
+              "httpOnly": false,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "simple",
+              "value": "simple"
+            },
+            {
+              "name": "with-expiration",
+              "value": "with-expiration"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "http-only",
+              "value": "http-only",
+              "expires": "",
+              "httpOnly": true,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "http-only",
+              "value": "http-only"
+            },
+            {
+              "name": "simple",
+              "value": "simple"
+            },
+            {
+              "name": "with-expiration",
+              "value": "with-expiration"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "same-site-strict",
+              "value": "same-site-strict",
+              "path": "/",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false,
+              "sameSite": "Strict"
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "http-only",
+              "value": "http-only"
+            },
+            {
+              "name": "same-site-strict",
+              "value": "same-site-strict"
+            },
+            {
+              "name": "simple",
+              "value": "simple"
+            },
+            {
+              "name": "with-expiration",
+              "value": "with-expiration"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "same-site-ignored-because-no-value",
+              "value": "same-site-ignored-because-no-value",
+              "path": "/",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "http-only",
+              "value": "http-only"
+            },
+            {
+              "name": "same-site-ignored-because-no-value",
+              "value": "same-site-ignored-because-no-value"
+            },
+            {
+              "name": "same-site-strict",
+              "value": "same-site-strict"
+            },
+            {
+              "name": "simple",
+              "value": "simple"
+            },
+            {
+              "name": "with-expiration",
+              "value": "with-expiration"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "same-site-ignored-because-unknown-value",
+              "value": "same-site-ignored-because-unknown-value",
+              "path": "/",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      },
+      {
+        "pageref": "page_0",
+        "startedDateTime": "<filtered>",
+        "time": "<filtered>",
+        "request": {
+          "method": "GET",
+          "url": "http://127.0.0.1:8000/cookies/resources/setCookies.cgi",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "http-only",
+              "value": "http-only"
+            },
+            {
+              "name": "same-site-ignored-because-no-value",
+              "value": "same-site-ignored-because-no-value"
+            },
+            {
+              "name": "same-site-ignored-because-unknown-value",
+              "value": "same-site-ignored-because-unknown-value"
+            },
+            {
+              "name": "same-site-strict",
+              "value": "same-site-strict"
+            },
+            {
+              "name": "simple",
+              "value": "simple"
+            },
+            {
+              "name": "with-expiration",
+              "value": "with-expiration"
+            }
+          ],
+          "headers": "<filtered>",
+          "queryString": [],
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "response": {
+          "status": 200,
+          "statusText": "OK",
+          "httpVersion": "<filtered>",
+          "cookies": [
+            {
+              "name": "same-site-lax",
+              "value": "same-site-lax",
+              "path": "/",
+              "expires": "",
+              "httpOnly": false,
+              "secure": false,
+              "sameSite": "Lax"
+            }
+          ],
+          "headers": "<filtered>",
+          "content": {
+            "size": "<filtered>",
+            "compression": 0,
+            "mimeType": "text/plain"
+          },
+          "redirectURL": "",
+          "headersSize": "<filtered>",
+          "bodySize": "<filtered>"
+        },
+        "cache": {},
+        "timings": {
+          "blocked": "<filtered>",
+          "dns": "<filtered>",
+          "connect": "<filtered>",
+          "ssl": -1,
+          "send": "<filtered>",
+          "wait": "<filtered>",
+          "receive": "<filtered>"
+        },
+        "_serverPort": 8000,
+        "_priority": "medium"
+      }
+    ]
+  }
+}
+

Added: trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc.html (0 => 259009)


--- trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/network/har/har-page-aggressive-gc.html	2020-03-25 21:16:13 UTC (rev 259009)
@@ -0,0 +1,136 @@
+<!DOCTYPE html><!-- webkit-test-runner [ jscOptions=--slowPathAllocsBetweenGCs=5 ] -->
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+<script src=""
+<script>
+function test()
+{
+    function HARJSONFilter(key, value) {
+        // Filter out the creator.version / browser.version but leave a top level version.
+        if ((key === "creator" || key === "browser") && value.version) {
+            value.version = "<filtered>";
+            return value;
+        }
+
+        // Cache may or may not have been used so remove optional fields which may or may not be set.
+        if (key === "connection")
+            return undefined;
+        if (key === "serverIPAddress")
+            return undefined;
+        if (key === "_transferSize")
+            return undefined;
+        if (key === "_fetchType")
+            return undefined;
+
+        // If cached the protocol might not be available.
+        if (key === "httpVersion")
+            return "<filtered>";
+
+        // Headers include dynamic data.
+        if (key === "headers")
+            return "<filtered>";
+
+        // Dates would change between test runs.
+        if (key.endsWith("DateTime"))
+            return "<filtered>";
+
+        // If any files are modified, we'd need to update the test without this filter.
+        if (key == "size")
+            return "<filtered>";
+
+        // Size data may or may not be available, but could change based on headers.
+        if (key.endsWith("Size"))
+            return "<filtered>";
+
+        // Reduce the file contents.
+        if (key === "text") {
+            InspectorTest.assert(value.length > 0, "The file should have contents.");
+            return "<filtered>";
+        }
+
+        // Sort cookies by name to make cookie order deterministic between test runs. We assume
+        // that cookies have names that consist of only ASCII characters.
+        if (key === "cookies") {
+            value.sort((a, b) => { return a.name === b.name ? 0 : (a.name < b.name ? -1 : 1); });
+            return value;
+        }
+
+        // Since cache may or may not be used, timing data may be variable.
+        // NOTE: SSL should always be -1 for this test case.
+        if (key === "time")
+            return "<filtered>";
+        if (key === "timings") {
+            value.blocked = "<filtered>";
+            value.dns = "<filtered>";
+            value.connect = "<filtered>";
+            value.send = "<filtered>";
+            value.wait = "<filtered>";
+            value.receive = "<filtered>";
+        }
+
+        // PageTimings can be variable.
+        if (key === "onContentLoad" || key === "onLoad")
+            return "<filtered>";
+
+        return value;
+    }
+
+    let suite = InspectorTest.createAsyncSuite("HAR.Page");
+
+    suite.addTestCase({
+        name: "HAR.Basic.Page",
+        description: "Should be able to generate a HAR with all of this test page's resources.",
+        async test() {
+            InspectorTest.reloadPage({ignoreCache: true});
+            await InspectorTest.awaitEvent("LoadComplete");
+            InspectorTest.evaluateInPage("setup()");
+            await InspectorTest.awaitEvent("SetupComplete");
+
+            let resources = [WI.networkManager.mainFrame.mainResource, ...WI.networkManager.mainFrame.resourceCollection];
+            let har = await WI.HARBuilder.buildArchive(resources);
+            InspectorTest.json(har, HARJSONFilter);
+            InspectorTest.evaluateInPage("cleanup()");
+            await InspectorTest.awaitEvent("CleanComplete");
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>HAR Page Test.</p>
+<script>
+async function setup()
+{
+    await resetCookies();
+    await setCookie("simple", "simple");
+    await setCookie("path", "path", {"path": "/A/B/C"}); // This cookie will not be sent in subsequent requests from this page.
+
+    // Secure cookies will only be sent in subsequent requests from this page if the page was served over HTTPS.
+    await setCookie("secure", "secure", {"secure": null});
+    await setCookie("secure-and-http-only", "secure-and-http-only", {"secure": null, "HttpOnly": null});
+
+    await setCookie("with-expiration", "with-expiration", {"Expires": new Date("01/04/5000").toGMTString()});
+    await setCookie("http-only", "http-only", {"HttpOnly": null});
+    await setCookie("same-site-strict", "same-site-strict", {"SameSite": "Strict", "path": "/"});
+    await setCookie("same-site-ignored-because-no-value", "same-site-ignored-because-no-value", {"SameSite": null, "path": "/"});
+    await setCookie("same-site-ignored-because-unknown-value", "same-site-ignored-because-unknown-value", {"SameSite": "invalid", "path": "/"});
+    await setCookie("same-site-lax", "same-site-lax", {"SameSite": "Lax", "path": "/"});
+    TestPage.dispatchEventToFrontend("SetupComplete");
+}
+
+async function cleanup()
+{
+    await resetCookies();
+    TestPage.dispatchEventToFrontend("CleanComplete");
+}
+
+window.addEventListener("load", () => {
+    TestPage.dispatchEventToFrontend("LoadComplete");
+});
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (259008 => 259009)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2020-03-25 21:16:13 UTC (rev 259009)
@@ -2068,6 +2068,7 @@
 webkit.org/b/191497 http/tests/inspector/network/resource-security-certificate.html [ Skip ]
 webkit.org/b/191497 http/tests/inspector/network/getSerializedCertificate.html [ Skip ]
 webkit.org/b/179173 [ Release ] http/tests/inspector/network/har/har-page.html [ Failure Pass ]
+webkit.org/b/179173 [ Release ] http/tests/inspector/network/har/har-page-aggressive-gc.html [ Failure Pass ]
 
 webkit.org/b/186851 imported/w3c/web-platform-tests/xhr/formdata.htm [ Pass Failure ]
 webkit.org/b/186851 imported/w3c/web-platform-tests/xhr/formdata-blob.htm [ Pass Failure ]

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (259008 => 259009)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2020-03-25 21:16:13 UTC (rev 259009)
@@ -562,6 +562,7 @@
 http/tests/inspector/network/resource-sizes-network.html [ Failure ]
 http/tests/inspector/network/resource-sizes-memory-cache.html [ Failure ]
 http/tests/inspector/network/har/har-page.html [ Failure ]
+http/tests/inspector/network/har/har-page-aggressive-gc.html [ Failure ]
 
 # Local Overrides not available in WebKit1
 http/tests/inspector/network/local-resource-override-basic.html [ Failure ]
@@ -966,4 +967,4 @@
 
 webkit.org/b/209499 [ Catalina ] storage/indexeddb/modern/abort-requests-cancelled-private.html [ Pass Timeout Crash ]
 
-webkit.org/b/209560 imported/w3c/web-platform-tests/xhr/send-response-upload-event-progress.htm [ Pass Crash Failure ]
\ No newline at end of file
+webkit.org/b/209560 imported/w3c/web-platform-tests/xhr/send-response-upload-event-progress.htm [ Pass Crash Failure ]

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (259008 => 259009)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2020-03-25 21:16:13 UTC (rev 259009)
@@ -1013,6 +1013,7 @@
 webkit.org/b/207938 http/wpt/crypto/derive-hmac-key-crash.any.html [ Pass Crash ]
 
 webkit.org/b/207954 http/tests/inspector/network/har/har-page.html [ Pass Failure ]
+webkit.org/b/207954 http/tests/inspector/network/har/har-page-aggressive-gc.html [ Pass Failure ]
 
 webkit.org/b/207962 accessibility/mac/aria-menu-item-selected-notification.html [ Slow ]
 
@@ -1068,4 +1069,4 @@
 
 webkit.org/b/209490 http/tests/media/track-in-band-hls-metadata.html [ Pass Crash Timeout ]
 
-webkit.org/b/209503 [ Debug ] http/tests/referrer-policy-anchor/origin/cross-origin-http.https.html [ Pass Crash ]
\ No newline at end of file
+webkit.org/b/209503 [ Debug ] http/tests/referrer-policy-anchor/origin/cross-origin-http.https.html [ Pass Crash ]

Modified: trunk/LayoutTests/platform/win/TestExpectations (259008 => 259009)


--- trunk/LayoutTests/platform/win/TestExpectations	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/LayoutTests/platform/win/TestExpectations	2020-03-25 21:16:13 UTC (rev 259009)
@@ -3656,6 +3656,7 @@
 http/tests/cache/cache-control-immutable-https.html [ Skip ]
 http/tests/inspector/network/resource-request-headers.html [ Skip ]
 http/tests/inspector/network/har/har-page.html [ Skip ]
+http/tests/inspector/network/har/har-page-aggressive-gc.html [ Skip ]
 http/tests/local/blob/send-hybrid-blob-using-open-panel.html [ Skip ]
 http/tests/security/contentSecurityPolicy/cross-origin-plugin-document-allowed-in-child-window.html [ Skip ]
 http/tests/security/contentSecurityPolicy/same-origin-plugin-document-blocked-in-child-window.html [ Skip ]

Modified: trunk/Source/_javascript_Core/ChangeLog (259008 => 259009)


--- trunk/Source/_javascript_Core/ChangeLog	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-03-25 21:16:13 UTC (rev 259009)
@@ -1,3 +1,17 @@
+2020-03-25  Chris Dumez  <cdu...@apple.com>
+
+        Event listeners registered with 'once' option may get garbage collected too soon
+        https://bugs.webkit.org/show_bug.cgi?id=209504
+        <rdar://problem/60541567>
+
+        Reviewed by Yusuke Suzuki.
+
+        Add EnsureStillAliveScope RAII object for ensureStillAliveHere().
+
+        * runtime/JSCJSValue.h:
+        (JSC::EnsureStillAliveScope::EnsureStillAliveScope):
+        (JSC::EnsureStillAliveScope::~EnsureStillAliveScope):
+
 2020-03-25  Alexey Shvayka  <shvaikal...@gmail.com>
 
         \b escapes inside character classes should be valid in Unicode patterns

Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.h (259008 => 259009)


--- trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2020-03-25 21:16:13 UTC (rev 259009)
@@ -34,6 +34,7 @@
 #include <wtf/HashTraits.h>
 #include <wtf/MathExtras.h>
 #include <wtf/MediaTime.h>
+#include <wtf/Nonmovable.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/TriState.h>
 
@@ -650,4 +651,26 @@
 JS_EXPORT_PRIVATE void ensureStillAliveHere(JSValue);
 #endif
 
+// Use EnsureStillAliveScope when you have a data structure that includes GC pointers, and you need
+// to remove it from the DOM and then use it in the same scope. For example, a 'once' event listener
+// needs to be removed from the DOM and then fired.
+class EnsureStillAliveScope {
+    WTF_FORBID_HEAP_ALLOCATION;
+    WTF_MAKE_NONCOPYABLE(EnsureStillAliveScope);
+    WTF_MAKE_NONMOVABLE(EnsureStillAliveScope);
+public:
+    EnsureStillAliveScope(JSValue value)
+        : m_value(value)
+    {
+    }
+
+    ~EnsureStillAliveScope()
+    {
+        ensureStillAliveHere(m_value);
+    }
+
+private:
+    JSValue m_value;
+};
+
 } // namespace JSC

Modified: trunk/Source/WebCore/ChangeLog (259008 => 259009)


--- trunk/Source/WebCore/ChangeLog	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/ChangeLog	2020-03-25 21:16:13 UTC (rev 259009)
@@ -1,3 +1,29 @@
+2020-03-25  Chris Dumez  <cdu...@apple.com>
+
+        Event listeners registered with 'once' option may get garbage collected too soon
+        https://bugs.webkit.org/show_bug.cgi?id=209504
+        <rdar://problem/60541567>
+
+        Reviewed by Yusuke Suzuki.
+
+        In EventTarget::innerInvokeEventListeners, if the listener we're about to call is a one-time
+        listener (has 'once' flag set), we would first unregister the event listener and then call
+        it, as per the DOM specification. However, once unregistered, the event listener is no longer
+        visited for GC purposes and its internal JS Function may get garbage collected before we get
+        a chance to call it.
+
+        To address the issue, we now make sure the JS Function (and its wrapper) stay alive for the
+        duration of the scope using ensureStillAliveHere().
+
+        Test: http/tests/inspector/network/har/har-page-aggressive-gc.html
+
+        * bindings/js/JSEventListener.h:
+        * dom/EventListener.h:
+        (WebCore::EventListener::jsFunction const):
+        (WebCore::EventListener::wrapper const):
+        * dom/EventTarget.cpp:
+        (WebCore::EventTarget::innerInvokeEventListeners):
+
 2020-03-25  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Rename "data interaction pasteboard" to "drag and drop pasteboard"

Modified: trunk/Source/WebCore/bindings/js/JSErrorHandler.cpp (259008 => 259009)


--- trunk/Source/WebCore/bindings/js/JSErrorHandler.cpp	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/bindings/js/JSErrorHandler.cpp	2020-03-25 21:16:13 UTC (rev 259009)
@@ -67,7 +67,7 @@
     VM& vm = scriptExecutionContext.vm();
     JSLockHolder lock(vm);
 
-    JSObject* jsFunction = this->jsFunction(scriptExecutionContext);
+    JSObject* jsFunction = this->ensureJSFunction(scriptExecutionContext);
     if (!jsFunction)
         return;
 

Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (259008 => 259009)


--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp	2020-03-25 21:16:13 UTC (rev 259009)
@@ -110,7 +110,7 @@
     // "If this throws an exception, report the exception." It should not propagate the
     // exception.
 
-    JSObject* jsFunction = this->jsFunction(scriptExecutionContext);
+    JSObject* jsFunction = ensureJSFunction(scriptExecutionContext);
     if (!jsFunction)
         return;
 
@@ -242,7 +242,7 @@
     if (!is<JSEventListener>(abstractListener))
         return jsNull();
 
-    auto* function = downcast<JSEventListener>(*abstractListener).jsFunction(context);
+    auto* function = downcast<JSEventListener>(*abstractListener).ensureJSFunction(context);
     if (!function)
         return jsNull();
 

Modified: trunk/Source/WebCore/bindings/js/JSEventListener.h (259008 => 259009)


--- trunk/Source/WebCore/bindings/js/JSEventListener.h	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.h	2020-03-25 21:16:13 UTC (rev 259009)
@@ -48,11 +48,13 @@
     // Returns true if this event listener was created for an event handler attribute, like "onload" or "onclick".
     bool isAttribute() const final { return m_isAttribute; }
 
-    JSC::JSObject* jsFunction(ScriptExecutionContext&) const;
+    JSC::JSObject* ensureJSFunction(ScriptExecutionContext&) const;
     DOMWrapperWorld& isolatedWorld() const { return m_isolatedWorld; }
 
-    JSC::JSObject* wrapper() const { return m_wrapper.get(); }
 
+    JSC::JSObject* jsFunction() const final { return m_jsFunction.get(); }
+    JSC::JSObject* wrapper() const final { return m_wrapper.get(); }
+
     virtual String sourceURL() const { return String(); }
     virtual TextPosition sourcePosition() const { return TextPosition(); }
 
@@ -92,7 +94,7 @@
 JSC::JSValue documentEventHandlerAttribute(Document&, const AtomString& eventType, DOMWrapperWorld&);
 void setDocumentEventHandlerAttribute(JSC::JSGlobalObject&, JSC::JSObject&, Document&, const AtomString& eventType, JSC::JSValue);
 
-inline JSC::JSObject* JSEventListener::jsFunction(ScriptExecutionContext& scriptExecutionContext) const
+inline JSC::JSObject* JSEventListener::ensureJSFunction(ScriptExecutionContext& scriptExecutionContext) const
 {
     // initializeJSFunction can trigger code that deletes this event listener
     // before we're done. It should always return null in this case.

Modified: trunk/Source/WebCore/dom/EventListener.h (259008 => 259009)


--- trunk/Source/WebCore/dom/EventListener.h	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/dom/EventListener.h	2020-03-25 21:16:13 UTC (rev 259009)
@@ -60,6 +60,9 @@
     virtual void checkValidityForEventTarget(EventTarget&) { }
 #endif
 
+    virtual JSC::JSObject* jsFunction() const { return nullptr; }
+    virtual JSC::JSObject* wrapper() const { return nullptr; }
+
 protected:
     explicit EventListener(Type type)
         : m_type(type)

Modified: trunk/Source/WebCore/dom/EventTarget.cpp (259008 => 259009)


--- trunk/Source/WebCore/dom/EventTarget.cpp	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/dom/EventTarget.cpp	2020-03-25 21:16:13 UTC (rev 259009)
@@ -304,6 +304,12 @@
         if (event.immediatePropagationStopped())
             break;
 
+        // Make sure the JS wrapper and function stay alive until the end of this scope. Otherwise,
+        // event listeners with 'once' flag may get collected as soon as they get unregistered below,
+        // before we call the js function.
+        JSC::EnsureStillAliveScope wrapperProtector(registeredListener->callback().wrapper());
+        JSC::EnsureStillAliveScope jsFunctionProtector(registeredListener->callback().jsFunction());
+
         // Do this before invocation to avoid reentrancy issues.
         if (registeredListener->isOnce())
             removeEventListener(event.type(), registeredListener->callback(), ListenerOptions(registeredListener->useCapture()));

Modified: trunk/Source/WebCore/inspector/CommandLineAPIHost.cpp (259008 => 259009)


--- trunk/Source/WebCore/inspector/CommandLineAPIHost.cpp	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/inspector/CommandLineAPIHost.cpp	2020-03-25 21:16:13 UTC (rev 259009)
@@ -113,7 +113,7 @@
             if (&jsListener.isolatedWorld() != &currentWorld(lexicalGlobalObject))
                 continue;
 
-            auto* function = jsListener.jsFunction(*scriptExecutionContext);
+            auto* function = jsListener.ensureJSFunction(*scriptExecutionContext);
             if (!function)
                 continue;
 

Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp (259008 => 259009)


--- trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp	2020-03-25 21:13:55 UTC (rev 259008)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp	2020-03-25 21:16:13 UTC (rev 259009)
@@ -1770,7 +1770,7 @@
         JSC::JSLockHolder lock(scriptListener.isolatedWorld().vm());
 
         if (document) {
-            handlerObject = scriptListener.jsFunction(*document);
+            handlerObject = scriptListener.ensureJSFunction(*document);
             exec = execStateFromNode(scriptListener.isolatedWorld(), document);
         }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to