STEPS TO REPRODUCE: 1) Build a revision of WebKit after 71884. 2) Run it with Safari 3) Install the attached extension EXPECTED RESULTS: The browser shouldn't crash ACTUAL RESULTS: It does - it hits an assert, then crashes: > WebKit.dll!WTF::Deque<WTF::RefPtr<WebCore::ResourceLoader> >::removeFirst() Line 470 + 0x2b bytes C++ WebKit.dll!WebCore::ResourceLoadScheduler::servePendingRequests(WebCore::ResourceLoadScheduler::HostInformation * host=0x1580af70, WebCore::ResourceLoadScheduler::Priority minimumPriority=Medium) Line 198 C++ WebKit.dll!WebCore::ResourceLoadScheduler::scheduleLoad(WebCore::ResourceLoader * resourceLoader=0x21714d58, WebCore::ResourceLoadScheduler::Priority priority=Medium) Line 120 C++ WebKit.dll!WebCore::ResourceLoadScheduler::scheduleSubresourceLoad(WebCore::Frame * frame=0x1fc118d0, WebCore::SubresourceLoaderClient * client=0x15800e5c, const WebCore::ResourceRequest & request={...}, WebCore::ResourceLoadScheduler::Priority priority=Medium, WebCore::SecurityCheckPolicy securityCheck=DoSecurityCheck, bool sendResourceLoadCallbacks=true, bool shouldContentSniff=true) Line 89 C++ WebKit.dll!WebCore::Loader::load(WebCore::CachedResourceLoader * cachedResourceLoader=0x1edcef78, WebCore::CachedResource * resource=0x22888e08, bool incremental=false, WebCore::SecurityCheckPolicy securityCheck=DoSecurityCheck, bool sendResourceLoadCallbacks=true) Line 132 + 0x4f bytes C++ WebKit.dll!WebCore::CachedResource::load(WebCore::CachedResourceLoader * cachedResourceLoader=0x1edcef78, bool incremental=false, WebCore::SecurityCheckPolicy securityCheck=DoSecurityCheck, bool sendResourceLoadCallbacks=true) Line 111 C++ WebKit.dll!WebCore::CachedResource::load(WebCore::CachedResourceLoader * cachedResourceLoader=0x1edcef78) Line 79 + 0x20 bytes C++ WebKit.dll!WebCore::MemoryCache::requestResource(WebCore::CachedResourceLoader * cachedResourceLoader=0x1edcef78, WebCore::CachedResource::Type type=Script, const WebCore::KURL & url={???}, const WTF::String & charset={???}, bool requestIsPreload=false) Line 131 + 0x13 bytes C++ WebKit.dll!WebCore::CachedResourceLoader::requestResource(WebCore::CachedResource::Type type=Script, const WTF::String & url={???}, const WTF::String & charset={???}, bool isPreload=false) Line 266 + 0x20 bytes C++ WebKit.dll!WebCore::CachedResourceLoader::requestScript(const WTF::String & url={???}, const WTF::String & charset={???}) Line 163 C++ WebKit.dll!WebCore::HTMLScriptRunner::requestPendingScript(WebCore::PendingScript & pendingScript={...}, WebCore::Element * script=0x1eb5cfa0) Line 284 + 0x44 bytes C++ WebKit.dll!WebCore::HTMLScriptRunner::requestParsingBlockingScript(WebCore::Element * element=0x1eb5cfa0) Line 250 + 0x13 bytes C++ WebKit.dll!WebCore::HTMLScriptRunner::runScript(WebCore::Element * script=0x1eb5cfa0, const WTF::TextPosition<WTF::OneBasedNumber> & scriptStartPosition={...}) Line 316 C++ WebKit.dll!WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element> scriptElement={...}, const WTF::TextPosition<WTF::OneBasedNumber> & scriptStartPosition={...}) Line 185 C++ WebKit.dll!WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() Line 199 + 0x23 bytes C++ WebKit.dll!WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode mode=AllowYield) Line 235 + 0x8 bytes C++ WebKit.dll!WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode mode=AllowYield) Line 170 C++ WebKit.dll!WebCore::HTMLDocumentParser::append(const WebCore::SegmentedString & source={...}) Line 312 C++ WebKit.dll!WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter * writer=0x1fc11a54, const char * data=0x00000000, int length=0, bool shouldFlush=true) Line 54 + 0x1f bytes C++ WebKit.dll!WebCore::DocumentWriter::addData(const char * str=0x00000000, int len=0, bool flush=true) Line 200 + 0x1f bytes C++ WebKit.dll!WebCore::DocumentWriter::endIfNotLoadingMainResource() Line 221 C++ WebKit.dll!WebCore::DocumentWriter::end() Line 207 C++ WebKit.dll!WebCore::DocumentLoader::finishedLoading() Line 278 C++ WebKit.dll!WebCore::FrameLoader::finishedLoading() Line 2174 C++ WebKit.dll!WebCore::MainResourceLoader::didFinishLoading(double finishTime=0.00000000000000000) Line 458 C++ WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x0d0aaff0, double finishTime=0.00000000000000000) Line 435 + 0x18 bytes C++ WebKit.dll!WebCore::didFinishLoading(_CFURLConnection * conn=0x1f075fe0, const void * clientInfo=0x0d0aaff0) Line 244 + 0x26 bytes C++
Created attachment 74166 [details] Extension that crashes
<rdar://problem/8680809>
I'll take a look at this first thing tomorrow.
(In reply to comment #3) > I'll take a look at this first thing tomorrow. Thanks, let me know if you have any questions.
Created attachment 74280 [details] patch + test It's a pretty simple fix. If we start the load, then remove the ResourceLoader from the queue after, it might have been cancelled and already removed from the queue (causing a crash if the queue is now empty). Instead, we should remove the ResourceLoader from the queue before starting it.
Created attachment 74281 [details] new patch + test This version moves both removing from the queue and adding to the set of loads in progress. By doing them both before, we can trust ResourceLoadScheduler::remove() to do the right thing without any special logic in servePendingRequests(), even when called syncrhonously.
looks good to me too
The commit-queue encountered the following flaky tests while processing attachment 74281 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html compositing/iframes/overlapped-nested-iframes.html Please file bugs against the tests. These tests were authored by dumi@chromium.org and simon.fraser@apple.com. The commit-queue is continuing to process your patch.
The commit-queue encountered the following flaky tests while processing attachment 74281 [details]: inspector/timeline-parse-html.html Please file bugs against the tests. These tests were authored by pfeldman@chromium.org and yurys@chromium.org. The commit-queue is continuing to process your patch.
Comment on attachment 74281 [details] new patch + test Clearing flags on attachment: 74281 Committed r72435: <http://trac.webkit.org/changeset/72435>
All reviewed patches have been landed. Closing bug.