33830 2010-01-18 23:55:51 -0800 assertion failure calling history.pushState within popstate event handler 2010-01-20 18:23:35 -0800 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fishd webkit-unassigned beidson oldest_to_newest 182158 0 46892 fishd 2010-01-18 23:55:51 -0800 Created attachment 46892 test case assertion failure calling history.pushState within popstate event handler call stack: > test_shell.exe!WebCore::FrameLoader::navigateWithinDocument(WebCore::HistoryItem * item=0x096e80f8) Line 3707 + 0x30 bytes C++ test_shell.exe!WebCore::FrameLoader::loadItem(WebCore::HistoryItem * item=0x096e80f8, WebCore::FrameLoadType loadType=FrameLoadTypeIndexedBackForward) Line 3830 C++ test_shell.exe!WebCore::HistoryController::recursiveGoToItem(WebCore::HistoryItem * item=0x096e80f8, WebCore::HistoryItem * fromItem=0x008cc148, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward) Line 587 C++ test_shell.exe!WebCore::HistoryController::goToItem(WebCore::HistoryItem * targetItem=0x096e80f8, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward) Line 228 C++ test_shell.exe!WebKit::WebFrameImpl::loadHistoryItem(const WebKit::WebHistoryItem & item={...}) Line 759 C++ test_shell.exe!TestShell::Navigate(const TestNavigationEntry & entry={...}, bool reload=false) Line 627 + 0x2b bytes C++ test_shell.exe!TestNavigationController::NavigateToPendingEntry(bool reload=false) Line 219 + 0x16 bytes C++ test_shell.exe!TestNavigationController::GoToIndex(int index=2) Line 87 C++ test_shell.exe!TestNavigationController::GoToOffset(int offset=-1) Line 77 C++ test_shell.exe!TestWebViewDelegate::navigateBackForwardSoon(int offset=-1) Line 552 C++ test_shell.exe!WebKit::FrameLoaderClientImpl::handleBackForwardNavigation(const WebCore::KURL & url={...}) Line 1464 + 0x21 bytes C++ test_shell.exe!WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction(void (WebCore::PolicyAction)* function=0x01f0acd0, const WebCore::NavigationAction & action={...}, const WebCore::ResourceRequest & request={...}, WTF::PassRefPtr<WebCore::FormState> formState={...}) Line 899 C++ test_shell.exe!WebCore::PolicyChecker::checkNavigationPolicy(const WebCore::ResourceRequest & request={...}, WebCore::DocumentLoader * loader=0x097016d8, WTF::PassRefPtr<WebCore::FormState> formState={...}, void (void *, const WebCore::ResourceRequest &, WTF::PassRefPtr<WebCore::FormState>, bool)* function=0x01ad5030, void * argument=0x0083e478) Line 89 C++ test_shell.exe!WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader * loader=0x097016d8, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward, WTF::PassRefPtr<WebCore::FormState> prpFormState={...}) Line 2087 C++ test_shell.exe!WebCore::FrameLoader::loadWithNavigationAction(const WebCore::ResourceRequest & request={...}, const WebCore::NavigationAction & action={...}, bool lockHistory=false, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward, WTF::PassRefPtr<WebCore::FormState> formState={...}) Line 2010 C++ test_shell.exe!WebCore::FrameLoader::navigateToDifferentDocument(WebCore::HistoryItem * item=0x008cb890, WebCore::FrameLoadType loadType=FrameLoadTypeIndexedBackForward) Line 3806 C++ test_shell.exe!WebCore::FrameLoader::loadItem(WebCore::HistoryItem * item=0x008cb890, WebCore::FrameLoadType loadType=FrameLoadTypeIndexedBackForward) Line 3832 C++ test_shell.exe!WebCore::HistoryController::recursiveGoToItem(WebCore::HistoryItem * item=0x008cb890, WebCore::HistoryItem * fromItem=0x008cc148, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward) Line 587 C++ test_shell.exe!WebCore::HistoryController::goToItem(WebCore::HistoryItem * targetItem=0x008cb890, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward) Line 228 C++ test_shell.exe!WebCore::Page::goToItem(WebCore::HistoryItem * item=0x008cb890, WebCore::FrameLoadType type=FrameLoadTypeIndexedBackForward) Line 311 C++ test_shell.exe!WebCore::Page::goBackOrForward(int distance=-1) Line 288 C++ test_shell.exe!WebCore::RedirectScheduler::timerFired(WebCore::Timer<WebCore::RedirectScheduler> * __formal=0x0083e790) Line 302 C++ test_shell.exe!WebCore::Timer<WebCore::RedirectScheduler>::fired() Line 98 + 0x23 bytes C++ test_shell.exe!WebCore::ThreadTimers::sharedTimerFiredInternal() Line 112 + 0xf bytes C++ test_shell.exe!WebCore::ThreadTimers::sharedTimerFired() Line 91 C++ code snippet: history()->setCurrentItem(item); // loadInSameDocument() actually changes the URL and notifies load delegates of a "fake" load loadInSameDocument(item->url(), item->stateObject(), false); // Restore user view state from the current history item here since we don't do a normal load. // Even though we just manually set the current history item, this ASSERT verifies nothing // inside of loadInSameDocument() caused it to change. ASSERT(history()->currentItem() == item); ^^^ within loadInSameDocument, we dispatch the popstate event. the test case, calls pushState, which has the effect of synchronously changing the current HistoryItem. 182254 1 beidson 2010-01-19 08:36:55 -0800 Coincidentally enough I'd already found this in my WIP for https://bugs.webkit.org/show_bug.cgi?id=33538 and was just going to include removing the ASSERT in that patch. We might as well just do a one-liner to remove the assertion here. 183008 2 beidson 2010-01-20 18:12:27 -0800 There is a real bug the ASSERT is trying to describe. In this case, if there was meaningful state to restore, then we'd end up restoring it from the wrong item. That said, I'd argue that is a minor edge case bug compared to this ASSERTion causing constant grief for people working heavily in this area (which I certainly am). Filed https://bugs.webkit.org/show_bug.cgi?id=33931 to follow up with the real release build symptom. Patch to remove this assert is upcoming. 183011 3 47089 beidson 2010-01-20 18:19:38 -0800 Created attachment 47089 Patch 183013 4 beidson 2010-01-20 18:23:35 -0800 http://trac.webkit.org/changeset/53590 46892 2010-01-18 23:55:51 -0800 2010-01-18 23:55:51 -0800 test case pushstate-during-popstate.html text/html 742 fishd PHNjcmlwdD4KZnVuY3Rpb24gbG9nKHMpIHsKICBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgibG9n IikuZGF0YSArPSBzICsgIlxuIjsKfQpmdW5jdGlvbiBjaGFuZ2VIaXN0b3J5KGNtZCwgcykgewog IGxvZygiaGlzdG9yeS4iICsgY21kICsgIigiICsgcyArICIpIik7CiAgaGlzdG9yeVtjbWRdKHMs IHMsICIjIiArIHMpOwp9Cm9ucG9wc3RhdGUgPSBmdW5jdGlvbihlKSB7CiAgbG9nKCJwb3BzdGF0 ZTogc3RhdGU9IiArIGUuc3RhdGUpOwogIG9ucG9wc3RhdGUgPSBudWxsOwogIGNoYW5nZUhpc3Rv cnkoInB1c2hTdGF0ZSIsICJ0aGlyZCIpOwogIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xs ZXIpCiAgICBsYXlvdXRUZXN0Q29udHJvbGxlci5ub3RpZnlEb25lKCk7Cn0Kb25sb2FkID0gZnVu Y3Rpb24oZSkgewogIGxvZygibG9hZCIpOwogIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xs ZXIpIHsKICAgIGxheW91dFRlc3RDb250cm9sbGVyLmNsZWFyQmFja0ZvcndhcmRMaXN0KCk7CiAg ICBsYXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7CiAgICBsYXlvdXRUZXN0Q29udHJv bGxlci53YWl0VW50aWxEb25lKCk7CiAgfQogIGNoYW5nZUhpc3RvcnkoInJlcGxhY2VTdGF0ZSIs ICJmaXJzdCIpOwogIGNoYW5nZUhpc3RvcnkoInB1c2hTdGF0ZSIsICJzZWNvbmQiKTsKICBoaXN0 b3J5LmJhY2soKTsKfQo8L3NjcmlwdD4KPGJvZHk+PHByZSBpZD0ibG9nIj48L3ByZT48L2JvZHk+ Cg== 47089 2010-01-20 18:19:38 -0800 2010-01-20 18:20:52 -0800 Patch patch.txt text/plain 3626 beidson SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MzU4OSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDEtMjAgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFz c2VydGlvbiBmYWlsdXJlIGNhbGxpbmcgaGlzdG9yeS5wdXNoU3RhdGUgd2l0aGluIHBvcHN0YXRl IGV2ZW50IGhhbmRsZXIuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0zMzgzMAorCisgICAgICAgIFRlc3Q6IGZhc3QvbG9hZGVyL3N0YXRlb2JqZWN0cy9w dXNoc3RhdGUtd2l0aGluLXBvcHN0YXRlLWhhbmRsZXItYXNzZXJ0Lmh0bWwKKworICAgICAgICAq IGxvYWRlci9GcmFtZUxvYWRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZUxvYWRlcjo6 bmF2aWdhdGVXaXRoaW5Eb2N1bWVudCk6IFJlbW92ZSB0aGUgQVNTRVJUIGFuZCBpbnZhbGlkIHBh cnQgb2YgdGhlIGNvbW1lbnQuCisKIDIwMTAtMDEtMjAgIE5pa29sYXMgWmltbWVybWFubiAgPG56 aW1tZXJtYW5uQHJpbS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVyIEh1bnQuCklu ZGV4OiBXZWJDb3JlL2xvYWRlci9GcmFtZUxvYWRlci5jcHAKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29y ZS9sb2FkZXIvRnJhbWVMb2FkZXIuY3BwCShyZXZpc2lvbiA1MzU3NCkKKysrIFdlYkNvcmUvbG9h ZGVyL0ZyYW1lTG9hZGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzcwMCw5ICszNzAwLDYgQEAg dm9pZCBGcmFtZUxvYWRlcjo6bmF2aWdhdGVXaXRoaW5Eb2N1bWVudAogICAgIGxvYWRJblNhbWVE b2N1bWVudChpdGVtLT51cmwoKSwgaXRlbS0+c3RhdGVPYmplY3QoKSwgZmFsc2UpOwogCiAgICAg Ly8gUmVzdG9yZSB1c2VyIHZpZXcgc3RhdGUgZnJvbSB0aGUgY3VycmVudCBoaXN0b3J5IGl0ZW0g aGVyZSBzaW5jZSB3ZSBkb24ndCBkbyBhIG5vcm1hbCBsb2FkLgotICAgIC8vIEV2ZW4gdGhvdWdo IHdlIGp1c3QgbWFudWFsbHkgc2V0IHRoZSBjdXJyZW50IGhpc3RvcnkgaXRlbSwgdGhpcyBBU1NF UlQgdmVyaWZpZXMgbm90aGluZyAKLSAgICAvLyBpbnNpZGUgb2YgbG9hZEluU2FtZURvY3VtZW50 KCkgY2F1c2VkIGl0IHRvIGNoYW5nZS4KLSAgICBBU1NFUlQoaGlzdG9yeSgpLT5jdXJyZW50SXRl bSgpID09IGl0ZW0pOwogICAgIGhpc3RvcnkoKS0+cmVzdG9yZVNjcm9sbFBvc2l0aW9uQW5kVmll d1N0YXRlKCk7CiB9CiAKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDUzNTY3KQorKysgTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTAtMDEtMjAgIEJy YWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIENyYXNoIGluIFBhZ2U6OmJhY2tGb3J3YXJkTGlzdCB3 aGVuIHVzaW5nIEhpc3Rvcnkgb2JqZWN0IGZyb20gYSBkZXRhY2hlZCB3aW5kb3cKKyAgICAgICAg PHJkYXI6Ly9wcm9ibGVtLzc1NTYyNTI+IGFuZCBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MzM4MjgKKworICAgICAgICAqIGZhc3QvbG9hZGVyL3N0YXRlb2JqZWN0cy9z dGF0ZS1hcGktb24tZGV0YWNoZWQtZnJhbWUtY3Jhc2gtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAg ICAgICAgKiBmYXN0L2xvYWRlci9zdGF0ZW9iamVjdHMvc3RhdGUtYXBpLW9uLWRldGFjaGVkLWZy YW1lLWNyYXNoLmh0bWw6IEFkZGVkLgorCiAyMDEwLTAxLTIwICBKb24gSG9uZXljdXR0ICA8amhv bmV5Y3V0dEBhcHBsZS5jb20+CiAKICAgICAgICAgTVNBQTogYWNjU2VsZWN0KCkgaXMgbm90IGlt cGxlbWVudGVkCkluZGV4OiBMYXlvdXRUZXN0cy9mYXN0L2xvYWRlci9zdGF0ZW9iamVjdHMvcHVz aHN0YXRlLXdpdGhpbi1wb3BzdGF0ZS1oYW5kbGVyLWFzc2VydC1leHBlY3RlZC50eHQKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9sb2FkZXIvc3RhdGVvYmplY3RzL3B1c2hzdGF0ZS13 aXRoaW4tcG9wc3RhdGUtaGFuZGxlci1hc3NlcnQtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQor KysgTGF5b3V0VGVzdHMvZmFzdC9sb2FkZXIvc3RhdGVvYmplY3RzL3B1c2hzdGF0ZS13aXRoaW4t cG9wc3RhdGUtaGFuZGxlci1hc3NlcnQtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQpAQCAtMCww ICsxIEBACitUaGlzIHRlc3QgY2hlY2tzIHRvIG1ha2Ugc3VyZSB0aGF0IGNhbGxpbmcgcHVzaFN0 YXRlKCkgZnJvbSB3aXRoaW4gYSBwb3BzdGF0ZSBldmVudCBoYW5kbGVyIGRvZXNuJ3QgQVNTRVJU IG9yIGNyYXNoLgpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9sb2FkZXIvc3RhdGVvYmplY3RzL3B1 c2hzdGF0ZS13aXRoaW4tcG9wc3RhdGUtaGFuZGxlci1hc3NlcnQuaHRtbAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9mYXN0L2xvYWRlci9zdGF0ZW9iamVjdHMvcHVzaHN0YXRlLXdpdGhpbi1w b3BzdGF0ZS1oYW5kbGVyLWFzc2VydC5odG1sCShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMv ZmFzdC9sb2FkZXIvc3RhdGVvYmplY3RzL3B1c2hzdGF0ZS13aXRoaW4tcG9wc3RhdGUtaGFuZGxl ci1hc3NlcnQuaHRtbAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwyMyBAQAorPHNjcmlwdD4KKwor aWYgKHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikgeworICBsYXlvdXRUZXN0Q29udHJvbGxl ci5jbGVhckJhY2tGb3J3YXJkTGlzdCgpOworICBsYXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNU ZXh0KCk7CisgIGxheW91dFRlc3RDb250cm9sbGVyLndhaXRVbnRpbERvbmUoKTsKK30KKworb25w b3BzdGF0ZSA9IGZ1bmN0aW9uKCkgeworICAgIGhpc3RvcnkucHVzaFN0YXRlKG51bGwsIG51bGwp OworICAgIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIpCisgICAgICAgIGxheW91dFRl c3RDb250cm9sbGVyLm5vdGlmeURvbmUoKTsKK30KKworb25sb2FkID0gZnVuY3Rpb24oKSB7Cisg ICAgaGlzdG9yeS5wdXNoU3RhdGUobnVsbCwgbnVsbCk7CisgICAgaGlzdG9yeS5iYWNrKCk7Cit9 CisKKzwvc2NyaXB0PgorPGJvZHk+CitUaGlzIHRlc3QgY2hlY2tzIHRvIG1ha2Ugc3VyZSB0aGF0 IGNhbGxpbmcgcHVzaFN0YXRlKCkgZnJvbSB3aXRoaW4gYSBwb3BzdGF0ZSBldmVudCBoYW5kbGVy IGRvZXNuJ3QgQVNTRVJUIG9yIGNyYXNoLgorPC9ib2R5Pgo=