Summary: | Follow-up to r286479 to add API test and address issues found by the test | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
Component: | Tools / Tests | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, darin, ggaren, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=233783 | ||||||||||||
Bug Depends on: | 233874 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Chris Dumez
2021-12-02 20:31:20 PST
Created attachment 445857 [details]
Patch
Comment on attachment 445857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445857&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:419 > +- (void)_isLayerTreeFrozen:(void (^)(BOOL frozen))completionHandler; availability macros, WKWebViewPrivateForTesting.h > Source/WebKit/WebProcess/WebPage/WebPage.messages.in:405 > + isLayerTreeFrozenForTesting() -> (bool isFrozen) Async The freezing isn't for testing, the query is. I'm not sure if the naming could be more clear. > Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:7275 > + sourceHeaders.add("Content-Type"_s, "text/html"_s); These can be put in the initializer list. That would clean up the syntax a lot. (In reply to Alex Christensen from comment #2) > Comment on attachment 445857 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445857&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:419 > > +- (void)_isLayerTreeFrozen:(void (^)(BOOL frozen))completionHandler; > > availability macros, WKWebViewPrivateForTesting.h I thought we didn't need TBA macros for SPI? We do Created attachment 445863 [details]
Patch
Comment on attachment 445863 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445863&action=review > Source/WebKit/ChangeLog:3 > + Add API test for Bug 233783 Looks like you found something additional to fix in ProvisionalPageProxy, so this is not just adding a test any more. Is that correct? Should we retitle? > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:419 > +- (void)_isLayerTreeFrozen:(void (^)(BOOL frozen))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); Why did you decide to put this into Private rather than PrivateForTesting? (In reply to Darin Adler from comment #6) > Comment on attachment 445863 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445863&action=review > > > Source/WebKit/ChangeLog:3 > > + Add API test for Bug 233783 > > Looks like you found something additional to fix in ProvisionalPageProxy, so > this is not just adding a test any more. Is that correct? Should we retitle? Correct, I will retitle. > > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:419 > > +- (void)_isLayerTreeFrozen:(void (^)(BOOL frozen))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > Why did you decide to put this into Private rather than PrivateForTesting? Because I wasn't aware of the WKWebViewPrivateForTesting.h header. I will move it there. Created attachment 445865 [details]
Patch
Comment on attachment 445865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445865&action=review > Source/WebKit/WebProcess/WebPage/WebPage.messages.in:405 > + isLayerTreeFrozen() -> (bool isFrozen) Async nit: for some reason, all the messages.in names start with a capital letter. Created attachment 445866 [details]
Patch
Committed r286505 (244844@main): <https://commits.webkit.org/244844@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445866 [details]. |