1647109 - can't navigate away from file:// URL

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Cameron McCormack (:heycam)]

Since enabling Fission, I ran into a problem today with navigation. In this one tab, I have a file:// URL page open. If I type an https:// URL into the address bar and press Enter, the spinner starts, and I see the hostname in the status bar at the bottom of the window, but the navigation never succeeds. In the browser console, I see this exception:

TypeError: can't access property "legacySHistory", history is null [SessionHistory.jsm:109:1]
    collect resource://gre/modules/sessionstore/SessionHistory.jsm:109
    collect resource://gre/modules/sessionstore/SessionHistory.jsm:39
    collectFrom resource:///modules/sessionstore/ContentSessionStore.jsm:200
    send resource:///modules/sessionstore/ContentSessionStore.jsm:469
    flush resource:///modules/sessionstore/ContentSessionStore.jsm:800
    receiveMessage resource:///modules/sessionstore/ContentSessionStore.jsm:616

If I try to visit a different file:// URL in that tab, it works. And navigating to an https:// URL from another tab that has a file:// URL document shown it works too, so something's gone wrong with this tab / process in particular.

Comment 1

[Alphan Chen [:alchen]]

I will check this problem today.

Comment 2

[Alphan Chen [:alchen]]

I cannot reproduce this problem with fission enabled by

1. open a "file:// URL" page
2. type "an "https:// URL" into the address bar" and press Enter

I also take a look at the implementation before my rewriting.

[In the previous implementation]
First, history = webNavigation.sessionHistory;
https://searchfox.org/mozilla-central/rev/93ec4f0381e5a30076a465aec3d18c05f3c9b1d6/toolkit/modules/sessionstore/SessionHistory.jsm#84
Here we will have a similar problem.
We just access the legacySHistory without check if history is null or not.
https://searchfox.org/mozilla-central/rev/93ec4f0381e5a30076a465aec3d18c05f3c9b1d6/toolkit/modules/sessionstore/SessionHistory.jsm#89

Comment 3

[Alphan Chen [:alchen]]

We could check if history is null before accessing property "legacySHistory".
https://searchfox.org/mozilla-central/rev/46e3b1ce2cc120a188f6940b5c6eab6b24530e4f/toolkit/modules/sessionstore/SessionHistory.jsm#109

How about "data.requestedIndex"?
What should we do if there is no legacySHistory?
https://searchfox.org/mozilla-central/rev/46e3b1ce2cc120a188f6940b5c6eab6b24530e4f/toolkit/modules/sessionstore/SessionHistory.jsm#119

We don't handle this before.
So I suppose that it is not a big problem before.
Not sure how often do we meet this case when enabling fission?
It is the first time I saw this problem.

Comment 4

[Chris Peterson [:cpeterson]]

@ heycam, can you please share which extensions you have installed? Copy/pasting the list from about:support is the easiest way to share. Nika suspects this bug might be related to container tabs.

Tracking for Fission M6a Nightly because many people are reporting this problem.

Comment 5

[Cameron McCormack (:heycam)]

Attachment: my installed addons

Comment 6

[Paul Bone [:pbone]]

While working on Bug 1647205 I hit the same exception as heycam, without fission but with preserve browsing contexts (some of the same code-paths).

I haven't tried with fission, but here's how I reproduce it:

1. New profile
2. Enable session restore
3. Open two new tabs, they appear on the new tab home page.
4. Close all other tabs.
5. Close the browser
6. Open the browser
   Repeat steps 3-6 once more (not sure how necessary this is).
7. Navigate the first one to wikipedia.com (you can click the link on the new tab page)
8. Switch to the second one, it restores now.
9. Attempt to navigate it, it doesn't navigate and you get the exception above.

file:// -> http:// is a remoteness change, so is the about:newtab -> http:// So I guess they're hitting the same code path. heycam, can you remember if session restore may have been involved in your problem?

Comment 7

[Cameron McCormack (:heycam)]

(In reply to Paul Bone [:pbone] from comment #6)

heycam, can you remember if session restore may have been involved in your problem?

I don't recall if those tabs were session restored at some point (I tend to update only once a week or two), but it's certainly possible.

Comment 8

[Paul Bone [:pbone]]

I tried my STR above with mozregression (not this is without fission) and found Bug 1550571 in that regression range also.

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fab7c4f54054ceb06504c7ddac380858e2521fc4&tochange=10ad7868f3ca27cb90db9bd1d392ff4d7852a0cd

Unless there are multiple bugs with the same symptom I'd say this is the culprit.

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to Paul Bone [:pbone] from comment #6)

1. New profile
2. Enable session restore
3. Open two new tabs, they appear on the new tab home page.
4. Close all other tabs.
5. Close the browser
6. Open the browser
   Repeat steps 3-6 once more (not sure how necessary this is).
7. Navigate the first one to wikipedia.com (you can click the link on the new tab page)
8. Switch to the second one, it restores now.
9. Attempt to navigate it, it doesn't navigate and you get the exception above.

These STR don't work for me.

Comment 10

[Paul Bone [:pbone]]

BrowsingContext::GetChildSessionHistory will only return mChildSessionHistory if it has been set as being in this process, with SetIsInProcess. This is usually set if the BrowsingContext has a docshell. But not by the CreateChildSHistory() method. This method is used by DidSet(FieldIndex<IDX_HasSessionHistory>, bool aOldValue), which I think is used to synchronise properties between BCs in different processes. I think what's happening is that some synchronisation is causing DidSet to call CreateChildSHistory() which makes a new ChildSHistory object and does not set that it is in this process. I'm not yet sure why this property is set elsewhere and then synchronised, but maybe instead CreateChildSHIstory() should just call SetIsInProcess()? I'm testing this now.

Comment 11

[Paul Bone [:pbone]]

Attachment: Bug 1647109 - Set the child history as in process when the BC is in process r=kmag

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f83cf116474
Set the child history as in process when the BC is in process r=kmag

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4f83cf116474