Null derefs in successful resource revalidation after r100311

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Nate Chapin]

Reported at http://code.google.com/p/chromium/issues/detail?id=104747

There's nothing to stop SubresourceLoaders from getting cancelled in the middle of successful revalidation. This can lead to CachedResource::m_resourceToRevalidate getting nulled out before expected.

I think the solution is to return to the pre-100311 technique: Ensure SubresourceLoader enters its finishing state when we get a 304 in didReceiveResponse.

[Nate Chapin]

Created attachment 115876 [details]
patch

[Adam Barth]

Comment on attachment 115876 [details]
patch

Ok.  It's unfortunate we don't have a test for this change.

[WebKit Review Bot]

Comment on attachment 115876 [details]
patch

Attachment 115876 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10517259

New failing tests:
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/resource-tree/resource-tree-reload.html

[Nate Chapin]

Comment on attachment 115876 [details]
patch

Oops, the uploaded patch has unintended side effects.  I'm going to sit on this until after thanksgiving, since I'm on vacation next week.

[Nate Chapin]

Created attachment 117241 [details]
Better fix + test

[WebKit Review Bot]

Comment on attachment 117241 [details]
Better fix + test

Attachment 117241 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10687469

New failing tests:
http/tests/inspector/resource-har-conversion.html

[Nate Chapin]

Comment on attachment 117241 [details]
Better fix + test

Working on finding a patch that doesn't regress tests.....

[Nate Chapin]

Created attachment 118281 [details]
attempt #3

[Adam Barth]

Comment on attachment 118281 [details]
attempt #3

Nate and I talked about this patch via chat.  We think it might be better to introduce a Revaldiating state so that we don't need to call cancel() in the revalidating case.

[Nate Chapin]

Created attachment 118471 [details]
Finish, rather than cancel, 304s

[WebKit Review Bot]

Comment on attachment 118471 [details]
Finish, rather than cancel, 304s

Clearing flags on attachment: 118471

Committed r102602: <http://trac.webkit.org/changeset/102602>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.