300802 - Doing a LOAD_NORMAL or LOAD_LINK of the same URI may confuse fastback/bfcache

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The fastback CanSavePresentation code has:

4825     // Only save presentation for "normal" loads and link loads.  Anything else
4826     // probably wants to refetch the page, so caching the old presentation
4827     // would be incorrect.
4828     if (aLoadType != LOAD_NORMAL &&
4829         aLoadType != LOAD_HISTORY &&
4830         aLoadType != LOAD_LINK)
4831         return PR_FALSE;

During a pageload, the last time this is called is when we hit
nsDocShell::CreateContentViewer.  _After_ this is called, we call OnLoadingSite,
which calls OnNewURI, which has the following code:

7058     if (equalUri &&
7059         (mLoadType == LOAD_NORMAL ||
7060          mLoadType == LOAD_LINK) &&
7061         !inputStream)
7062     {
7063         mLoadType = LOAD_NORMAL_REPLACE;
7064     }

After this we don't create a new SHEntry, etc, but we probably still save the
old viewer in the SHEntry, which could cause problems...

Comment 1

[Brian Ryner (not reading)]

We actually do create a new history entry in this case, it just replaces the old
one in the history list.  I think that's what the comment means by "won't mess
with the current SHEntry".  So, I don't think anything _really_ bad will happen
here.  We could add a matching Equals() check to avoid the work of caching the
presentation though, if we're just going to throw it away.

(The only case I can find where we reuse the old entry is with LOAD_REFRESH).

Comment 2

[Brian Ryner (not reading)]

Moving out, I don't think this will break anything for the next release.

Comment 3

[Brian Ryner (not reading)]

Reassigning my bugs, since I'm not actually working on them.