WebKitTestRunner get stuck on this: <script> onload = () => document.createElement('embed'); </script> the results are the same for the object and applet tags. Steps To Reproduce: DYLD_FRAMEWORK_PATH=<WEBKIT_PATH>/WebKitBuild/Debug <WEBKIT_PATH>/WebKitBuild/Debug/WebKitTestRunner repro_407.html Results: WebKitTestRunner gets stuck, waiting forever. For the slightly modified test case: <script> onload = () => { testRunner.waitUntilDone(); testRunner.notifyDone(); $vm.print('before'); document.createElement('embed'); $vm.print('after'); throw new Error(`an error`); } </script> <rdar://problem/69554695>
Created attachment 419592 [details] Patch
Comment on attachment 419592 [details] Patch Definitely needs a test. I don't think this is the right fix, because if it were we would need to do this at all call sites of incrementLoadEventDelayCount and we don't.
Created attachment 419728 [details] Patch
(In reply to Alex Christensen from comment #2) > Comment on attachment 419592 [details] > Patch > > Definitely needs a test. > I don't think this is the right fix, because if it were we would need to do > this at all call sites of incrementLoadEventDelayCount and we don't. No test at the time since I was not sure if it is a security issue. I am now more convinced it is not a security issue, so I added a test in the new patch.
Comment on attachment 419728 [details] Patch This isn't obviously wrong like the first one was, but I don't know why finishCreating was there in the first place, or what the consequences of this can be.
(In reply to Alex Christensen from comment #5) > Comment on attachment 419728 [details] > Patch > > This isn't obviously wrong like the first one was, but I don't know why > finishCreating was there in the first place, or what the consequences of > this can be. The relevant change seems to be https://bugs.webkit.org/show_bug.cgi?id=130653. AFAICS that change moved scheduling to an earlier point in time, i.e. element creation instead of children parsing to be finished or src to be set. Not sure why that change in behaviour (if I have read that code right) would be needed, maybe Darin remembers? The scheduling is done even though the embed element may never be appended to the document, like in the test case in this bug.
Comment on attachment 419728 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419728&action=review This looks right. We call scheduleUpdateForAfterStyleResolution() whenever we parse an attribute so it should be safe to remove the call in the constructor. > LayoutTests/plugins/embed-creation-crash-expected.txt:1 > +This test should not crash. You mean this shouldn't hang?
Comment on attachment 419728 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419728&action=review >> LayoutTests/plugins/embed-creation-crash-expected.txt:1 >> +This test should not crash. > > You mean this shouldn't hang? Good point, will fix before landing.
Created attachment 419818 [details] Patch
Committed r272640: <https://commits.webkit.org/r272640> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419818 [details].
(In reply to Rob Buis from comment #8) > Comment on attachment 419728 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=419728&action=review > > >> LayoutTests/plugins/embed-creation-crash-expected.txt:1 > >> +This test should not crash. > > > > You mean this shouldn't hang? > > Good point, will fix before landing. You forgot to rename the file name LOL. rs=me on the rename from -crash to -hang.
Reopening to attach new patch.
Created attachment 419846 [details] Patch
Created attachment 419852 [details] Patch
Committed r272676: <https://commits.webkit.org/r272676> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419852 [details].