Bug 222701

Summary:

Clean up API::ResourceLoadStatisticsThirdParty / API::ResourceLoadStatisticsFirstParty

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebKit2

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, beidson, ggaren, katherine_cheney, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Chris Dumez

Reported 2021-03-03 16:42:32 PST

Clean up API::ResourceLoadStatisticsThirdParty / API::ResourceLoadStatisticsFirstParty.

Attachments
Patch (4.21 KB, patch) 2021-03-03 16:44 PST, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (4.69 KB, patch) 2021-03-03 16:58 PST, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (4.71 KB, patch) 2021-03-04 14:02 PST, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2021-03-03 16:44:40 PST

Created attachment 422162 [details] Patch

Chris Dumez

Comment 2 2021-03-03 16:58:49 PST

Created attachment 422165 [details] Patch

Geoffrey Garen

Comment 3 2021-03-04 13:38:07 PST

Comment on attachment 422165 [details] Patch r=me Maybe we should make this a release assert so we can catch it in CrashTracer.

Alex Christensen

Comment 4 2021-03-04 13:39:18 PST

I think we need to do something more general. It should be made always OK to deallocate our ObjC objects on a background thread.

Chris Dumez

Comment 5 2021-03-04 13:54:05 PST

(In reply to Alex Christensen from comment #4) > I think we need to do something more general. It should be made always OK > to deallocate our ObjC objects on a background thread. It is a slippery slope. If we start allowing our objects to be destroyed on a background thread, then clients are doing to be tempted to also *USE* these objects on a background thread and our API is currently not thread-safe by any means.

Chris Dumez

Comment 6 2021-03-04 14:02:31 PST

Created attachment 422282 [details] Patch

Geoffrey Garen

Comment 7 2021-03-04 15:14:35 PST

Sometimes it can be hard, especially with ARC, to ensure deallocation on the main thread -- even when there is no use on a secondary thread -- just because of the transitive set of pointers in an object graph. In the past, we have sometimes written code to forward the deallocation of an API object to the main thread if needed. It might be something worth considering from some of our API objects. That said, in the cases we know about right now, the bug appears to be *use* on a secondary thread, and not just deallocation.

EWS

Comment 8 2021-03-04 15:14:59 PST

Committed r273923: <https://commits.webkit.org/r273923> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422282 [details].

Radar WebKit Bug Importer

Comment 9 2021-03-04 15:15:21 PST

<rdar://problem/75059950>

Chris Dumez

Comment 10 2021-03-04 15:19:34 PST

(In reply to Geoffrey Garen from comment #7) > Sometimes it can be hard, especially with ARC, to ensure deallocation on the > main thread -- even when there is no use on a secondary thread -- just > because of the transitive set of pointers in an object graph. In the past, > we have sometimes written code to forward the deallocation of an API object > to the main thread if needed. It might be something worth considering from > some of our API objects. That said, in the cases we know about right now, > the bug appears to be *use* on a secondary thread, and not just deallocation. I see, then I agree that it may be nice to make our API objects dispatch to main thread on destruction.

Note You need to log in before you can comment on or make changes to this bug.