WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
44217
Assertion failure when going back inside frame during onload
https://bugs.webkit.org/show_bug.cgi?id=44217
Summary
Assertion failure when going back inside frame during onload
Mihai Parparita
Reported
2010-08-18 16:53:43 PDT
While working on
bug 44183
, I ran into another assert with navigation inside iframes.
http://persistent.info/webkit/test-cases/back-button-crash/history-back-within-subframe-url.html
triggers this assertion: ASSERTION FAILED: !m_previousItem (/Users/mihaip/Developer/source/chromium1/src/third_party/WebKit/WebCore/loader/HistoryController.cpp:567 void WebCore::HistoryController::recursiveGoToItem(WebCore::HistoryItem*, WebCore::HistoryItem*, WebCore::FrameLoadType)) Stack trace: WebCore::HistoryController::recursiveGoToItem (this=0x10702d5c8, item=0x10615bc00, fromItem=0x106193c40, type=WebCore::FrameLoadTypeIndexedBackForward) at /WebCore/loader/HistoryController.cpp:567 WebCore::HistoryController::goToItem (this=0x10702d5c8, targetItem=0x10615bc00, type=WebCore::FrameLoadTypeIndexedBackForward) at /WebCore/loader/HistoryController.cpp:238 WebCore::RedirectScheduler::scheduleHistoryNavigation (this=0x10702d7f0, steps=-1) at /WebCore/loader/RedirectScheduler.cpp:349 WebCore::History::back (this=0x10618dee0) at /WebCore/page/History.cpp:66 WebCore::jsHistoryPrototypeFunctionBack (exec=0x11c80c0c0) at /WebKitBuild/Debug/DerivedSources/WebCore/JSHistory.cpp:145 ?? () JSC::JITCode::execute (this=0x1061ab958, registerFile=0x10615e1e8, callFrame=0x11c80c040, globalData=0x106878600, exception=0x106879f48) at JITCode.h:77 This happens when calling parent.onFrameLoaded() once the inner frame navigates, which in turn calls history.back(). The triggering conditions are: - The history.back() call is in the parent, not in the iframe - The call happens immediately, not in a 0 interval timeout
Attachments
Patch
(9.31 KB, patch)
2010-08-19 16:47 PDT
,
Mihai Parparita
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Mihai Parparita
Comment 1
2010-08-18 17:10:53 PDT
+Brady since this only happens in Safari/DRT, not in Chromium/TestShell, presumably because HISTORY_ALWAYS_ASYNC is set to 1 in the latter.
Mihai Parparita
Comment 2
2010-08-18 18:41:50 PDT
As best as I can tell, this is the ordering of what's happening when history.back() is synchronous: FrameLoader::checkCompleted() checkCallImplicitClose() Document::implicitClose dispatchWindowLoadEvent (skipping over JS bindings/JIT): History::back RedirectScheduler::scheduleHistoryNavigation HistoryController::goToItem HistoryController::recursiveGoToItem check that m_previousItem is cleared <-- checkLoadComplete() recursiveCheckLoadComplete() calls checkLoadCompleteForThisFrame() frameLoadCompleted() history()->updateForFrameLoadCompleted() clears m_previousItem The check indicated by the <-- happens before the actual clearing. One possible fix is for the "goToItem should be called synchronously" check in scheduleHistoryNavigation to not just look at the documentSequenceNumber of the root of the HistoryItem, but also to recursively compare all of the children frames' itemSequenceNumbers, so that we only do the call synchronously if we really are doing just a "state object traversal or a fragment traversal" (as the comment says). However, I'm not sure what the intention is with the synchronous history.back(). I spent some time reading
bug 33538
, but it was also interesting to see that all the fast/history/stateobjects layout tests still pass even with asynchronous history.back(). Was there a reason why the original test case in the bug was never landed?
Mihai Parparita
Comment 3
2010-08-19 16:47:37 PDT
Created
attachment 64910
[details]
Patch
Mihai Parparita
Comment 4
2010-08-19 16:51:45 PDT
The attached patch fixes this assert by (I believe) staying close to the sprit of the "navigation should be synchronous" check in RedirectScheduler::scheduleHistoryNavigation by also having it check the document sequence numbers. The test cases no longer crashes. Brady, do you have time to review this?
WebKit Commit Bot
Comment 5
2010-08-19 20:57:37 PDT
Comment on
attachment 64910
[details]
Patch Clearing flags on attachment: 64910 Committed
r65724
: <
http://trac.webkit.org/changeset/65724
>
WebKit Commit Bot
Comment 6
2010-08-19 20:57:42 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug