Summary: | storage/indexeddb/detached-iframe.html is flaky on some bots | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | Tools / Tests | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, alecflett, beidson, ews-watchlist, ggaren, jsbell, Lawrence.j, sihui_liu, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Chris Dumez
2021-07-28 15:24:14 PDT
Created attachment 434472 [details]
Patch
Comment on attachment 434472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434472&action=review > LayoutTests/storage/indexeddb/detached-iframe.html:7 > +jsTestIsAsync = true; Isn't it set in shared.js? Comment on attachment 434472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434472&action=review >> LayoutTests/storage/indexeddb/detached-iframe.html:7 >> +jsTestIsAsync = true; > > Isn't it set in shared.js? Hmm, it does. Yet, my change is causing the output to show up so there is a behavior change somehow.. Comment on attachment 434472 [details]
Patch
Will investigate further.
(In reply to Chris Dumez from comment #4) > Comment on attachment 434472 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=434472&action=review > > >> LayoutTests/storage/indexeddb/detached-iframe.html:7 > >> +jsTestIsAsync = true; > > > > Isn't it set in shared.js? > > Hmm, it does. Yet, my change is causing the output to show up so there is a > behavior change somehow.. Probably output from description() Comment on attachment 434472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434472&action=review > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:2 > +IndexedDB edge case with a detached iframe. description() would add this logging ... > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:4 > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". ... and this one. > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:7 > +indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; description() does not explain this logging? (In reply to Chris Dumez from comment #7) > Comment on attachment 434472 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=434472&action=review > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:2 > > +IndexedDB edge case with a detached iframe. > > description() would add this logging ... > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:4 > > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > ... and this one. > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:7 > > +indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; > > description() does not explain this logging? Right, I thought these lines should always be printed. Something wrong with the DOM nodes... testIframe.parentNode.remove(testIframe); should probably be: testIframe.parentNode.removeChild(testIframe); (In reply to Sihui Liu from comment #8) > (In reply to Chris Dumez from comment #7) > > Comment on attachment 434472 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=434472&action=review > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:2 > > > +IndexedDB edge case with a detached iframe. > > > > description() would add this logging ... > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:4 > > > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > > > ... and this one. > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:7 > > > +indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; > > > > description() does not explain this logging? > > Right, I thought these lines should always be printed. Something wrong with > the DOM nodes... > > testIframe.parentNode.remove(testIframe); > should probably be: > testIframe.parentNode.removeChild(testIframe); It's equivalent, so is `testIframe.remove()`. It has no impact on the output of the test. Just adding the description() makes the output show (seemingly consistently) but I don't have an explanation why at the moment... Created attachment 434523 [details]
Patch
(In reply to Chris Dumez from comment #9) > (In reply to Sihui Liu from comment #8) > > (In reply to Chris Dumez from comment #7) > > > Comment on attachment 434472 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=434472&action=review > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:2 > > > > +IndexedDB edge case with a detached iframe. > > > > > > description() would add this logging ... > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:4 > > > > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > > > > > ... and this one. > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:7 > > > > +indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; > > > > > > description() does not explain this logging? > > > > Right, I thought these lines should always be printed. Something wrong with > > the DOM nodes... > > > > testIframe.parentNode.remove(testIframe); > > should probably be: > > testIframe.parentNode.removeChild(testIframe); > > It's equivalent, so is `testIframe.remove()`. It has no impact on the output > of the test. Just adding the description() makes the output show (seemingly > consistently) but I don't have an explanation why at the moment... I checked with MutationObserver, testIframe.parentNode.remove(testIframe) removes document.body, testIframe.parentNode.removeChild(testIframe) removes iframe. Removing document.body makes the console element added in different places (function getOrCreate(id, tagName)@js-tests.js)... (In reply to Sihui Liu from comment #11) > (In reply to Chris Dumez from comment #9) > > (In reply to Sihui Liu from comment #8) > > > (In reply to Chris Dumez from comment #7) > > > > Comment on attachment 434472 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=434472&action=review > > > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:2 > > > > > +IndexedDB edge case with a detached iframe. > > > > > > > > description() would add this logging ... > > > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:4 > > > > > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > > > > > > > ... and this one. > > > > > > > > > LayoutTests/storage/indexeddb/detached-iframe-expected.txt:7 > > > > > +indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; > > > > > > > > description() does not explain this logging? > > > > > > Right, I thought these lines should always be printed. Something wrong with > > > the DOM nodes... > > > > > > testIframe.parentNode.remove(testIframe); > > > should probably be: > > > testIframe.parentNode.removeChild(testIframe); > > > > It's equivalent, so is `testIframe.remove()`. It has no impact on the output > > of the test. Just adding the description() makes the output show (seemingly > > consistently) but I don't have an explanation why at the moment... > > I checked with MutationObserver, testIframe.parentNode.remove(testIframe) > removes document.body, testIframe.parentNode.removeChild(testIframe) removes > iframe. Removing document.body makes the console element added in different > places (function getOrCreate(id, tagName)@js-tests.js)... Oh, you are right that testIframe.parentNode.remove(testIframe) seems very wrong. It removes the parent node instead of the frame, LOL :) I got confused because ParentNode.idl has append/prepend (but not remove). I'll test again but I believe fixing that didn't make the output appear though. For me, it was switching to js-test-pre.js/js-test-post.js (or calling description()) that seemed to do the trick. Comment on attachment 434523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434523&action=review > LayoutTests/storage/indexeddb/detached-iframe.html:8 > + document.getElementById('testIframe').remove(); Actually, this fix alone is sufficient to make the output show. I got confused because the test is marked as flaky internally. I'll revert the js-test-pre.js / js-test-post.js change. Created attachment 434527 [details]
Patch
Committed r280426 (240066@main): <https://commits.webkit.org/240066@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 434527 [details]. *** Bug 207844 has been marked as a duplicate of this bug. *** Unskipped test in <https://commits.webkit.org/r280427>. |