STEPS TO REPRODUCE: 1) Build and enable bfcache 2) Start browser 3) Load http://www.mozilla.org/ 4) Wait for load to finish 5) Click "reload" button EXPECTED RESULTS: reload quietly ACTUAL RESULTS: ###!!! ASSERTION: RestorePresentation should only be called for history loads: 'mLoadType & LOAD_CMD_HISTORY', file /home/bzbarsky/mozilla/vanilla/mozilla/docshell/base/nsDocShell.cpp, line 4952

OK, this is actually happening with bfcache disabled too. Could we get some traction on this? Why are we ending up inside RestorePresentation() for non-history loads, exactly?

To be precise, this is happening for loads with mType == LOAD_RELOAD_NORMAL. Those have a SHEntry, but are not exactly history loads in the sense we want here. That is, I don't think we should be doing fastback-like stuff for LOAD_RELOAD_NORMAL. We should probably just change this assert to a check-and-early-return, even before we look at the history entry.

be good if we could resolve in firefox 1.1. see https://bugzilla.mozilla.org/show_bug.cgi?id=295813

Don't restore saved presentation unless there is one

Doesn't this skip saving the current presentation too? That looks wrong...

Er, nevermind comment 5.

Comment on attachment 185894 [details] [diff] [review] patch to fix Seems better to just check at the call site whether (mLoadType & LOAD_CMD_HISTORY)? That's the assumption of RestorePresentation, not that the viewer is non-null.

The reason i checked the viewer was that otherwise we'd end up calling SyncPresentationState on both SHEntries. That seemed like unneccesary work? Also, it seemed like a good sanity check to assert if we have a viewer but aren't performing a history load. Though that could be added separatly.

Maybe we could only do the SyncPresentationState work for the case where NS_FAILED(rv)? The only cases where |restored| is false, but the method succeeds are: 1. no saved presentation 2. the saved content viewer has a different container In the first case here, there's no need to SyncPresentation, but in the second case, since we null out the saved content viewer, we probably do want to SyncPresentation. So maybe we do this: 1. Change case #2 above to return a failure code 2. Make the SyncPresentation work only happen if NS_FAILED(rv). 3. Wrap the whole thing with a check of mLoadType

fixed by bug 292954 checkin