Bug 231919

Summary:

WebAVPlayerController should use WeakPtr<> for C++ instance variables

Product:

WebKit

Reporter:

David Kilzer (:ddkilzer) <ddkilzer>

Component:

Media

Assignee:

David Kilzer (:ddkilzer) <ddkilzer>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, darin, eric.carlson, jer.noble, peng.liu6, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=231956
https://bugs.webkit.org/show_bug.cgi?id=232676

Attachments:

Description	Flags
Patch v1	eric.carlson: review+
Patch for landing	none
Patch for landing (retry for api-ios tests for the third time)	none
Patch for landing (retry for api-ios tests for the fourth time)	none

Description David Kilzer (:ddkilzer) 2021-10-18 16:30:46 PDT

WebAVPlayerController should use WeakPtr<> for C++ instance variables.

The "rules" I'm following here are:
- RefCounted objects should be held weakly instead of using raw pointers (WebCore::PlaybackSessionInterfaceAVKit).
  - I assume this isn't a RefPtr<> to prevent a retain cycle.
- Delegate objects should always be held weakly (WebCore::PlaybackSessionModel, which already subclasses WeakPtr<>).

Comment 1 David Kilzer (:ddkilzer) 2021-10-18 16:32:17 PDT

Created attachment 441656 [details]
Patch v1

Comment 2 David Kilzer (:ddkilzer) 2021-10-20 11:58:52 PDT

Created attachment 441914 [details]
Patch for landing

Comment 3 David Kilzer (:ddkilzer) 2021-10-20 12:00:29 PDT

(In reply to David Kilzer (:ddkilzer) from comment #2)
> Created attachment 441914 [details]
> Patch for landing

The ios-wk2 bot for Attachment #441656 [details] is stuck on orange, and will probably never fail over to allow a retry, so I'm posting this "Patch for landing" to make the ios-wk2 and api-ios bots retest the patch.

Comment 4 David Kilzer (:ddkilzer) 2021-10-21 21:23:49 PDT

Created attachment 442117 [details]
Patch for landing (retry for api-ios tests for the third time)

Comment 5 David Kilzer (:ddkilzer) 2021-10-22 16:27:02 PDT

Created attachment 442216 [details]
Patch for landing (retry for api-ios tests for the fourth time)

Comment 6 David Kilzer (:ddkilzer) 2021-10-23 07:45:45 PDT

Comment on attachment 442216 [details]
Patch for landing (retry for api-ios tests for the fourth time)

Marking cq+ since the api-ios and ios-wk2 bots passed separately on different posts.

Comment 7 EWS 2021-10-23 08:21:23 PDT

Committed r284743 (243452@main): <https://commits.webkit.org/243452@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442216 [details].

Comment 8 Radar WebKit Bug Importer 2021-10-23 08:22:17 PDT

<rdar://problem/84580153>

Comment 9 David Kilzer (:ddkilzer) 2021-12-01 13:31:52 PST

(In reply to EWS from comment #7)
> Committed r284743 (243452@main): <https://commits.webkit.org/243452@main>
> 
> All reviewed patches have been landed. Closing bug and clearing flags on
> attachment 442216 [details].

This caused:

Bug 232676: [ iOS ] TestWebKitAPI.WebKitLegacy.AudioSessionCategoryIOS is crashing