WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
232049
Web Inspector: extension iframes leak when disabling an extension
https://bugs.webkit.org/show_bug.cgi?id=232049
Summary
Web Inspector: extension iframes leak when disabling an extension
Blaze Burg
Reported
2021-10-20 15:11:41 PDT
D'oh.
Attachments
Patch v1.0
(2.94 KB, patch)
2021-10-21 11:17 PDT
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Patch v1.1
(3.00 KB, patch)
2021-10-26 13:42 PDT
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-10-20 15:33:44 PDT
<
rdar://problem/84482021
>
Blaze Burg
Comment 2
2021-10-21 11:17:49 PDT
Created
attachment 442050
[details]
Patch v1.0
Devin Rousso
Comment 3
2021-10-21 11:30:39 PDT
Comment on
attachment 442050
[details]
Patch v1.0 View in context:
https://bugs.webkit.org/attachment.cgi?id=442050&action=review
> Source/WebInspectorUI/ChangeLog:9 > + Since shouldNotRemoveFromDOMWhenHidden() is true, the <iframe> will not > + be detached and unload its document in the normal tab-closing code path.
This will still remove the `WI.View` from the parent `_subviews` even though the node wont be removed from the DOM, right? Can we somehow confirm that removing an extension does in fact GC all the related objects?
> Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js:74 > + tabContentView.dispose();
I would _strongly_ prefer that this be on `WI.WebInspectorExtensionTabContentView` instead of the more generic `WI.TabContentView` as the latter shouldn't ever need this mechanism (i.e. it should _always_ use `WI.View.prototype.removeSubview`)
Blaze Burg
Comment 4
2021-10-21 12:19:34 PDT
Comment on
attachment 442050
[details]
Patch v1.0 View in context:
https://bugs.webkit.org/attachment.cgi?id=442050&action=review
>> Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js:74 >> + tabContentView.dispose(); > > I would _strongly_ prefer that this be on `WI.WebInspectorExtensionTabContentView` instead of the more generic `WI.TabContentView` as the latter shouldn't ever need this mechanism (i.e. it should _always_ use `WI.View.prototype.removeSubview`)
Oh, woops, wrong file :)(
Blaze Burg
Comment 5
2021-10-26 13:42:58 PDT
Created
attachment 442525
[details]
Patch v1.1
EWS
Comment 6
2021-10-27 14:23:50 PDT
Committed
r284958
(
243610@main
): <
https://commits.webkit.org/243610@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 442525
[details]
.
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