Bug 61733 – [Chromium] REGRESSION: Crash in WebCore::HTMLLinkElement::onloadTimerFired after r87628

Bug 61733 - [Chromium] REGRESSION: Crash in WebCore::HTMLLinkElement::onloadTimerFired after r87628


Summary:	[Chromium] REGRESSION: Crash in WebCore::HTMLLinkElement::onloadTimerFired af...

Status:	RESOLVED FIXED

Product:	WebKit
Component:	HTML DOM
Version:	528+ (Nightly build)
Platform:	All All

Importance:	P1 Normal
Assigned To:	Mikhail Naganov

URL:
Keywords:

Depends on:	61736
Blocks:
	Show dependency tree / graph

Reported:	2011-05-30 09:31 PST by Mikhail Naganov
Modified:	2011-05-30 12:29 PST (History)

Attachments
patch (1.10 KB, patch) 2011-05-30 09:37 PST, Mikhail Naganov	no flags	Review Patch \| Details \| Formatted Diff \| Diff
Updated patch (2.02 KB, patch) 2011-05-30 10:15 PST, Adam Barth	no flags	Review Patch \| Details \| Formatted Diff \| Diff
Hide 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 From Mikhail Naganov 2011-05-30 09:31:32 PST

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 From Mikhail Naganov 2011-05-30 09:37:28 PST -------

Created an attachment (id=95348) [details]
patch

------- Comment #2 From Adam Barth 2011-05-30 09:54:43 PST -------

(From update of attachment 95348 [details])
I'm not sure this patch is correct.  Why is notifyFinished being called with a different cached resource?

------- Comment #3 From Adam Barth 2011-05-30 10:15:41 PST -------

Created an attachment (id=95351) [details]
Updated patch

------- Comment #4 From Adam Barth 2011-05-30 10:19:12 PST -------

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 From Adam Barth 2011-05-30 10:21:19 PST -------

Committed r87693: <http://trac.webkit.org/changeset/87693>

------- Comment #6 From Alexey Proskuryakov 2011-05-30 12:29:26 PST -------

Thanks Adam! I don't have the time to deeply investigate this right now, but the patch looks very reasonable.