Created attachment 46890 [details]
test case (will crash)
Thread 0 Crashed: Dispatch queue: com.apple.main-thread
0 com.apple.WebCore 0x0000000100e43564 WebCore::Page::backForwardList() + 4
1 com.apple.WebCore 0x0000000100a64cb8 WebCore::HistoryController::replaceState(WTF::PassRefPtr<WebCore::SerializedScriptValue>, WebCore::String const&, WebCore::String const&) + 56
2 com.apple.WebCore 0x0000000100a64978 WebCore::History::stateObjectAdded(WTF::PassRefPtr<WebCore::SerializedScriptValue>, WebCore::String const&, WebCore::String const&, WebCore::History::StateObjectType, int&) + 440
3 com.apple.WebCore 0x0000000100c0f32a WebCore::JSHistory::replaceState(JSC::ExecState*, JSC::ArgList const&) + 650
4 com.apple.WebCore 0x0000000100c0e489 WebCore::jsHistoryPrototypeFunctionReplaceState(JSC::ExecState*, JSC::JSObject*, JSC::JSValue, JSC::ArgList const&) + 137
The real question here is... what's the expected behavior? Should you be able to pushState() or replaceState() on a detached frame?
Clearly we could just do an early return, but what if someone does the state-API on the detached frame then reattaches it?
The question of "does a detached frame even have a back/forward list to manipulate?" already has an answer elsewhere in our code - calling history.length() on a detached frame will return 0 (not even 1!!)
I'm pretty sure an early return is the right way to go here. If we later uncover an incompatibility with the spec or the real web, then we can do what's needed to fix that.
Created attachment 46922 [details]
Comment on attachment 46922 [details]
"The spec really cover expected behavior"?
The test should not just have empty output - a link to the bug and a phrase like "Passed in didn't crash" would suffice.
Please add try/catch around each subtest - we want to test pushState even if replaceState raises an exception.
r=me if you fix the above nitpicks.