Bug 231919 - WebAVPlayerController should use WeakPtr<> for C++ instance variables
Summary: WebAVPlayerController should use WeakPtr<> for C++ instance variables
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-18 16:30 PDT by David Kilzer (:ddkilzer)
Modified: 2021-12-01 13:31 PST (History)
7 users (show)

See Also:

Attachments
Patch v1 (4.73 KB, patch)
2021-10-18 16:32 PDT, David Kilzer (:ddkilzer) 		eric.carlson: review+
Details | Formatted Diff | Diff
Patch for landing (4.70 KB, patch)
2021-10-20 11:58 PDT, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
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 Details | Formatted Diff | Diff
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 Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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