Bug 225815 – Web Inspector: `_WKInspector` leaks `WebInspectorUIProxy`

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Patrick Angle]

<rdar://77178283>

[Patrick Angle]

Created attachment 428640 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 428640 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=428640&action=review

> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:217
> +    _inspector->~WebInspectorUIProxy();

`WebInspectorUIPRoxy` is also held by `WebPageProxy`.  Is this safe and/or the right thing to do?  I'd suggest talking with some folks familiar with WebKit's API to see if this is the correct approach.

[Patrick Angle]

Comment on attachment 428640 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=428640&action=review

>> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:217
>> +    _inspector->~WebInspectorUIProxy();
> 
> `WebInspectorUIPRoxy` is also held by `WebPageProxy`.  Is this safe and/or the right thing to do?  I'd suggest talking with some folks familiar with WebKit's API to see if this is the correct approach.

This is consistent with other API objects, like is the case in `_WKGeolocationPosition` and `_WKFrameHandle` which have a API::ObjectStorage wrapped object, and explicitly call the destructor during dealloc. Also re: WebPageProxy, I see where WebPageProxy creates a new WebInspectorUIProxy, but I don't see anywhere that specific proxy makes it to _WKInspector?

[EWS]

Committed r277518 (237747@main): <https://commits.webkit.org/237747@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428640 [details].