Bug 61733 - [Chromium] REGRESSION: Crash in WebCore::HTMLLinkElement::onloadTimerFired after r87628
Summary: [Chromium] REGRESSION: Crash in WebCore::HTMLLinkElement::onloadTimerFired af...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Mikhail Naganov
URL:
Keywords:
Depends on: 61736
Blocks:
  Show dependency tree / graph
 
Reported: 2011-05-30 09:31 PDT by Mikhail Naganov
Modified: 2011-05-30 12:29 PDT (History)
2 users (show)

See Also:

Attachments
patch (1.10 KB, patch)
2011-05-30 09:37 PDT, Mikhail Naganov 		no flags Details | Formatted Diff | Diff
Updated patch (2.02 KB, patch)
2011-05-30 10:15 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Naganov 2011-05-30 09:31:32 PDT 
Having r87628 in place, Chrome reliability bot crashes in WebCore::HTMLLinkElement::onloadTimerFired

http://build.chromium.org/p/chromium/builders/Win%20Reliability/builds/4073/steps/reliability%3A%20partial%20result%20of%20current%20build/logs/stdio

This is because the change makes WebCore::CachedResource::setRequest to call checkNotify on request reset.
HTMLLinkElement registers itself as CachedResource client via m_cachedSheet, which can happen even if m_cachedLinkResource wasn't set.
As a result, WebCore::HTMLLinkElement::notifyFinished is got called with unset m_cachedLinkResource, which causes a crash in HTMLLinkElement::onloadTimerFired
Comment 1 Mikhail Naganov 2011-05-30 09:37:28 PDT 
Created attachment 95348 [details]
patch
Comment 2 Adam Barth 2011-05-30 09:54:43 PDT 
Comment on attachment 95348 [details]
patch

I'm not sure this patch is correct.  Why is notifyFinished being called with a different cached resource?
Comment 3 Adam Barth 2011-05-30 10:15:41 PDT 
Created attachment 95351 [details]
Updated patch
Comment 4 Adam Barth 2011-05-30 10:19:12 PDT 
I'm going to land this patch without a test because this is blocking WebKit => Chromium integration.  I'll add the test in Bug 61736.
Comment 5 Adam Barth 2011-05-30 10:21:19 PDT 
Committed r87693: <http://trac.webkit.org/changeset/87693>
Comment 6 Alexey Proskuryakov 2011-05-30 12:29:26 PDT 
Thanks Adam! I don't have the time to deeply investigate this right now, but the patch looks very reasonable.