Closed Bug 1670080 Opened 4 years ago Closed 4 years ago

Session history in parent shouldn't add entries for URLs that HTTP redirect

Categories

(Core :: DOM: Navigation, defect, P2)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1668083
Fission Milestone M6c

People

(Reporter: u608768, Unassigned)

References

(Blocks 1 open bug)

Details

This is causing browser/components/sessionstore/test/browser_restore_redirect.js to fail with

INFO - TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_restore_redirect.js | Should be the right session history entry - Got "http://example.com/browser/browser/components/sessionstore/test/restore_redirect_http.html", expected "http://example.com/browser/browser/components/sessionstore/test/restore_redirect_target.html"

I've found it's easier to see what's going on with the following:

  1. Open a non-google.com tab.

  2. Run the following in the Browser Toolbox (gmail.com is known to 302).

    SessionStore.setTabState(gBrowser.selectedTab, {entries: [{url: "https://gmail.com", triggeringPrincipal_base64: E10SUtils.SERIALIZED_SYSTEMPRINCIPAL}]})
    
  3. Check the history dropdown. You should see that we've added an entry for all URLs that were part of the load (gmail.com -> google.com/mail -> mail.google.com/mail -> https://mail.google.com/mail/u/0/#inbox in my case). With fission/ship disabled, we only add an entry for the final URL.

I've dug into this a little bit. By the time we've seen the HTTP redirect (and redirected the load to the real channel), we have the correct "target" URL in DocumentChannelChild::RecvRedirectToRealChannel, but the entry that we end up saving to session history (in CanonicalBrowsingContext::SessionHistoryCommit) is for the document that asked for the redirect (restore_redirect_http.html in the test). So I think we're either adding the pending load entry to CanonicalBrowsingContext::mLoadingEntries too early, or we need to add another entry for the "target" load, after we're made aware of the redirect, and have content send the load ID for that instead (this happens in nsDocShell::MoveLoadingToActiveEntry).

Kashav, please try this again with Olli's patch from bug 1668083.

This is fixed by the patches from bug 1668083.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.