1646212 - OOP iframe navigations that affect session history don't update the browser back/forward buttons

Status: RESOLVED WORKSFORME

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

Description

[Matt Woodrow (:mattwoodrow)]

Some iframe navigations are recorded in the top-level session history, and affect whether we can go back/forward.

This is normally handled by the nsIWebProgress::OnLocationMessage (for the iframe) bubbling up to the root docshell, where we send it to BrowserChild::OnLocationChange.

BrowserChild reads the latest can go back/forward values from the root docshell, and sends them along with the OnLocationChange message to BrowserParent.

BrowserParent writes the new can go back/forward values using UpdateWebNavigationForLocationChange, and then notifies OnLocationChange, which is what actually updates the button state.

This doesn't work for an OOP iframe, since we don't send remote web progress events in this case. The root docshell is in a different process, and I'm not sure we're updating the can go forward/back values correctly anyway.

We probably need to fix this to work fairly differently for session history in the parent, and hopefully not rely on the browser receiving OnLocationChange for a subframe in order to update the buttons (maybe a custom session history updated event?)

As far as I can tell, this is the only usage of subframe nsIWebProgress events in the frontend, so getting rid of this would be nice.

docshell/test/browser/browser_backforward_userinteraction.js tests this behaviour, but doesn't cover the fission case.

Comment 1

[Neha Kochar [:neha]]

This is nightly blocking for Fission.
Neil, can you discuss with peterv about how to handle this with history in parent?

Comment 2

[Neha Kochar [:neha]]

Olli, can you please look into this?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

At least basic OOPIF tests seem to work here. Back button is updated and going back does what it should.

Comment 4

[Neha Kochar [:neha]]

Reopening this because session history in parent pref isn't yet enabled with Fission. Olli verified that this is fixed with that pref so we can close this once the pref is enabled for Fission.

Comment 5

[Neha Kochar [:neha]]

Cleaning up the M6b list, closing the bugs that have been confirmed to be fixed with sessionHistoryInParent pref with Fission, as that's definitely getting fixed in M6b in bug 1656208. Closing per Olli's confirmation in comment 3.

Comment 6

[Johann Hofmann [:johannh]]

Hmm is this the same as bug 1670933? Then it's probably not fixed yet?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

The testcase I was using was working.
bug 1670933 is probably doing something else.