From 2dbd18c6c8cbae910e1127bd89a4539d9c36c89a Mon Sep 17 00:00:00 2001 From: Alphan Chen Date: Wed, 22 May 2019 09:06:14 +0000 Subject: [PATCH] Bug 1549973 - Force tabState flush from C++ listener before tab is closed r=peterv Differential Revision: https://phabricator.services.mozilla.com/D30475 --HG-- extra : moz-landing-system : lando --- browser/base/content/browser.js | 17 +++++++---------- .../components/sessionstore/SessionStore.jsm | 17 ++++++++++------- dom/base/nsFrameLoader.cpp | 9 ++++++--- dom/base/nsFrameLoader.h | 2 +- dom/ipc/BrowserChild.cpp | 8 ++++---- dom/ipc/BrowserChild.h | 5 +++-- dom/ipc/BrowserParent.cpp | 5 +++-- dom/ipc/BrowserParent.h | 3 ++- dom/ipc/PBrowser.ipdl | 5 +++-- .../sessionstore/SessionStoreListener.cpp | 8 ++++---- .../sessionstore/SessionStoreListener.h | 4 ++-- xpfe/appshell/nsIXULBrowserWindow.idl | 2 +- 12 files changed, 46 insertions(+), 39 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index b3c62c04a179..ab255978c00b 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -5318,16 +5318,13 @@ var XULBrowserWindow = { this._sessionData.isPrivate = aIsPrivate; }, - updateSessionStore: function XWB_updateSessionStore(aBrowser, aFlushId) { - let tab = gBrowser.getTabForBrowser(aBrowser); - if (tab) { - SessionStore.updateSessionStoreFromTablistener(tab, { - data: this._sessionData, - flushID: aFlushId, - isFinal: false, - }); - this._sessionData = {}; - } + updateSessionStore: function XWB_updateSessionStore(aBrowser, aFlushId, aIsFinal) { + SessionStore.updateSessionStoreFromTablistener(aBrowser, { + data: this._sessionData, + flushID: aFlushId, + isFinal: aIsFinal, + }); + this._sessionData = {}; }, }; diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm index ef2c013b616f..6a10e519644d 100644 --- a/browser/components/sessionstore/SessionStore.jsm +++ b/browser/components/sessionstore/SessionStore.jsm @@ -369,8 +369,8 @@ var SessionStore = { return SessionStoreInternal.navigateAndRestore(tab, loadArguments, historyIndex); }, - updateSessionStoreFromTablistener(aTab, aData) { - return SessionStoreInternal.updateSessionStoreFromTablistener(aTab, aData); + updateSessionStoreFromTablistener(aBrowser, aData) { + return SessionStoreInternal.updateSessionStoreFromTablistener(aBrowser, aData); }, getSessionHistory(tab, updatedCallback) { @@ -822,17 +822,20 @@ var SessionStoreInternal = { } }, - updateSessionStoreFromTablistener(aTab, aData) { - let browser = aTab.linkedBrowser; - let win = browser.ownerGlobal; - TabState.update(browser, aData); + updateSessionStoreFromTablistener(aBrowser, aData) { + if (aBrowser.permanentKey == undefined) { + return; + } + + TabState.update(aBrowser, aData); + let win = aBrowser.ownerGlobal; this.saveStateDelayed(win); if (aData.flushID) { // This is an update kicked off by an async flush request. Notify the // TabStateFlusher so that it can finish the request and notify its // consumer that's waiting for the flush to be done. - TabStateFlusher.resolve(browser, aData.flushID); + TabStateFlusher.resolve(aBrowser, aData.flushID); } }, diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index b1fa833906cf..db8654bcca69 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -1788,6 +1788,9 @@ void nsFrameLoader::StartDestroy() { } mDestroyCalled = true; + // request a tabStateFlush before tab is closed + RequestTabStateFlush(/*flushId*/ 0, /*isFinal*/ true); + // After this point, we return an error when trying to send a message using // the message manager on the frame. if (mMessageManager) { @@ -3172,16 +3175,16 @@ void nsFrameLoader::RequestUpdatePosition(ErrorResult& aRv) { } } -bool nsFrameLoader::RequestTabStateFlush(uint32_t aFlushId) { +bool nsFrameLoader::RequestTabStateFlush(uint32_t aFlushId, bool aIsFinal) { if (mSessionStoreListener) { - mSessionStoreListener->ForceFlushFromParent(aFlushId); + mSessionStoreListener->ForceFlushFromParent(aFlushId, aIsFinal); // No async ipc call is involved in parent only case return false; } // If remote browsing (e10s), handle this with the BrowserParent. if (mBrowserParent) { - Unused << mBrowserParent->SendFlushTabState(aFlushId); + Unused << mBrowserParent->SendFlushTabState(aFlushId, aIsFinal); return true; } diff --git a/dom/base/nsFrameLoader.h b/dom/base/nsFrameLoader.h index bfc314c7b6c1..4c22faa71dcf 100644 --- a/dom/base/nsFrameLoader.h +++ b/dom/base/nsFrameLoader.h @@ -195,7 +195,7 @@ class nsFrameLoader final : public nsStubMutationObserver, void RequestUpdatePosition(mozilla::ErrorResult& aRv); - bool RequestTabStateFlush(uint32_t aFlushId); + bool RequestTabStateFlush(uint32_t aFlushId, bool aIsFinal = false); void Print(uint64_t aOuterWindowID, nsIPrintSettings* aPrintSettings, nsIWebProgressListener* aProgressListener, diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 1c16e7be4a6b..2398fe65b228 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -1940,8 +1940,8 @@ mozilla::ipc::IPCResult BrowserChild::RecvNativeSynthesisResponse( } mozilla::ipc::IPCResult BrowserChild::RecvFlushTabState( - const uint32_t& aFlushId) { - UpdateSessionStore(aFlushId); + const uint32_t& aFlushId, const bool& aIsFinal) { + UpdateSessionStore(aFlushId, aIsFinal); return IPC_OK(); } @@ -3626,7 +3626,7 @@ nsresult BrowserChild::PrepareProgressListenerData( return NS_OK; } -bool BrowserChild::UpdateSessionStore(uint32_t aFlushId) { +bool BrowserChild::UpdateSessionStore(uint32_t aFlushId, bool aIsFinal) { if (!mSessionStoreListener) { return false; } @@ -3649,7 +3649,7 @@ bool BrowserChild::UpdateSessionStore(uint32_t aFlushId) { } Unused << SendSessionStoreUpdate(docShellCaps, privatedMode, positions, - positionDescendants, aFlushId); + positionDescendants, aFlushId, aIsFinal); return true; } diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index 6bf174573479..47592c1e4b4f 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -396,7 +396,8 @@ class BrowserChild final : public BrowserChildBase, const WidgetTouchEvent& aEvent, const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId, const nsEventStatus& aApzResponse); - mozilla::ipc::IPCResult RecvFlushTabState(const uint32_t& aFlushId); + mozilla::ipc::IPCResult RecvFlushTabState(const uint32_t& aFlushId, + const bool& aIsFinal); mozilla::ipc::IPCResult RecvNativeSynthesisResponse( const uint64_t& aObserverId, const nsCString& aResponse); @@ -690,7 +691,7 @@ class BrowserChild final : public BrowserChildBase, return *sVisibleTabs; } - bool UpdateSessionStore(uint32_t aFlushId); + bool UpdateSessionStore(uint32_t aFlushId, bool aIsFinal = false); protected: virtual ~BrowserChild(); diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index b86a312c47a4..de561baacc0d 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -2522,7 +2522,8 @@ void BrowserParent::ReconstructWebProgressAndRequest( mozilla::ipc::IPCResult BrowserParent::RecvSessionStoreUpdate( const Maybe& aDocShellCaps, const Maybe& aPrivatedMode, const nsTArray& aPositions, - const nsTArray& aPositionDescendants, const uint32_t& aFlushId) { + const nsTArray& aPositionDescendants, const uint32_t& aFlushId, + const bool& aIsFinal) { nsCOMPtr xulBrowserWindow = GetXULBrowserWindow(); if (!xulBrowserWindow) { return IPC_OK(); @@ -2540,7 +2541,7 @@ mozilla::ipc::IPCResult BrowserParent::RecvSessionStoreUpdate( xulBrowserWindow->UpdateScrollPositions(aPositions, aPositionDescendants); } - xulBrowserWindow->UpdateSessionStore(mFrameElement, aFlushId); + xulBrowserWindow->UpdateSessionStore(mFrameElement, aFlushId, aIsFinal); return IPC_OK(); } diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index cda214d7552b..e232a47cd7e5 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -296,7 +296,8 @@ class BrowserParent final : public PBrowserParent, mozilla::ipc::IPCResult RecvSessionStoreUpdate( const Maybe& aDocShellCaps, const Maybe& aPrivatedMode, const nsTArray& aPositions, - const nsTArray& aPositionDescendants, const uint32_t& aFlushId); + const nsTArray& aPositionDescendants, const uint32_t& aFlushId, + const bool& aIsFinal); mozilla::ipc::IPCResult RecvBrowserFrameOpenWindow( PBrowserParent* aOpener, const nsString& aURL, const nsString& aName, diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index b217dbee8cbb..d981136562bc 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -560,11 +560,12 @@ parent: bool? aPrivatedMode, nsCString[] aPositions, int32_t[] aPositionDescendants, - uint32_t aFlushId); + uint32_t aFlushId, + bool aIsFinal); child: async NativeSynthesisResponse(uint64_t aObserverId, nsCString aResponse); - async FlushTabState(uint32_t aFlushId); + async FlushTabState(uint32_t aFlushId, bool aIsFinal); parent: diff --git a/toolkit/components/sessionstore/SessionStoreListener.cpp b/toolkit/components/sessionstore/SessionStoreListener.cpp index f96b4edb44f4..4f471dd65fc0 100644 --- a/toolkit/components/sessionstore/SessionStoreListener.cpp +++ b/toolkit/components/sessionstore/SessionStoreListener.cpp @@ -413,17 +413,17 @@ void ContentSessionStore::GetScrollPositions( mScrollChanged = NO_CHANGE; } -bool TabListener::ForceFlushFromParent(uint32_t aFlushId) { +bool TabListener::ForceFlushFromParent(uint32_t aFlushId, bool aIsFinal) { if (!XRE_IsParentProcess()) { return false; } if (!mSessionStore) { return false; } - return UpdateSessionStore(aFlushId); + return UpdateSessionStore(aFlushId, aIsFinal); } -bool TabListener::UpdateSessionStore(uint32_t aFlushId) { +bool TabListener::UpdateSessionStore(uint32_t aFlushId, bool aIsFinal) { if (!aFlushId) { if (!mSessionStore || !mSessionStore->UpdateNeeded()) { return false; @@ -475,7 +475,7 @@ bool TabListener::UpdateSessionStore(uint32_t aFlushId) { mSessionStore->GetScrollPositions(positions, descendants); xulBrowserWindow->UpdateScrollPositions(positions, descendants); } - xulBrowserWindow->UpdateSessionStore(mOwnerContent, aFlushId); + xulBrowserWindow->UpdateSessionStore(mOwnerContent, aFlushId, aIsFinal); StopTimerForUpdate(); return true; } diff --git a/toolkit/components/sessionstore/SessionStoreListener.h b/toolkit/components/sessionstore/SessionStoreListener.h index 3dd9f14be679..abe43bf86063 100644 --- a/toolkit/components/sessionstore/SessionStoreListener.h +++ b/toolkit/components/sessionstore/SessionStoreListener.h @@ -63,7 +63,7 @@ class TabListener : public nsIDOMEventListener, nsresult Init(); ContentSessionStore* GetSessionStore() { return mSessionStore; } // the function is called only when TabListener is in parent process - bool ForceFlushFromParent(uint32_t aFlushId); + bool ForceFlushFromParent(uint32_t aFlushId, bool aIsFinal = false); void RemoveListeners(); NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -78,7 +78,7 @@ class TabListener : public nsIDOMEventListener, static void TimerCallback(nsITimer* aTimer, void* aClosure); void AddTimerForUpdate(); void StopTimerForUpdate(); - bool UpdateSessionStore(uint32_t aFlushId = 0); + bool UpdateSessionStore(uint32_t aFlushId = 0, bool aIsFinal = false); virtual ~TabListener(); nsCOMPtr mDocShell; diff --git a/xpfe/appshell/nsIXULBrowserWindow.idl b/xpfe/appshell/nsIXULBrowserWindow.idl index 2cb92bcd54ed..8d2added3df1 100644 --- a/xpfe/appshell/nsIXULBrowserWindow.idl +++ b/xpfe/appshell/nsIXULBrowserWindow.idl @@ -96,6 +96,6 @@ interface nsIXULBrowserWindow : nsISupports void updateDocShellCaps(in ACString aDisCaps); void updateIsPrivate(in boolean aIsPrivate); void updateScrollPositions(in Array aPositions, in Array aChildren); - void updateSessionStore(in Element aBrowser, in uint32_t aFlushId); + void updateSessionStore(in Element aBrowser, in uint32_t aFlushId, in boolean aIsFinal); };