Bug 231919

Summary: WebAVPlayerController should use WeakPtr<> for C++ instance variables
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: MediaAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, darin, eric.carlson, jer.noble, peng.liu6, rniwa, webkit-bug-importer
Priority: P2 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

David Kilzer (:ddkilzer)
Reported 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<>).
Attachments
Patch v1 (4.73 KB, patch)
2021-10-18 16:32 PDT, David Kilzer (:ddkilzer) 		eric.carlson: review+
DetailsFormatted DiffDiff
Patch for landing (4.70 KB, patch)
2021-10-20 11:58 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch for landing (retry for api-ios tests for the third time) (4.70 KB, patch)
2021-10-21 21:23 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch for landing (retry for api-ios tests for the fourth time) (4.70 KB, patch)
2021-10-22 16:27 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2021-10-18 16:32:17 PDT
Created attachment 441656 [details] Patch v1
David Kilzer (:ddkilzer)
Comment 2 2021-10-20 11:58:52 PDT
Created attachment 441914 [details] Patch for landing
David Kilzer (:ddkilzer)
Comment 3 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.
David Kilzer (:ddkilzer)
Comment 4 2021-10-21 21:23:49 PDT
Created attachment 442117 [details] Patch for landing (retry for api-ios tests for the third time)
David Kilzer (:ddkilzer)
Comment 5 2021-10-22 16:27:02 PDT
Created attachment 442216 [details] Patch for landing (retry for api-ios tests for the fourth time)
David Kilzer (:ddkilzer)
Comment 6 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.
EWS
Comment 7 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].
Radar WebKit Bug Importer
Comment 8 2021-10-23 08:22:17 PDT
<rdar://problem/84580153>
David Kilzer (:ddkilzer)
Comment 9 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
Note You need to log in before you can comment on or make changes to this bug.