PageGroupLoadDeferrer suspends all active dom timers and loading jobs. But it doesn't suspend Document::m_executeScriptSoonTimer. In the case a m_executeScriptSoonTimer is already scheduled, and a JS alert window pops up (which is guarded by PageGroupLoadDeferrer), Document::m_executeScriptSoonTimer can still fire and run JS on the same doc, and then can schedule DOM timers on the doc. I've seen it results ASSERTION failure in DOMTimer when the PageGroupLoadDeferrer destructs. (it resumes dom timers, and every dom timer asserts that it is currently suspended.) But this problem cannot be reproduced reliably because it depends on timing. It happens only when m_executeScriptSoonTimer has been scheduled by not fired yet before a PageGroupLoadDeferrer is constructed. The solution is simple: Just suspend all executeScriptSoonTimer timers in PageGroupLoadDeferrer ctor, and resume them in PageGroupLoadDeferrer dtro. But it's very hard to create a layout test for it. Help needed!!!
Created attachment 55707 [details] the patch
Created attachment 55718 [details] fix merge error
Created attachment 57099 [details] Test case included thanks to Robin! Thanks to Robin Cao, who created the test case. This test cast can reproduce the problem reliably. Robin has verified this problem happens on both Safari for Windows and QtLauncher.
Attachment 57099 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/manual-tests/resources/load-deferrer-script-element.js:1: One or more unexpected \r (^M) found; better to use only a \n [whitespace/carriage_return] [1] Suppressing further [whitespace/carriage_return] reports for this file. Total errors found: 5 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 57102 [details] With enhanced test case
Comment on attachment 57102 [details] With enhanced test case Can this work be put into the suspendActiveDOMObjects and resumeActiveDOMObjects function rather than adding two more member functions to Document? I’d prefer to do it that way. It’s OK this way, but I’d prefer not to modify Document.h and PageGroupLoadDeferrer.cpp. r=me
Let's open a second bug report to refactor it. I think we have a couple of additional similar patches coming.
Comment on attachment 57102 [details] With enhanced test case I agree they should be merged into one function, but the old function name seems a little bit misleading. PageGroupLoadDeferrer needs to be purified more, and refactoring can be done in next patches. Committing it for now?
Comment on attachment 57102 [details] With enhanced test case Clearing flags on attachment: 57102 Committed r61126: <http://trac.webkit.org/changeset/61126>
All reviewed patches have been landed. Closing bug.