[macOS] -[WKFullScreenWindowController exitFullScreenImmediately] does not exit fullscreen immediately
Created attachment 437566 [details] Patch
<rdar://82526303>
Comment on attachment 437566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437566&action=review LGTM. I'd take this opportunity to rename -finishedExitFullScreenAnimation: to something that says what the parameter is, like -finishedExitFullScreenAnimationAndExitImmediately:(BOOL)immediately. > Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm:572 > + if (immediately) > + [self completeFinishExitFullScreenAnimationAfterRepaint]; > + else { > + _page->forceRepaint([weakSelf = WeakObjCPtr<WKFullScreenWindowController>(self)] { > + [weakSelf completeFinishExitFullScreenAnimationAfterRepaint]; > + }); > + } Nit: I'd have an early return here, rather than an else.
Created attachment 437651 [details] Revise the patch based on Jer's comments
(In reply to Jer Noble from comment #3) > Comment on attachment 437566 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437566&action=review > > LGTM. I'd take this opportunity to rename -finishedExitFullScreenAnimation: > to something that says what the parameter is, like > -finishedExitFullScreenAnimationAndExitImmediately:(BOOL)immediately. Good idea! Fixed. > > > Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm:572 > > + if (immediately) > > + [self completeFinishExitFullScreenAnimationAfterRepaint]; > > + else { > > + _page->forceRepaint([weakSelf = WeakObjCPtr<WKFullScreenWindowController>(self)] { > > + [weakSelf completeFinishExitFullScreenAnimationAfterRepaint]; > > + }); > > + } > > Nit: I'd have an early return here, rather than an else. Fixed. I also renamed `completeFinishExitFullScreenAnimationAfterRepaint` to `completeFinishExitFullScreenAnimation` because it is not always called "after repaint".
Committed r282175 (241467@main): <https://commits.webkit.org/241467@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437651 [details].