Closed Bug 1596776 Opened 8 months ago Closed 7 months ago

nsDocShellLoadState(DocShellLoadStateInit&) doesn't set mSHEntry

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla73
Fission Milestone M5
Tracking Status
firefox73 --- fixed

People

(Reporter: alchen, Assigned: annyG)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

https://searchfox.org/mozilla-central/rev/cac340f10816707e91c46db6d285f80259420f07/dom/ipc/DOMTypes.ipdlh#256
We don't support mSHEntry now.
It also needs to be serialized in nsDocShellLoadState::Serialize.

Blocks: 1507287

With bug 1596077, we use "BrowsingContext->LoadURI()" in nsSHistory.cpp to prevent docShell is null case.

In this case, we use "Canonical()->GetCurrentWindowGlobal()->SendLoadURIInChild(aLoadState, aSetNavigating);" in "BrowsingContext->LoadURI()"
https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/docshell/base/BrowsingContext.cpp#869

Because we don't set mSHEntry into DocShellLoadStateInit, we lost it when arriving WindowGlobalChild::RecvLoadURIInChild().
Then we cannot complete sessionHistory.reloadCurrentEntry() when using it from the parent process.

Depends on: 1596077

I think this is part of the implementation of sessionHistory in parent(Bug 1438272).
Bug 1539482 is also related to nsISHEntry in IPC call.

Hi Peter and Anny,
could you take a look on this?

Flags: needinfo?(peterv)
Flags: needinfo?(agakhokidze)

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

I think this is part of the implementation of sessionHistory in parent(Bug 1438272).
Bug 1539482 is also related to nsISHEntry in IPC call.

The problem is not caused by Bug 1539482.
This call flow comes from sessionHistory.reloadCurrentEntry().
There is no this kind of call flow before enabling session history in the parent process.
At that time, we always call sessionHistory.reloadCurrentEntry() from the child process.

Some additional notes as well on other things that need to be fixed - when I created CreateLoadInfo() here https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/docshell/shistory/nsISHEntry.idl#422, we decided to set the shentry on the nsDocShellLoadState in SHEntryChild::CreateLoadInfo instead, when session history in parent in enabled, to avoid (de)serializing the entry. With session history in parent disabled, this path https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/docshell/shistory/SHEntryChild.cpp#991-993 is never invoked and thus we never set it. I should fix this as well.

Assignee: nobody → agakhokidze
Status: NEW → ASSIGNED
Flags: needinfo?(agakhokidze)

STR:

  1. apply the patch(https://phabricator.services.mozilla.com/D46281)
  2. Run browser/components/sessionstore/test/browser_formdata.js
  3. Check logs and result. Now we have two failures for this test.

[Current log]
0:06.34 GECKO(9588) 870@BrowsingContext::LoadURI aLoadResult.SHEntry() exists
...
0:06.34 GECKO(9588) ----DEBUG--------------- nsDocShell::LoadURI:795[0x7fbbf8050800]: InternalLoad() without aLoadState->SHEntry()
...
0:06.63 FAIL form data not restored - Got value-0.6022411674343265, expected

[Expected log]
0:06.34 GECKO(9588) 870@BrowsingContext::LoadURI aLoadResult.SHEntry() exists
...
0:06.34 GECKO(9588) ----DEBUG--------------- nsDocShell::LoadURI:763[0x7fbbf8050800]:loading from session history
...
0:06.63 PASS form data not restored

Flags: needinfo?(peterv)
Fission Milestone: --- → M5
Priority: -- → P2
Attached file Nov-25-mochitest_1.log

By applying this patch, sometimes I found the following logs when doing the test.
GECKO(30539) IPDL protocol error: could not lookup id for PSHEntry
GECKO(30539) IPDL protocol error: Error deserializing MaybeNewPSHEntry
It happened randomly when testing the whole folder. (./mach test browser/components/sessionstore/test/)
But it won't happen if I only run a single test.

[From Nov-25-mochitest_1.log]
0:14.94 GECKO(5654) aLoadResult.SHEntry() exists
0:14.94 GECKO(5654) 870@BrowsingContext::LoadURI aLoadResult.SHEntry() exists
0:14.94 GECKO(5654) BrowsingContext::LoadURI mDocShell not exists, XRE_IsParentProcess()=1
0:14.94 GECKO(5654) ====DEBUG==== @ SessionStore.jsm receiveMessage(SessionStore:restoreTabContentStarted), data={"epoch":1,"isRemotenessUpdate":false,"reason":0}!!!
0:14.94 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
0:14.94 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry

2:43.25 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
2:43.25 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry

4:13.31 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
4:13.31 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry
4:13.31 GECKO(5654) ----DEBUG--------------- nsDocShell::LoadURI:795[0x7f1d80255800]: InternalLoad() without aLoadState->SHEntry()

4:59.11 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
4:59.11 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry

6:28.52 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
6:28.52 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry

7:17.87 GECKO(5654) IPDL protocol error: could not lookup id for PSHEntry
7:17.87 GECKO(5654) IPDL protocol error: Error deserializing MaybeNewPSHEntry
7:18.09 GECKO(5654) ----DEBUG--------------- nsDocShell::LoadURI:795[0x7f6c8d6ac800]: InternalLoad() without aLoadState->SHEntry()

Blocks: 1599105
Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fde732c4e940
Serialize SHEntry when sending DocShellLoadStateInit over IPC, r=peterv
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
You need to log in before you can comment on or make changes to this bug.