Bug 14400 – Assertion failure (SHOULD NEVER BE REACHED) going back on YouTube

• Component: Forms
• Status: RESOLVED FIXED

[Matt Lilek]

Saw this going back from one search page to another on YouTube with r23753:

SHOULD NEVER BE REACHED
(/Users/matt/Code/WebKit/WebCore/html/HTMLGenericFormElement.cpp:205 virtual void WebCore::HTMLGenericFormElement::restoreState(const WebCore::String&))

Thread 0 Crashed:
0   com.apple.WebCore         	0x010de7ec WebCore::HTMLGenericFormElement::restoreState(WebCore::String const&) + 76 (HTMLGenericFormElement.cpp:205)
1   com.apple.WebCore         	0x010deacc WebCore::HTMLGenericFormElement::closeRenderer() + 248 (HTMLGenericFormElement.cpp:214)
2   com.apple.WebCore         	0x015e9154 WebCore::HTMLParser::popOneBlockCommon() + 120 (HTMLParser.cpp:1296)
3   com.apple.WebCore         	0x010207a8 WebCore::HTMLParser::popOneBlock() + 56 (HTMLParser.cpp:1312)
4   com.apple.WebCore         	0x0102184c WebCore::HTMLParser::popBlock(WebCore::AtomicString const&, bool) + 404 (HTMLParser.cpp:1246)
5   com.apple.WebCore         	0x01024f48 WebCore::HTMLParser::processCloseTag(WebCore::Token*) + 480 (HTMLParser.cpp:856)
6   com.apple.WebCore         	0x010246e0 WebCore::HTMLParser::parseToken(WebCore::Token*) + 468 (HTMLParser.cpp:205)
7   com.apple.WebCore         	0x01027504 WebCore::HTMLTokenizer::processToken() + 608 (HTMLTokenizer.cpp:1641)
8   com.apple.WebCore         	0x0102b2e8 WebCore::HTMLTokenizer::parseTag(WebCore::SegmentedString&, WebCore::HTMLTokenizer::State) + 6936 (HTMLTokenizer.cpp:1206)
9   com.apple.WebCore         	0x0102bea0 WebCore::HTMLTokenizer::write(WebCore::SegmentedString const&, bool) + 1524 (HTMLTokenizer.cpp:1437)
10  com.apple.WebCore         	0x0149f6b8 WebCore::FrameLoader::write(char const*, int, bool) + 1288 (FrameLoader.cpp:948)
11  com.apple.WebCore         	0x0149f824 WebCore::FrameLoader::addData(char const*, int) + 320 (FrameLoader.cpp:1602)
12  com.apple.WebCore         	0x0111effc -[WebCoreFrameBridge addData:] + 232 (WebCoreFrameBridge.mm:290)
13  com.apple.WebCore         	0x01127c14 -[WebCoreFrameBridge receivedData:textEncodingName:] + 316 (WebCoreFrameBridge.mm:1431)
14  com.apple.WebKit          	0x00343fac -[WebHTMLRepresentation receivedData:withDataSource:] + 296 (WebHTMLRepresentation.mm:175)
15  com.apple.WebKit          	0x0033d214 -[WebDataSource(WebInternal) _receivedData:] + 116 (WebDataSource.mm:177)
16  com.apple.WebKit          	0x003c3b14 WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 184
17  com.apple.WebCore         	0x014943f0 WebCore::FrameLoader::committedLoad(WebCore::DocumentLoader*, char const*, int) + 92 (FrameLoader.cpp:3058)
18  com.apple.WebCore         	0x014abad0 WebCore::DocumentLoader::commitLoad(char const*, int) + 104 (DocumentLoader.cpp:351)
19  com.apple.WebCore         	0x014abd38 WebCore::DocumentLoader::receivedData(char const*, int) + 104 (DocumentLoader.cpp:364)
20  com.apple.WebCore         	0x014930a8 WebCore::FrameLoader::receivedData(char const*, int) + 60 (FrameLoader.cpp:2049)
21  com.apple.WebCore         	0x014abfa8 WebCore::MainResourceLoader::addData(char const*, int, bool) + 92 (MainResourceLoader.cpp:137)
22  com.apple.WebCore         	0x014aec8c WebCore::ResourceLoader::didReceiveData(char const*, int, long long, bool) + 104
23  com.apple.WebCore         	0x014ac1f0 WebCore::MainResourceLoader::didReceiveData(char const*, int, long long, bool) + 288 (MainResourceLoader.cpp:292)
24  com.apple.WebCore         	0x014ae5dc WebCore::ResourceLoader::didReceiveData(WebCore::ResourceHandle*, char const*, int, int) + 108
25  com.apple.WebCore         	0x01482b34 -[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:] + 240 (ResourceHandleMac.mm:352)
26  com.apple.Foundation      	0x92c12624 -[NSURLConnection(NSURLConnectionInternal) _sendDidReceiveDataCallback] + 564
27  com.apple.Foundation      	0x92c10ac4 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 488
28  com.apple.Foundation      	0x92c10860 _sendCallbacks + 156
29  com.apple.CoreFoundation  	0x907de4fc __CFRunLoopDoSources0 + 384
30  com.apple.CoreFoundation  	0x907dda2c __CFRunLoopRun + 452
31  com.apple.CoreFoundation  	0x907dd4ac CFRunLoopRunSpecific + 268
32  com.apple.HIToolbox       	0x93297b20 RunCurrentEventLoopInMode + 264
33  com.apple.HIToolbox       	0x932971b4 ReceiveNextEventCommon + 380
34  com.apple.HIToolbox       	0x93297020 BlockUntilNextEventMatchingListInMode + 96
35  com.apple.AppKit          	0x9379dae4 _DPSNextEvent + 384
36  com.apple.AppKit          	0x9379d7a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
37  com.apple.Safari          	0x00006770 0x1000 + 22384
38  com.apple.AppKit          	0x93799cec -[NSApplication run] + 472
39  com.apple.AppKit          	0x9388a87c NSApplicationMain + 452
40  com.apple.Safari          	0x0000244c 0x1000 + 5196

Created attachment 15313 [details]
Reduction

Created attachment 15383 [details]
Avoid restoring state for elements that do not register for saving state

The layout test works on release builds as well (i.e. fails without the patch).

[Darin Adler]

Comment on attachment 15383 [details]
Avoid restoring state for elements that do not register for saving state

This registersWithDocument function can be private rather than protected, because you don't need access to a function to override it.

But it also seems more robust to me to have HTMLGenericFormElement::closeRenderer check the hash table in the document rather than have a virtual function that has to be kept in sync with the registerFormElementWithState call. Maybe a function called hasFormElementWithState or isFormElementWithStateRegistered in Document. To be called in closeRenderer.

Comment on attachment 15383 [details]
Avoid restoring state for elements that do not register for saving state

Going to do it the way Darin suggested.

Created attachment 15397 [details]
Avoid restoring state for elements that did not register for saving state

The reason I didn't do it this way in the first place is that I mistakenly thought that closeRenderer() was called before the element registered if it wanted to. That's not the case, and this patch is indeed smaller, simpler and more future-proof.

[Maciej Stachowiak]

Comment on attachment 15397 [details]
Avoid restoring state for elements that did not register for saving state

r=me

[Mark Rowe (bdash)]

Landed in r24055.

[Mark Rowe (bdash)]

Landed in r24055.