Bug 312577 – Crash when closing web inspector window

• Component: Web Inspector
• Status: RESOLVED FIXED

[Ricci Adams]

Created attachment 479138 [details]
Crash log

I can replicate a crash 100% of the time on my machine by doing the following:

1) Click "Develop" in the menu bar
2) Click my local machine
3) Click on a remote process, such as Adobe Creative Cloud.
4) The web inspector window opens. Press ⌘W to close it.
5) Crash.

I ran with MallocStackLogging enabled and then ran Safari in the debugger. `_NSKeyValueObservationInfoGetObservances` is trying to access `0x600003824960`, but that has already been freed in `WebKit::RemoteWebInspectorUIProxy::closeFrontendPageAndWindow()`.

I believe the root cause is:
- `WebInspectorUIProxy::attachmentDidMoveToWindow()` registers `m_objCAdapter` as a KVO observer.
- `WebInspectorUIProxy::closeFrontendPage()` removes `m_objCAdapter` from NSNotificationCenter, but forgets to remove it from any KVO observation.

[Ricci Adams]

Created attachment 479139 [details]
Last alloc/free from malloc_history

[Radar WebKit Bug Importer]

<rdar://problem/175014253>

[Ricci Adams]

After some more debugging, my original analysis was wrong.

I used frida-trace to log KVO calls. I see -addObserver:forKeyPath:options:context: being called on WKWebView in WKInspectorViewController.mm.

 24731 ms  -[WKWebView addObserver:0x60000211cbd0 forKeyPath:safeAreaInsets options:0x0 context:0x10a57f150]

However, I'm never seeing the observer be removed. If I attach to Safari and then close the window, it crashes as expected with _NSKeyValueObservationInfoGetObservances  is trying to access 0x60000211cbd0.

The safeAreaInsets observer is only removed in -webViewWebContentProcessDidTerminate:

I believe that it also needs to be removed in -dealloc before the _webView is set to nil.

[Ricci Adams]

Created attachment 479152 [details]
Untested patch.

[Qianlang Chen]

Pull request: https://github.com/WebKit/WebKit/pull/63271

[EWS]

Committed 311899@main (0ee66ba15e9f): <https://commits.webkit.org/311899@main>

Reviewed commits have been landed. Closing PR #63271 and removing active labels.