The issue is that when going back from a non-bfcacheable page to a bfcacheable page and the first page
modifies requested index around the same time, UpdateIndex() doesn't have the right requested index. This is a variant of bug 1725680.
The test case loads a non-bfcacheable page, then bfcachable, then another non-bfcacheable and then goes back and forward.
Without the patch indexes go wrong and forward doesn't do anything.
Differential Revision: https://phabricator.services.mozilla.com/D127963
When we navigate in history to the same entry that we're current at then we
actually do a reload. The problem is in the way we detect whether to do a reload
in the parent process.
If a page does a back and a forward one after the other in a script, then the
parent will calculate the index for the back and tell the child to load the
entry at that index. While the child is processing the load of that entry, the
BC in the parent process still has the same entry as its active entry (until the
child commits the load of the entry over IPC). The parent then processes the
forward, calculates the index for the forward and finds the entry at that index.
This is the same entry that we were at before doing anything, and so the same
entry as the active entry in the BC in the parent process. We used to compare
the entry that we're going to load with the active entry in the BC to determine
whether we're doing a reload, and so in this situation we would assume the
forward navigation was actually doing a reload. The child would reload the page,
and we'd run the script again and we'd end up in a reload loop.
Comparing the offset with 0 to determine whether we're doing a reload fixes this
issue.
Differential Revision: https://phabricator.services.mozilla.com/D126585
When we navigate in history to the same entry that we're current at then we
actually do a reload. The problem is in the way we detect whether to do a reload
in the parent process.
If a page does a back and a forward one after the other in a script, then the
parent will calculate the index for the back and tell the child to load the
entry at that index. While the child is processing the load of that entry, the
BC in the parent process still has the same entry as its active entry (until the
child commits the load of the entry over IPC). The parent then processes the
forward, calculates the index for the forward and finds the entry at that index.
This is the same entry that we were at before doing anything, and so the same
entry as the active entry in the BC in the parent process. We used to compare
the entry that we're going to load with the active entry in the BC to determine
whether we're doing a reload, and so in this situation we would assume the
forward navigation was actually doing a reload. The child would reload the page,
and we'd run the script again and we'd end up in a reload loop.
Comparing the offset with 0 to determine whether we're doing a reload fixes this
issue.
Differential Revision: https://phabricator.services.mozilla.com/D126585
Using requestedIndex on the child side is hard, because there are race conditions when a session history load is triggered
and at the same time a non-session history load commits a new active entry.
Differential Revision: https://phabricator.services.mozilla.com/D126619
When we navigate in history to the same entry that we're current at then we
actually do a reload. The problem is in the way we detect whether to do a reload
in the parent process.
If a page does a back and a forward one after the other in a script, then the
parent will calculate the index for the back and tell the child to load the
entry at that index. While the child is processing the load of that entry, the
BC in the parent process still has the same entry as its active entry (until the
child commits the load of the entry over IPC). The parent then processes the
forward, calculates the index for the forward and finds the entry at that index.
This is the same entry that we were at before doing anything, and so the same
entry as the active entry in the BC in the parent process. We used to compare
the entry that we're going to load with the active entry in the BC to determine
whether we're doing a reload, and so in this situation we would assume the
forward navigation was actually doing a reload. The child would reload the page,
and we'd run the script again and we'd end up in a reload loop.
Comparing the offset with 0 to determine whether we're doing a reload fixes this
issue.
Differential Revision: https://phabricator.services.mozilla.com/D126585
The fix let's ContentParent::RecvSynchronizeLayoutHistoryState update the layout history state.
Using an existing test to launch a subtest for this. Hopefully the description of the test helps with reviewing it.
(These BroadcastChannel based tests can be hard to follow.)
Differential Revision: https://phabricator.services.mozilla.com/D122376
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
Ensure firing the onStateChange on the BrowsingContext's WebProgress
of the page we navigate *from* instead of the one we navigate *to*.
(which was what used to happen before fission/bfcacheInParent)
Differential Revision: https://phabricator.services.mozilla.com/D119648
I don't have a test for this, since the race condition is super hard to trigger.
The patch is based on code auditing.
The patch just prevents bfcaching if we're in middle of another load.
Differential Revision: https://phabricator.services.mozilla.com/D119992
I think there might be a case for the issue happening also without SHIP, but
since it is way less asynchronous, triggering that issue is hard.
So the patch and test are for SHIP only.
Differential Revision: https://phabricator.services.mozilla.com/D116744
nsFrameLoaderOwner::ReplaceFrameLoader calls nsFocusManager::ActivateRemoteFrameIfNeeded.
If the call to nsFrameLoaderOwner::ReplaceFrameLoader happens before the BC is marked as
not in BFCache, then the focus manager will not fire focus events because the window is
still frozen.
Differential Revision: https://phabricator.services.mozilla.com/D114227
Otherwise when navigating to a replaced browsing context we might have
an incorrect "is single top-level in history".
Fixes test_window_close.html with Fission+bfcache.
Differential Revision: https://phabricator.services.mozilla.com/D111458
The patch makes HistoryTracker rely on SHEntrySharedParentState instead of nsSHEntryShared.
nsSHEntryShared already extends SHEntrySharedParentState.
The test was modified a tiny bit to make it easier to see the results. The test does pass
with SHIP+BFCache.
Depends on D108851
Differential Revision: https://phabricator.services.mozilla.com/D108984
Note that this patch only transforms the use of the nsDataHashtable type alias
to a directly equivalent use of nsTHashMap. It does not change the specification
of the hash key type to make use of the key class deduction that nsTHashMap
allows for in some cases. That can be done in a separate step, but requires more
attention.
Differential Revision: https://phabricator.services.mozilla.com/D106008
