Summary: | Safari’s Privacy Report window is completely blank | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kate Cheney <katherine_cheney> | ||||||||||||||
Component: | New Bugs | Assignee: | Kate Cheney <katherine_cheney> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | cdumez | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Kate Cheney
2021-09-02 20:02:54 PDT
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]. |