Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 492490a51a6c23ab76a9556c49c56ea445903b43
      
https://github.com/WebKit/WebKit/commit/492490a51a6c23ab76a9556c49c56ea445903b43
  Author: Wenson Hsieh <[email protected]>
  Date:   2022-12-14 (Wed, 14 Dec 2022)

  Changed paths:
    M Source/WebKit/UIProcess/mac/WebViewImpl.mm
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/PrepareForMoveToWindow.mm

  Log Message:
  -----------
  Mitigate crashes when removing KVO from NSWindow in 
-[WKWindowVisibilityObserver stopObserving:]
https://bugs.webkit.org/show_bug.cgi?id=249103
rdar://102360839

Reviewed by Patrick Angle.

After the fix in 256334@main, Music sometimes crashes when destroying 
`NSWindow`, when
`WKWindowVisibilityObserver` attempts to remove key-value observers for 
"contentLayoutRect" and
"titlebarAppearsTransparent" from the window that were not added in the first 
place.

While I haven't been able to reproduce the crash locally or come up with a test 
case that (exactly)
replicates the crashing stack during `NSWindow` destruction, it should be 
possible to avoid it
altogether by guarding KVO registration and unregistration by using an 
associated object on the
`NSWindow` to indicate when `WKWindowVisibilityObserver` has key-value 
observers to the window. If
this flag is not set, then we avoid attempting to unregister KVO; similarly, if 
this flag is set,
then we avoid attempting to re-register KVO.

Test: WKWebView.PrepareForMoveToWindowShouldNotCrashWhenRemovingWindowObservers

* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(-[WKWindowVisibilityObserver startObserving:]):
(-[WKWindowVisibilityObserver stopObserving:]):

Use the `_impl` pointer as the context key.

(WebKit::WebViewImpl::viewWillMoveToWindowImpl):
(WebKit::WebViewImpl::viewWillMoveToWindow):

Move a debug assertion from `viewWillMoveToWindowImpl` into the call site in 
`viewWillMoveToWindow`,
such that the assertion doesn't fire due to the SPI being misused by a client 
(which the new test
case exercises).

* Tools/TestWebKitAPI/Tests/WebKitCocoa/PrepareForMoveToWindow.mm:

Add a (somewhat contrived) API test that exercises the mitigation.

Canonical link: https://commits.webkit.org/257860@main


_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to