Bug 225815 - Web Inspector: `_WKInspector` leaks `WebInspectorUIProxy`
Summary: Web Inspector: `_WKInspector` leaks `WebInspectorUIProxy`
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL:
Keywords: InRadar
Depends on: 223899
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-14 10:42 PDT by Patrick Angle
Modified: 2021-05-14 16:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1.0 (1.28 KB, patch)
2021-05-14 10:48 PDT, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2021-05-14 10:42:16 PDT
<rdar://77178283>
Comment 1 Patrick Angle 2021-05-14 10:48:28 PDT
Created attachment 428640 [details]
Patch v1.0
Comment 2 Devin Rousso 2021-05-14 12:17:19 PDT
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.
Comment 3 Patrick Angle 2021-05-14 13:12:45 PDT
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?
Comment 4 EWS 2021-05-14 16:34:53 PDT
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].