From 15fcfd1312a92c39ced320fc9956131c972dfbd8 Mon Sep 17 00:00:00 2001 From: Johann Hofmann Date: Wed, 18 Nov 2020 23:42:46 +0000 Subject: [PATCH] Bug 1650095 - Part 4 - For Fission, reset SHEntryHasUserInteraction cache in the parent when adding/updating SH entries. r=smaug When re-enabling the test case blocked by bug 1670933 it became apparent that we were not allowing session history entries to be marked with user interaction when the SH entry was created by navigation through a sub-frame. The code that we had for this only covered updating SH entries after pushState etc., not adding new entries for document loads. When SH lives in the child this is easier to manage in nsDocShell, but with Fission it probably makes sense to move this code to the parent. Differential Revision: https://phabricator.services.mozilla.com/D97421 --- docshell/base/CanonicalBrowsingContext.cpp | 15 +++++++++++++++ docshell/base/CanonicalBrowsingContext.h | 6 ++++++ docshell/base/nsDocShell.cpp | 18 ------------------ .../browser_backforward_userinteraction.js | 5 ----- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index bacba9e6b3b9..92c4218517c9 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -535,6 +535,8 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId, } } + ResetSHEntryHasUserInteractionCache(); + HistoryCommitIndexAndLength(aChangeID, caller); return; @@ -650,6 +652,9 @@ void CanonicalBrowsingContext::SetActiveSessionHistoryEntry( UseRemoteSubframes()); } } + + ResetSHEntryHasUserInteractionCache(); + // FIXME Need to do the equivalent of EvictContentViewersOrReplaceEntry. HistoryCommitIndexAndLength(aChangeID, caller); } @@ -667,6 +672,9 @@ void CanonicalBrowsingContext::ReplaceActiveSessionHistoryEntry( shistory->NotifyOnHistoryReplaceEntry(); shistory->UpdateRootBrowsingContextState(); } + + ResetSHEntryHasUserInteractionCache(); + // FIXME Need to do the equivalent of EvictContentViewersOrReplaceEntry. } @@ -1558,6 +1566,13 @@ void CanonicalBrowsingContext::EndDocumentLoad(bool aForProcessSwitch) { } } +void CanonicalBrowsingContext::ResetSHEntryHasUserInteractionCache() { + WindowContext* topWc = GetTopWindowContext(); + if (topWc && !topWc->IsDiscarded()) { + MOZ_ALWAYS_SUCCEEDS(topWc->SetSHEntryHasUserInteraction(false)); + } +} + void CanonicalBrowsingContext::HistoryCommitIndexAndLength() { nsID changeID = {}; CallerWillNotifyHistoryIndexAndLengthChanges caller(nullptr); diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index 9793bfae1fd1..54c2c4e8bf3e 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -325,6 +325,12 @@ class CanonicalBrowsingContext final : public BrowsingContext { uint64_t mCrossGroupOpenerId = 0; + // This function will make the top window context reset its + // "SHEntryHasUserInteraction" cache that prevents documents from repeatedly + // setting user interaction on SH entries. Should be called anytime SH + // entries are added or replaced. + void ResetSHEntryHasUserInteractionCache(); + // The current remoteness change which is in a pending state. RefPtr mPendingRemotenessChange; diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index bafb05824fe1..e492ddb2a9f2 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -8909,15 +8909,6 @@ nsresult nsDocShell::HandleSameDocumentNavigation( scrollRestorationIsManual.value()); } - // As we're replacing the active session history entry we need to ensure - // the top window context resets its "SHEntryHasUserInteraction" cache - // that prevents documents from repeatedly setting user interaction - // on SH entries. - WindowContext* topWc = mBrowsingContext->GetTopWindowContext(); - if (topWc && !topWc->IsDiscarded()) { - MOZ_ALWAYS_SUCCEEDS(topWc->SetSHEntryHasUserInteraction(false)); - } - if (LOAD_TYPE_HAS_FLAGS(mLoadType, LOAD_FLAGS_REPLACE_HISTORY)) { mBrowsingContext->ReplaceActiveSessionHistoryEntry(mActiveEntry.get()); } else { @@ -11590,15 +11581,6 @@ void nsDocShell::UpdateActiveEntry( mActiveEntry->SetURIWasModified(aURIWasModified); mActiveEntry->SetScrollRestorationIsManual(aScrollRestorationIsManual); - // As we're replacing the active session history entry we need to ensure - // the top window context resets its "SHEntryHasUserInteraction" cache - // that prevents documents from repeatedly setting user interaction - // on SH entries. - WindowContext* topWc = mBrowsingContext->GetTopWindowContext(); - if (topWc && !topWc->IsDiscarded()) { - MOZ_ALWAYS_SUCCEEDS(topWc->SetSHEntryHasUserInteraction(false)); - } - if (replace) { mBrowsingContext->ReplaceActiveSessionHistoryEntry(mActiveEntry.get()); } else { diff --git a/docshell/test/browser/browser_backforward_userinteraction.js b/docshell/test/browser/browser_backforward_userinteraction.js index 7a0bfa1a8f23..a899004d34c7 100644 --- a/docshell/test/browser/browser_backforward_userinteraction.js +++ b/docshell/test/browser/browser_backforward_userinteraction.js @@ -376,10 +376,5 @@ add_task(async function test_iframe_pushState() { // entries without user interaction when navigating inside an iframe // by following links. add_task(async function test_iframe_followLink() { - // Bug 1670933 - if (gFissionBrowser) { - return; - } - await runIframeTest(followLink); });