Safari’s Privacy Report window is completely blank
rdar://80974688
Created attachment 437237 [details] Patch
Comment on attachment 437237 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437237&action=review r=me > Source/WebKit/ChangeLog:9 > + No new tests, suspension kicks in 30 seconds after a web process You could imagine an SPI to tweak that delay but OK.
Created attachment 437294 [details] Patch
Added a test, would like another review in case I did something silly!
Comment on attachment 437294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437294&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:86 > +- (void)_suspendAllCachedWebProcesses WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); I'd much rather we have a single SPI to tweak cachedProcessSuspensionDelay, as I suggested.
Created attachment 437296 [details] Patch
Comment on attachment 437296 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437296&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:86 > +- (void)_suspendAllCachedWebProcesses WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); This new iteration still has the same issue.
(In reply to Chris Dumez from comment #8) > Comment on attachment 437296 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437296&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:86 > > +- (void)_suspendAllCachedWebProcesses WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > This new iteration still has the same issue. Sorry, I missed your comment and was just posting to adjust the build failures.
Created attachment 437310 [details] Patch
Comment on attachment 437310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437310&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:85 > +- (void)_keepCachedProcessesAliveForTesting:(BOOL)shouldKeepCachedProcessesAliveForTesting WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); Why do we still have this? I think we should avoid adding this. > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:86 > +- (void)_setCachedProcessSuspensionDelayForTesting:(double)delayInSeconds WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); This could be: + (void)_setCachedProcessSuspensionDelayForTesting:(double)delayInSeconds WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); since this is static internally. > Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:1355 > + [dataStore _setCachedProcessSuspensionDelayForTesting:0]; Could be [WKWebsiteDataStore _setCachedProcessSuspensionDelayForTesting:0] with the change I suggested above.
Created attachment 437315 [details] Patch
Comment on attachment 437315 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437315&action=review > Source/WebKit/UIProcess/WebProcessCache.cpp:44 > +static Seconds& cachedProcessSuspensionDelay() Why do we need a function, I don't see any reason why we cannot keep my global and drop "constexpr", is there? > Source/WebKit/UIProcess/WebProcessCache.cpp:327 > +#if PLATFORM(MAC) Either outside the function or inside but not both :)
Comment on attachment 437315 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437315&action=review >> Source/WebKit/UIProcess/WebProcessCache.cpp:44 >> +static Seconds& cachedProcessSuspensionDelay() > > Why do we need a function, I don't see any reason why we cannot keep my global and drop "constexpr", is there? No good reason except me being silly >> Source/WebKit/UIProcess/WebProcessCache.cpp:327 >> +#if PLATFORM(MAC) > > Either outside the function or inside but not both :) Oops, Friday brain. Will fix.
Created attachment 437317 [details] Patch
Comment on attachment 437317 [details] Patch r=me
Committed r282034 (241336@main): <https://commits.webkit.org/241336@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437317 [details].