WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
221988
REGRESSION(
r266890
): [Cocoa] Fix InspectorDelegate / API::InspectorClient leak
https://bugs.webkit.org/show_bug.cgi?id=221988
Summary
REGRESSION(r266890): [Cocoa] Fix InspectorDelegate / API::InspectorClient leak
Blaze Burg
Reported
2021-02-16 12:52:19 PST
<
rdar://73594555
>
Attachments
Patch v1.0
(4.83 KB, patch)
2021-02-16 13:00 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Patch v1.1
(4.82 KB, patch)
2021-02-16 13:25 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Blaze Burg
Comment 1
2021-02-16 13:00:07 PST
Created
attachment 420528
[details]
Patch v1.0
Devin Rousso
Comment 2
2021-02-16 13:14:45 PST
Comment on
attachment 420528
[details]
Patch v1.0 View in context:
https://bugs.webkit.org/attachment.cgi?id=420528&action=review
r=me, nice fix :)
> Source/WebKit/ChangeLog:27 > + via the constructor. If a nil delegate is passedâfor example, when > + closing the WKWebViewâthen don't create an API::InspectorClient
i think there's non-ascii here :(
> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:61 > + if (!delegate && !_delegate) > + return;
What about if `!delegate` but `_delegate`? Should we `_delegate = nullptr;` in that scenario?
> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:63 > + _delegate = makeUnique<WebKit::InspectorDelegate>(self, delegate);
I could be crazy, but I think you can drop the `<WebKit::InspectorDelegate>` as `makeUnique` might be able to deduce the type.
Blaze Burg
Comment 3
2021-02-16 13:23:21 PST
Comment on
attachment 420528
[details]
Patch v1.0 View in context:
https://bugs.webkit.org/attachment.cgi?id=420528&action=review
>> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:61 >> + return; > > What about if `!delegate` but `_delegate`? Should we `_delegate = nullptr;` in that scenario?
No. If !delegate, then we recreate InspectorDelegate with nil ObjC delegate, which causes us to call WebInspectorProxy::setClient(nullptr) in the constructor. Otherwise we'd need to disconnect this ourselves which defeats the point of the refactoring herein.
Blaze Burg
Comment 4
2021-02-16 13:23:55 PST
Waiting for mac-wk2 tests to run prior to landing.
Blaze Burg
Comment 5
2021-02-16 13:25:31 PST
Created
attachment 420532
[details]
Patch v1.1
EWS
Comment 6
2021-02-16 15:43:42 PST
Committed
r272935
: <
https://commits.webkit.org/r272935
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 420532
[details]
.
Blaze Burg
Comment 7
2021-03-29 13:20:09 PDT
This didn't fix the problem entirely. I'm creating a new bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug