WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
238747
Use Ref and RefPtr pattern when handling document close calls
https://bugs.webkit.org/show_bug.cgi?id=238747
Summary
Use Ref and RefPtr pattern when handling document close calls
Gabriel Nava Marino
Reported
2022-04-04 11:12:21 PDT
...
Attachments
Patch
(9.88 KB, patch)
2022-04-04 11:19 PDT
,
Gabriel Nava Marino
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Gabriel Nava Marino
Comment 1
2022-04-04 11:19:43 PDT
Created
attachment 456594
[details]
Patch
Sam Weinig
Comment 2
2022-04-04 11:30:36 PDT
Comment on
attachment 456594
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=456594&action=review
> Source/WebCore/ChangeLog:8 > + Ensure document object remains for the scope of the call.
I wonder ifs we should come up with some convention (naming or otherwise) to indicate that a caller is responsible for maintaining an external ref. Like, Document::close_mayDestroy(), or perhaps even take it a step further and require a separate type to call close()? Ref closer = document.closer() closer.close() /* can safely use document as long as closer is in scope */
Gabriel Nava Marino
Comment 3
2022-04-04 17:06:01 PDT
(In reply to Sam Weinig from
comment #2
)
> Comment on
attachment 456594
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=456594&action=review
> > > Source/WebCore/ChangeLog:8 > > + Ensure document object remains for the scope of the call. > > I wonder ifs we should come up with some convention (naming or otherwise) to > indicate that a caller is responsible for maintaining an external ref. Like, > Document::close_mayDestroy(), or perhaps even take it a step further and > require a separate type to call close()? > > Ref closer = document.closer() > closer.close() > > /* can safely use document as long as closer is in scope */
I agree this could be executed by explicit calls such above, which themselves could be enforced for example via assertions. I can create a follow-up bug to track this suggestion.
Gabriel Nava Marino
Comment 4
2022-04-04 18:42:20 PDT
(In reply to Gabriel Nava Marino from
comment #3
)
> (In reply to Sam Weinig from
comment #2
) > > Comment on
attachment 456594
[details]
> > Patch > > > > View in context: > >
https://bugs.webkit.org/attachment.cgi?id=456594&action=review
> > > > > Source/WebCore/ChangeLog:8 > > > + Ensure document object remains for the scope of the call. > > > > I wonder ifs we should come up with some convention (naming or otherwise) to > > indicate that a caller is responsible for maintaining an external ref. Like, > > Document::close_mayDestroy(), or perhaps even take it a step further and > > require a separate type to call close()? > > > > Ref closer = document.closer() > > closer.close() > > > > /* can safely use document as long as closer is in scope */ > > I agree this could be executed by explicit calls such above, which > themselves could be enforced for example via assertions. > > I can create a follow-up bug to track this suggestion.
https://bugs.webkit.org/show_bug.cgi?id=238780
EWS
Comment 5
2022-04-04 18:48:04 PDT
Committed
r292361
(
249226@main
): <
https://commits.webkit.org/249226@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 456594
[details]
.
Radar WebKit Bug Importer
Comment 6
2022-04-04 18:49:16 PDT
<
rdar://problem/91272528
>
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