Since session history will move into the parent process, this bug will be held and wait for bug 1438272 and bug 1445459.

Rename the bug as "Convert SessionHistoryListener to C++".
This bug will be served as the stage III of ContentSessionStore rewriting.

This work should happen on the ash branch, where we've moved session history into the parent. The listeners are already called in the parent process on that branch, so the session store's listener should just be registered in the parent process too.

Looking at the existing code, we get the session history from a docshell and then register the listener. I think what needs to happen is instead to send that docshell's BrowsingContext over IPC to the parent process. In the parent process you'll then have a CanonicalBrowsingContext (might need to use CanonicalBrowsingContext::Cast to get the right type). From the CanonicalBrowsingContext you can get the session history object (CanonicalBrowsingContext::GetSessionHistory), and register the listener using that.

You have access to the entries in the parent process, so it should be straightforward to convert the code that gathers data from the entries. There are some places where we get data from the docshell, in the parent process we should get it from the CanonicalBrowsingContext instead. Please talk to nika or farre if anything is missing on CanonicalBrowsingContext that you need access to.

• Move StateChangeNotifier/SessionHistoryListener into SessionHistoryListener.jsm
• Implement sHistoryListener
  = (in C++) collect shistory when get "DOMTitleChanged" event, OnDocumentStart and OnDocumentEnd.
  = (in JS)
  1. AddSHistoryListener in parent process.
     (send "SessionStore:AddSHistoryListener" from child to parent)
  2. Store sHistoryListener into a new WeakMap()
     ("this._browserSHistoryListener" : Use browser.permanentKey as the key)
  3. Add uninstall() to remove the listener from BC
  4. Add notifySHistoryChanges() to notify SessionStoreListener about the changes
• Add collectFromParent() in SessionHistory.jsm

// Don't try to restore framesets containing wyciwyg URLs.
// Note that these may be left over from pre-wyciwyg-removal profiles.

That seems to have happened some 11-12 years ago, is the check still needed?

The collect() part is almost finished.
However we will meet crashes when getting "shEntry.layoutHistoryState" and "shEntry.BFCacheEntry".
I have no chance to try "shEntry.stateData" when running the test. (browser/components/sessionstore/test/browser_scrollPositions.js)

shEntry is from the following steps:
@SessionStore.jsm
sHistory = aBrowser.frameLoader.browsingContext.sessionHistory
@SessionHistory.jsm
let shistory = sHistory.QueryInterface(Ci.nsISHistory);
for (; entryCount < count; entryCount++) {
let shEntry = shistory.getEntryAtIndex(entryCount);
....
}

(In reply to Laurentiu Nicola from comment #5)

// Don't try to restore framesets containing wyciwyg URLs.
// Note that these may be left over from pre-wyciwyg-removal profiles.

That seems to have happened some 11-12 years ago, is the check still needed?

Thanks for your reminding.
We don't need this anymore.
In bug 1489308, we remove all the "wyciwyg" bits.

After adding the restore part, I meet crash when doing restore().

Reproduce Step:

1. Open a random page. (e.g. https://en.wikipedia.org/wiki/Wiki)
2. Trigger a restore by "Duplicate Tab".

[Call stack]
Thread 1 "firefox" received signal SIGSEGV, Segmentation fault.
mozilla::ipc::IPDLParamTraits<mozilla::dom::PSHEntryParent*>::Write (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aVar=@0x7fffffffbc50: 0xe5e5e5e5e5e5e5e5) at PSHEntryParent.cpp:3115
3115 id = aVar->Id();
(gdb) bt
#0 0x00007fffe733556d in mozilla::ipc::IPDLParamTraits<mozilla::dom::PSHEntryParent*>::Write(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::PSHEntryParent* const&) (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aVar=@0x7fffffffbc50: 0xe5e5e5e5e5e5e5e5) at PSHEntryParent.cpp:3115
#1 0x00007fffe710766f in mozilla::ipc::IPDLParamTraits<mozilla::dom::MaybeNewPSHEntry>::Write(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::MaybeNewPSHEntry const&) (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aVar=...)
at NewPSHEntry.cpp:809
#2 0x00007fffe733659a in mozilla::ipc::IPDLParamTraits<mozilla::dom::LoadSHEntryData>::Write(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::LoadSHEntryData const&) (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aVar=...)
at PSHistory.cpp:290
#3 0x00007fffe733659a in mozilla::ipc::WriteIPDLParam<mozilla::dom::LoadSHEntryData const&>(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::LoadSHEntryData const&) (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aParam=...)
at /home/alphan/src/SessionHistory/ash/obj-x86_64-pc-linux-gnu/dist/include/mozilla/ipc/IPDLParamTraits.h:59
#4 0x00007fffe73364b2 in mozilla::ipc::IPDLParamTraits<mozilla::dom::LoadSHEntryResult>::Write(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::LoadSHEntryResult const&) (aMsg=0x7fffd2b725f0, aActor=0x7fffd5bd4240, aVar=...)
at PSHistory.cpp:561
#5 0x00007fffe733b7bb in mozilla::dom::PSHistoryParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (this=0x7fffd5bd4240, msg__=..., reply__=@0x7fffffffbff8: 0x7fffd2b725f0) at PSHistoryParent.cpp:741
#6 0x00007fffe718ce4f in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (this=0x7fffd61ab000, msg__=..., reply__=<optimized out>) at PContentParent.cpp:11731
#7 0x00007fffe707308a in mozilla::ipc::MessageChannel::DispatchSyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&, IPC::Message*&) (this=0x7fffd61ab100, aProxy=0x7fffd614b6e0, aMsg=..., aReply=@0x7fffffffbff8: 0x7fffd2b725f0) at /home/alphan/src/SessionHistory/ash/ipc/glue/MessageChannel.cpp:2154
#8 0x00007fffe707206d in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (this=0x7fffd61ab100, aMsg=...) at /home/alphan/src/SessionHistory/ash/ipc/glue/MessageChannel.cpp:2105
#9 0x00007fffe7072827 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (this=0x7fffd61ab100, aTask=...) at /home/alphan/src/SessionHistory/ash/ipc/glue/MessageChannel.cpp:1954
#10 0x00007fffe7072c2e in mozilla::ipc::MessageChannel::MessageTask::Run() (this=0x7fffd0c64e20) at /home/alphan/src/SessionHistory/ash/ipc/glue/MessageChannel.cpp:1985
#11 0x00007fffe6a3ee72 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7fffe0d32200, aMayWait=<optimized out>, aResult=<optimized out>) at /home/alphan/src/SessionHistory/ash/xpcom/threads/nsThread.cpp:1225
#12 0x00007fffe6a40d48 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0xe5e5e5e5e5e5e5e5, aMayWait=<optimized out>) at /home/alphan/src/SessionHistory/ash/xpcom/threads/nsThreadUtils.cpp:486
#13 0x00007fffe707571a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7fffe0d1f500, aDelegate=0x7ffff6a6a3c0) at /home/alphan/src/Se

Update current status,

Now there are two symptoms:

1. In some test cases, I cannot get sessionHistory object in the parent process.
   If I cannot get sessionHistory object, the sessionHistory restore cannot be complete.
   [TEST] browser_467409-backslashplosion.js

2. NS_ERROR_FAILURE on "nsISHistory.reloadCurrentEntry()" in the parent process.
   [TEST] browser_586068-multi_window.js
   JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1430: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISHistory.reloadCurrentEntry]

See more details in https://phabricator.services.mozilla.com/D46281.

I found some mochitest test failures are caused by wrong behavior of doing "sessionHistory.reloadCurrentEntry()" in the parent process.
"aLoadState->SHEntry()" is lost when sending aLoadState from parent to child.
I file Bug 1596776 for this problem.

P2 because Alphan is actively working on this bug.

(In reply to Alphan Chen [:alchen] from comment #11)

I found some mochitest test failures are caused by wrong behavior of doing "sessionHistory.reloadCurrentEntry()" in the parent process.
"aLoadState->SHEntry()" is lost when sending aLoadState from parent to child.
I file Bug 1596776 for this problem.

With current patch, there are some random ipdl errors when running the whole sessionStore tests.
However, it can reduce the number of permanent failures to 4.

browser/components/sessionstore/test/browser_447951.js
browser/components/sessionstore/test/browser_500328.js ---> File Bug 1599105
browser/components/sessionstore/test/browser_scrollPositions.js ---> bug 1596776 - Synchronize layouthistorystate to parent process
browser/components/sessionstore/test/browser_swapDocShells.js

After Bug 1583614, the pref "fission.rebuild_frameloaders_on_remoteness_change" sets to true.
When running sessionStore mochitest with fission.sessionHistoryInParent, we meet failures in the following two mochitests.

browser/components/sessionstore/test/browser_closed_objects_changed_notifications_tabs.js
FAIL The expected number of notifications was received. - Got 4, expected 5

browser/components/sessionstore/test/browser_tab_label_during_restore.js
FAIL observed tab label changes - Got ["about:robots","New Tab","Gort! Klaatu barada nikto!"], expected ["about:robots","Gort! Klaatu barada nikto!"]

(In reply to Alphan Chen [:alchen] from comment #14)

After Bug 1583614, the pref "fission.rebuild_frameloaders_on_remoteness_change" sets to true.
When running sessionStore mochitest with fission.sessionHistoryInParent, we meet failures in the following two mochitests.

browser/components/sessionstore/test/browser_closed_objects_changed_notifications_tabs.js
FAIL The expected number of notifications was received. - Got 4, expected 5

In this case, reloadCurrentEntry() doesn't work as expected.

Create the document for this implementation.
You can also add the comments in the following link.
https://docs.google.com/document/d/1L_Elw0pKzBpDaeBfNDk3D1dNmDWsFSgm0EveIddlvYc/edit?usp=sharing

I meet crash when running browser_586068-window_state_override.js based on the latest codebase.
Mozilla crash reason: MOZ_RELEASE_ASSERT(!aParam->IsDiscarded()) (Cannot send discarded BrowsingContext between processes!)

Tracking for Fission Nightly (M6) because Session Restore doesn't need to block Fission dogfooding (M5).

Backed out changeset e51b4e883adc (Bug 1507287) for not having proper review

Backed out changeset 4c6b9209306d (Bug 1507287) for causing geckoview failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=293394761&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&searchStr=5a673baa108368237aedf3c44025791ec3840b6c&revision=4c6b9209306d7e1b1db070492e1e705b7a2b4d0d

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293394761&repo=autoland&lineNumber=7311

Backout: https://hg.mozilla.org/integration/autoland/rev/9ec693701ab01fb22ba14289d5342fcd8dac17f5

I will check this failure on geckoview.

Only the changes in SessionHistory.jsm have effects on geckoview.

So I trigger a try only contain these changes. From it, we can find the same failure.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bdc0b7defbb0faf525ffc8411da306bce6e6fe6

Then I divided the patch into two parts.
(rewrite restore only) Do Find the same failure.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf26e8f85428d1290f614b9f08fc358e5c132add
(rewrite collect only) Don't find the failure as before.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1836e0697f881e297770c5369d9578aa1000bdc9
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e48722e45be25dcdf85f335abac4aef0da9e44d1

Right now I don't understand why the changes of restore rewriting would cause the problem on gv-junit test.

org.mozilla.geckoview.test.NavigationDelegateTest.processSwitching | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms

I will keep investigating the symptom today

I found out the root cause of this failure.
Only geckoview uses the return value of SessionHistory.restore().
That's why I don't see any failure on the tests of firefox desktop.
Now I am re-running a full test.

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a341b3aef1c6c41f83ff4219eae2b3611040ff60

https://hg.mozilla.org/mozilla-central/rev/e90c2551e6d0

It is possible that this has caused us to save data to disk more often?
I'm seeing disk write spikes coming from firefox process every 15 seconds even if I don't do anything with Firefox.

No, I only change the behavior when pref(fission.sessionHistoryInParent) is on.
If pref is off, it works the same as before.

Update document.

Can add comments here.
https://docs.google.com/document/d/1vlTDfcqmLFqiOP3UNoW1HG-EE5TBSt3XXNE2-HtiAw0/edit?usp=sharing

(In reply to Alphan Chen [:alchen] from comment #35)

Created attachment 9137110 [details]
Document for new sessionHistory listener v1.1.pdf

Update document.

Can add comments here.
https://docs.google.com/document/d/1vlTDfcqmLFqiOP3UNoW1HG-EE5TBSt3XXNE2-HtiAw0/edit?usp=sharing

This is great! I think we can also consider put contents into https://firefox-source-docs.mozilla.org/dom