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

See Also:

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

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


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]
Comment 2 Adam Barth 2011-05-30 09:54:43 PDT
Comment on attachment 95348 [details]

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.