third_party/WebKit/LayoutTests/fast/events/crash-on-querying-event-path.html - Issue 1374533002: Null out LocalDOMWindow::m_frame on navigation.

Side by Side Diff: third_party/WebKit/LayoutTests/fast/events/crash-on-querying-event-path.html

Issue 1374533002: Null out LocalDOMWindow::m_frame on navigation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: try to fix crash-on-querying-event-path.html Created 5 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/fast/dom/resources/javascript-url-crash-function-iframe.html ('k') | third_party/WebKit/LayoutTests/fast/events/crash-on-querying-event-path-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <html>	1 <html>

2 <head>	2 <head>

3 <script src="/js-test-resources/js-test.js"></script>	3 <script src="../../resources/js-test.js"></script>

4 </head>	4 </head>

5 <body>	5 <body>

6 <script>	6 <script>

7 var jsTestIsAsync = true;	7 var jsTestIsAsync = true;

8 description('This is a regression test for crbug.com/400476. It should not crash and then brag about it.')	8 description('This is a regression test for crbug.com/400476. It should not crash and then brag about it.')

9	9

10 var root = document.documentElement;	10 var root = document.documentElement;

11 var iframe = root.ownerDocument.createElement('iframe');	11 var iframe = root.ownerDocument.createElement('iframe');

12 var timeouts = [];	12 iframe.onload = function() {

13 iframe.onload = iframeOnload;

14 root.appendChild(iframe);

15

16 function iframeOnload() {

17 var defaultView = iframe.contentDocument.defaultView;	13 var defaultView = iframe.contentDocument.defaultView;

18 defaultView.onpageshow = onPageShow;	14 defaultView.onpageshow = onPageShow;

19 iframe.src = null;	15 window.setTimeout(tryToCrash, 0);
dcheng 2015/09/30 00:46:18 This test was flaking, so I did some investigation This test was flaking, so I did some investigation: 1) It doesn't actually need the http server, so I copied it into fast/events. This was enough to unflake the test by itself. 2) I was unable to get either version of the test (fast/events vs http/tests/dom) to crash with the code changes in https://codereview.chromium.org/533633002 reverted (even with no test changes). 3) I synced back to 2014-Sep-02 and rebuilt without vogelheim@'s fix. Then both test cases started reliably crashing in ASAN. 4) I tweaked the test a bit to eliminate setTimeout and discovered: - iframe navigation is not required to make it crash. - it crashes at the gc() calls, before even accessing event.path. 5) In fact, even with the code fixes reapplied, both versions of the test still crash the same way in ASAN at Blink commit r181234. 6) The test (both versions) do not crash in ASAN at ToT. I don't honestly know what to make of #4-#6. With no navigation required, all the objects should still be live: there's nothing tricky happening with lifetimes. My only conclusion is that there's something really wrong with GC tracing in this version of Chrome. Since both versions of the test appear to crash ASAN the same way, I opted to remove the http test and just keep the modified version in fast/events.
20 timeouts[timeouts.length] = window.setTimeout(nextIframeLoaded, 100);	16 };

21 }	17 root.appendChild(iframe);

22	18

23 function onPageShow() {	19 function onPageShow() {

24 eventObj = arguments[0];	20 eventObj = arguments[0];

25 }	21 }

26	22

27 function nextIframeLoaded() {	23 function tryToCrash() {

28 timeouts.forEach(window.clearTimeout);

29

30 // Access of eventObj.path caused the crash.	24 // Access of eventObj.path caused the crash.

31 // The test is somewhat flaky, in that the test may pass as correct	25 // The test is somewhat flaky, in that the test may pass as correct

32 // despite the bug being the code. The exact conditions	26 // despite the bug being the code. The exact conditions

33 // are unclear, but 1, asan helps detect the crash and 2, the	27 // are unclear, but 1, asan helps detect the crash and 2, the

34 // preceeding gc()s increase the likelihood of it occurring.	28 // preceeding gc()s increase the likelihood of it occurring.

35 gc();	29 gc();

36 gc();	30 gc();

37 gc();	31 gc();

38 gc();	32 gc();

39 gc();	33 gc();

40 var path = eventObj.path;	34 var path = eventObj.path;

41 debug(path);	35 debug(path);

42	36

43 testPassed('totally did not crash.');	37 testPassed('totally did not crash.');

44 finishJSTest();	38 finishJSTest();

45 }	39 }

46 </script>	40 </script>

47 </body>	41 </body>

48 </html>	42 </html>

OLD	NEW