Have WebBackForwardCache class coordinate page caching in all WebProcesses.
<rdar://problem/56250421>
Created attachment 380889 [details] WIP Patch
Created attachment 380905 [details] WIP Patch
Created attachment 380921 [details] WIP Patch
Created attachment 380928 [details] WIP Patch
Created attachment 380947 [details] WIP Patch
Created attachment 380949 [details] WIP Patch
Created attachment 380950 [details] Patch
Created attachment 380951 [details] Patch
Comment on attachment 380951 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380951&action=review > Source/WebKit/UIProcess/WebBackForwardCache.h:70 > + Vector<WebBackForwardListItem*> m_itemsWithCachedPage; Might it be useful to have inline capacity of 2 here? > Source/WebKit/UIProcess/WebProcessPool.h:598 > WebProcessProxy* m_prewarmedProcess { nullptr }; > WebProcessProxy* m_dummyProcessProxy { nullptr }; // A lightweight WebProcessProxy without backing process. These should probably be WeakPtr<WebProcessProxy>, maybe not in this patch.
Created attachment 380956 [details] Patch
(In reply to Alex Christensen from comment #10) > Comment on attachment 380951 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=380951&action=review > > > Source/WebKit/UIProcess/WebBackForwardCache.h:70 > > + Vector<WebBackForwardListItem*> m_itemsWithCachedPage; > > Might it be useful to have inline capacity of 2 here? Done. > > > Source/WebKit/UIProcess/WebProcessPool.h:598 > > WebProcessProxy* m_prewarmedProcess { nullptr }; > > WebProcessProxy* m_dummyProcessProxy { nullptr }; // A lightweight WebProcessProxy without backing process. > > These should probably be WeakPtr<WebProcessProxy>, maybe not in this patch. Why not. I can take care of this in a follow-up.
Comment on attachment 380956 [details] Patch Clearing flags on attachment: 380956 Committed r251121: <https://trac.webkit.org/changeset/251121>
All reviewed patches have been landed. Closing bug.
Comment on attachment 380956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380956&action=review > Source/WebKit/UIProcess/WebBackForwardCache.cpp:41 > +class EntryWithSuspendedPage final : public WebBackForwardCacheEntry { Is subclassing here really necessary? I feel the code would be easier to follow if WebBackForwardCacheEntry was a concrete type and having a suspended page or not was just a state. > Source/WebKit/UIProcess/WebProcessProxy.cpp:635 > + if (!!item->backForwardCacheEntry() != itemState.hasCachedPage) { That !! seems unnecessary
(In reply to Antti Koivisto from comment #15) > Comment on attachment 380956 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=380956&action=review > > > Source/WebKit/UIProcess/WebBackForwardCache.cpp:41 > > +class EntryWithSuspendedPage final : public WebBackForwardCacheEntry { > > Is subclassing here really necessary? I feel the code would be easier to > follow if WebBackForwardCacheEntry was a concrete type and having a > suspended page or not was just a state. My earlier iteration wasn’t using inheritance and I found it less clear. However, given your feedback, I will give it another shot and upload a patch so you can decide. > > Source/WebKit/UIProcess/WebProcessProxy.cpp:635 > > + if (!!item->backForwardCacheEntry() != itemState.hasCachedPage) { > > That !! seems unnecessary Felt nicer to compare Booleans.