| Summary: | Nullptr crash in DocumentLoader::clearMainResourceLoader | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Pinki Gyanchandani <pgyanchandani> | ||||||||
| Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aakash_jain, achristensen, ap, beidson, bfulgham, cdumez, commit-queue, dbates, ews-feeder, ews-watchlist, japhet, product-security, rniwa, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=206304 | ||||||||||
| Bug Depends on: | 206306 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Pinki Gyanchandani
2020-01-13 16:05:49 PST
Created attachment 387592 [details]
Patch
Comment on attachment 387592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387592&action=review r- because change logs need to be fixed. > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). Please remove this line but add a description as to what caused the bug & how you're fixing it. > Source/WebCore/loader/DocumentLoader.cpp:159 > + Please revert this change. > Source/WebCore/loader/DocumentLoader.cpp:1273 > { Should the caller exit early as well? > Tools/ChangeLog:8 > + * WebKitTestRunner/WebKitTestRunner.xcodeproj/xcshareddata/xcschemes/WebKitTestRunner.xcscheme: Please revert this change log change. > ChangeLog:8 > + * WebKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings: Ditto. This is not a security bug. Created attachment 387706 [details]
Patch
Comment on attachment 387706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387706&action=review > Source/WebCore/loader/DocumentLoader.cpp:159 > + Again, please revert this unnecessary code change. I removed the unneeded space addition and committed this to http://trac.webkit.org/r254576 (In reply to Alex Christensen from comment #8) > I removed the unneeded space addition and committed this to http://trac.webkit.org/r254576 Newly added test change-src-during-iframe-load-crash.html seems to be consistently timing out, and slowing down commit-queue. Tracked in https://bugs.webkit.org/show_bug.cgi?id=206304 The test failure was also indicated by EWS in https://ews-build.webkit.org/#/builders/30/builds/604 and https://ews-build.webkit.org/#/builders/10/builds/3730 Re-opened since this is blocked by bug 206306 This also caused http/tests/security/http-0.9/xhr-blocked.html to fail, because of possibly unintended addition of 'asdf' https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Fsecurity%2Fhttp-0.9%2Fxhr-blocked.html Comment on attachment 387706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387706&action=review > LayoutTests/loader/change-src-during-iframe-load-crash.html:3 > +function load() { The issue is that in WebKit1, this event handler runs after eventhandler3 had finished running. The solution is to add a flag which eventhandler3 set, and only call waitUntilDone when the flag isn't set like this: let didLoad = false; let didFinishTesting = false; function load() { document.body.innerHTML = 'The test is declared pass if there is no crash observed.'; didLoad = true; if (window.testRunner) { testRunner.dumpAsText(); if (!didFinishTesting) testRunner.waitUntilDone(); } } function eventhandler3() { iframe1.srcdoc = "x"; didFinishTesting = true; if (window.testRunner && didLoad) testRunner.notifyDone(); } > LayoutTests/loader/change-src-during-iframe-load-crash.html:11 > +function eventhandler3() { Can we rename this event handler to something more sensible like didLoadFrame2. Comment on attachment 387706 [details]
Patch
r- given this patch caused the landed test to fail in WK1.
Created attachment 387876 [details]
Patch
Comment on attachment 387876 [details]
Patch
Let's wait for EWS before landing.
Committed r254662: <https://trac.webkit.org/changeset/254662> |