Closed Bug 1574261 Opened 2 years ago Closed 8 months ago

Make nsDocShell::MaybeHandleSubFrameHistory work with session history in parent

Categories

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

defect

Tracking

()

RESOLVED FIXED
82 Branch
Fission Milestone M6b
Tracking Status
firefox82 --- fixed

People

(Reporter: djvj, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Whiteboard: patch in review [rm-docshell-tree-item:session-history])

Attachments

(1 file)

https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#782

In this function, the same-type parent of the current document is obtained. If the parent is null or the same as this, then the this docshell is treated as the root.

This logic is erroneous in a post-fission semantics. The parent may be null if the parent document is out-of-process. To handle this, the logic around retrieving a parent should be changed to use the BrowsingContext to check for a parent.

Furthermore, the function goes on to retrieve the nsDocShell associated with the parent document, and perform various operations on it. The main job of the function is to adjust the session-history state of the child based on the state of the parent document.

This logic will need to be changed to use IPC to retrieve this information from a remote process (if needed). For performance reasons, it is probably a good idea to detect the situation where the parent is in process, and use the existing logic for that.

Component: DOM: Core & HTML → Document Navigation
Fission Milestone: --- → M5
Priority: -- → P3
Priority: P3 → P2
Whiteboard: [rm-docshell-tree-item:session-history]
Fission Milestone: M5 → Future

Kannan says replacing nsIDocShellTreeItem calls should block enabling Fission in Nightly (M6).

Fission Milestone: Future → M6

Auditing whether this use of nsIDocShellTreeItem breaks when Fission is enabled blocks Fission Nightly.

Fission Milestone: M6 → M6b
Fission Milestone: M6b → Future

Discussed in history meeting and we need to audit the usage and then decide whether or not it will block nightly.

Fission Milestone: Future → M6b
Flags: needinfo?(bugs)
Assignee: nobody → bugs
Status: NEW → ASSIGNED
Type: enhancement → defect
Flags: needinfo?(bugs)
Summary: Fix usage of nsIDocShellTreeItem in nsDocShell::MaybeHandleSubFrameHistory → Make nsDocShell::MaybeHandleSubFrameHistory work with session history in parent

This has couple of different pieces and one may want to focus on each of those separately when
reviewing. The first two as small changes.

  • Moving mDynamicallyCreated from nsDocShell to be a sync'ed field on BrowsingContext.
    CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad sets that on a newly created entry.
  • Adding mLoadingActiveEntry. mLoadingEntry + mLoadingActiveEntry has roughly the same lifetime as
    mLSHE. mLoadingActiveEntry is needed so that child docshell can know whether its parent is loading
    from session history.
  • The main part is in MaybeHandleSubframeHistory which checks if the parent docshell is loading from
    session history, and if so, asks for a LoadingSessionHistoryInfo.
    In the case of docshell living in a child process that operation is asynchronous, so when
    the data is back from the parent process, LoadURI is called again with the possibly
    updated data. One could possibly split the code to smaller methods and then deal
    with aContinueHandlingSubframeHistory only in LoadURI, but MaybeHandleSubframeHistory
    does have some early returns which would make that approach possibly hard to follow.
Whiteboard: [rm-docshell-tree-item:session-history] → patch in review [rm-docshell-tree-item:session-history]
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/db7ab542d0ec
Make nsDocShell::MaybeHandleSubFrameHistory work with session history in parent, r=peterv
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Depends on: 1666010
You need to log in before you can comment on or make changes to this bug.