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

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