Summary: | [macOS] -[WKFullScreenWindowController exitFullScreenImmediately] does not exit fullscreen immediately | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||
Component: | Media | Assignee: | Peng Liu <peng.liu6> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric.carlson, jer.noble, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Peng Liu
2021-09-07 15:17:47 PDT
Created attachment 437566 [details]
Patch
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]. |