1545474 - Change nsSHistory::EvictContentViewerForEntry to accommodate session history changes

Status: RESOLVED FIXED

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

Comment 1

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1545474 - Part 1: Consolidate sync IPC calls inside of nsDocShell::UpdateURLAndHistory,

Inside of nsDocShell::UpdateURLAndHistory, there are 4 sync IPC calls to
nsSHistory plus 1 static call, which contains at least one nsSHEntry::GetParent
sync IPC call. All of these calls can be moved inside of a new method
EvictContentViewersOrReplaceEntry on nsSHEntry, resulting in just 1 sync IPC
call.

Comment 2

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1545474 - Part 2: Modify eviction of content viewers to accomodate session history changes,

When we need to evict content viewers, we group SHEntrySharedParentState,
corresponding to session history entries that need to be evicted, by their
content parent, and create a runnable for each content parent. In the runnable
we use the passed in IDs of SHEntrySharedParentStates to retrieve corresponding content
viewers and evict them.

Depends on D32729

Comment 3

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1545474 - Part 3: Test eviction of content viewer entries,

Add a new method to session history listener to listen for content
viewers being evicted.

Depends on D32730

Comment 4

[Anny Gakhokidze [:annyG]]

Peter, here I will explain the rationale behind adding method NotifyListenersContentViewerEvicted to SHEntryChildShared and SHEntrySharedParentState. But first, a backstory!

Whenever NotifyListenersContentViewerEvicted is defined on nsSHistory or SHistoryChild, it actually uses the NOTIFY_LISTENERS macro and informs mListeners about an event. I have added NotifyListenersContentViewerEvicted() in nsISHistory.idl because nsSHistory and SHistoryChild both have their own listeners. Then I added calls to NotifyListenersContentViewerEvicted() in nsSHistory::EvictContentViewerForEntry and SHistoryChild::NotifyListenersContentViewerEvicted because these two places evict content viewers.

There are several other places where a content viewer gets evicted

• SHEntryChildShared::EvictContentViewers()
• nsSHEntry::EvictContentViewer()
• SHEntryChild::EvictContentViewer()

We don't have access to session history from those places to call nsISHistory::NotifyListenersContentViewerEvicted, however, we do have access to mShared, the type of which is SHEntryChildShared or SHEntrySHaredParentState, both of which have local mSHistory. Thus, we want to define a method on SHEntryChildShared and SHEntrySHaredParentState that will invoke NotifyListenersContentViewerEvicted() on the session history. The solution is to define a method NotifyListenersContentViewerEvicted which will use local mSHistory and call nsISHistory::NotifyListenersContentViewerEvicted on it.

Comment 6

[Anny Gakhokidze [:annyG]]

Pushed to ash branch

https://hg.mozilla.org/projects/ash/rev/2cd5cd24763ff320719aedb2142a79822efd6de4
https://hg.mozilla.org/projects/ash/rev/0feb286c5b85e6e2a0ac56058c596324ca28d817
https://hg.mozilla.org/projects/ash/rev/5e4d0ab9e52ce1e1d1684fdb59534a66e9f9d9b9

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/351e31cced31
Part 1: Consolidate sync IPC calls inside of nsDocShell::UpdateURLAndHistory, r=peterv, r=nika for adding sync IPC messages
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b772fd4f675
Part 2: Modify eviction of content viewers to accomodate session history changes, r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/8361b46b39db
Part 3: Test eviction of content viewer entries, r=peterv

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/351e31cced31
https://hg.mozilla.org/mozilla-central/rev/3b772fd4f675
https://hg.mozilla.org/mozilla-central/rev/8361b46b39db