Bug 229847 – Safari’s Privacy Report window is completely blank

• Component: New Bugs
• Status: RESOLVED FIXED

[Kate Cheney]

Safari’s Privacy Report window is completely blank

[Kate Cheney]

rdar://80974688

[Kate Cheney]

Created attachment 437237 [details]
Patch

[Chris Dumez]

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.

[Kate Cheney]

Created attachment 437294 [details]
Patch

[Kate Cheney]

Added a test, would like another review in case I did something silly!

[Chris Dumez]

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.

[Kate Cheney]

Created attachment 437296 [details]
Patch

[Chris Dumez]

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.

[Kate Cheney]

(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.

[Kate Cheney]

Created attachment 437310 [details]
Patch

[Chris Dumez]

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.

[Kate Cheney]

Created attachment 437315 [details]
Patch

[Chris Dumez]

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 :)

[Kate Cheney]

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.

[Kate Cheney]

Created attachment 437317 [details]
Patch

[Chris Dumez]

Comment on attachment 437317 [details]
Patch

r=me

[EWS]

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].