Closed Bug 1546759 Opened 3 years ago Closed 2 years ago

Change nsSHistory::WalkHistoryEntries to walk browsing context tree instead of doc shell tree

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla76
Fission Milestone M6
Tracking Status
firefox76 --- fixed

People

(Reporter: annyG, Assigned: annyG)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 10 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

To accomodate recent session history changes, we need to change nsSHistory::WalkHistoryEntries to walk browsing context tree instead of doc shell tree

Blocks: 1546762
Fission Milestone: --- → M3
Priority: -- → P3
Fission Milestone: M3 → M4

Depends on D42994

Depends on D42995

Blocks: 1579093
No longer blocks: 1579093
Attachment #9087263 - Attachment description: Bug 1546759 - Part 2: Add mOSHE and mLSHE to BrowsingContext and update them appropriately → Bug 1546759 - Part 2: Add mOSHE and mLSHE to BrowsingContext and update them appropriately,
Attachment #9087264 - Attachment description: Bug 1546759 - Part 3: Add an IPC call from parent to child to swap history entries in docshell → Bug 1546759 - Part 3: Add an IPC call from parent to child in PContent.ipdl to swap history entries in docshell,
Attachment #9087265 - Attachment description: Bug 1546759 - Part 4: Add a method for swapping history entries to Browsing Context → Bug 1546759 - Part 4: Add a method for swapping history entries to Browsing Context,
Attachment #9087266 - Attachment description: Bug 1546759 - Part 5: Move SetChildHistoryEntry to parent process → Bug 1546759 - Part 5: Move SetChildHistoryEntry to parent process,
Attachment #9087267 - Attachment description: Bug 1546759 - Part 6: Move CloneAndReplaceChild to parent process → Bug 1546759 - Part 6: Move CloneAndReplaceChild to parent process,
Attachment #9087268 - Attachment description: Bug 1546759 - Part 7.1 update nsDocShell::SetHistoryEntry to call ::Top() → Bug 1546759 - Part 7.1 Update nsDocShell::SetHistoryEntry to call ::Top(),
Attachment #9087269 - Attachment description: Bug 1546759 - Part 7.2 Consolidate calls inside of nsDocShell::SetHistoryEntry → Bug 1546759 - Part 7.2 Consolidate calls inside of nsDocShell::SetHistoryEntry,
Depends on: 1555964
Fission Milestone: M4 → M5
Priority: P3 → P2
Attachment #9087270 - Attachment description: Bug 1546759 - Part 8: Consolidate several IPC calls such that CloneAndReplace is called from parent again → Bug 1546759 - Part 8: Consolidate several IPC calls such that CloneAndReplace is called from parent again,
Depends on: 1581970
Depends on: 1583321
No longer depends on: 1581970
See Also: → 1581970
No longer depends on: 1555964
Attachment #9087271 - Attachment description: Bug 1546759 - Part 9: Make SetChildHistoryEntry and CloneAndReplaceChild fission compatible → Bug 1546759 - Part 9: Make SetChildHistoryEntry and CloneAndReplaceChild fission compatible,
Blocks: 1590521

TODO currently coming up with a great and extended commit message

Deferring to Fission Nightly (M6) because Anny says this bug doesn't need to block dogfooding (M5).

Fission Milestone: M5 → M6
Attachment #9087271 - Attachment is obsolete: true
Attachment #9087270 - Attachment is obsolete: true
Attachment #9087269 - Attachment is obsolete: true
Attachment #9087268 - Attachment is obsolete: true
Attachment #9087267 - Attachment is obsolete: true
Attachment #9087266 - Attachment is obsolete: true
Attachment #9087265 - Attachment is obsolete: true
Attachment #9087264 - Attachment is obsolete: true
Attachment #9087263 - Attachment is obsolete: true
Attachment #9087262 - Attachment is obsolete: true
Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ebc7618a5fa
Change nsSHistory::WalkHistoryEntries to walk browsing context tree instead of doc shell tree, r=peterv,nika
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Regressions: 1621748

There are 8 crashes with signature mozilla::dom::ContentParent::RecvUpdateSHEntriesInBC.
The moz_crash_reason is MOZ_DIAGNOSTIC_ASSERT(false) (Trying to update a child BrowsingContext in another child process).

Status: RESOLVED → REOPENED
Crash Signature: [@ mozilla::dom::ContentParent::RecvUpdateSHEntriesInBC]
Resolution: FIXED → ---

I think we are hitting this assertion because the BC we are trying to update is discarded thus is no longer owned by this process. Currently working on a patch.

Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/acfafba3fdcc
Do not update session history entries in BC when it is discarded, r=peterv

Calixte, I was wondering why this bug was re-opened? When a patch causes regressions don't we usually just file new bugs and mark them as regressions of this bug? I am not an expert on how this works so I am just wondering.

Flags: needinfo?(cdenizet)
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

:annyG, the crash reason is an assert you purposefully added to help you to fix the original bug so no need to file a new bug.

Flags: needinfo?(cdenizet)

I understand :) Thank you!

We're still seeing some ongoing crashes with this signature. Is that being tracked anywhere?

Flags: needinfo?(agakhokidze)
Regressions: 1624012

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)

We're still seeing some ongoing crashes with this signature. Is that being tracked anywhere?

I just filed bug 1624012 to track these crashes.

Crash Signature: [@ mozilla::dom::ContentParent::RecvUpdateSHEntriesInBC]
Flags: needinfo?(agakhokidze)
You need to log in before you can comment on or make changes to this bug.