Skip to content

Commit

Permalink
Remove default protocol implementations leading to potentially uncall…
Browse files Browse the repository at this point in the history
…ed implementations

Default implementations in protocol extensions are dangerous. If the protocol conformance is made on an extension any override will not be called.
  • Loading branch information
defagos committed Dec 21, 2023
1 parent 69a43d6 commit 2bb8597
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 27 deletions.
10 changes: 9 additions & 1 deletion Demo/Sources/Players/InlinePlayer/InlinePlayerViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,12 @@ final class InlinePlayerViewModel: ObservableObject {
}
}

extension InlinePlayerViewModel: PictureInPicturePersistable {}
extension InlinePlayerViewModel: PictureInPicturePersistable {
func pictureInPictureWillStart() {}

func pictureInPictureDidStart() {}

func pictureInPictureWillStop() {}

func pictureInPictureDidStop() {}
}
10 changes: 9 additions & 1 deletion Demo/Sources/Players/Player/PlayerViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,12 @@ final class PlayerViewModel: ObservableObject {
}
}

extension PlayerViewModel: PictureInPicturePersistable {}
extension PlayerViewModel: PictureInPicturePersistable {
func pictureInPictureWillStart() {}

func pictureInPictureDidStart() {}

func pictureInPictureWillStop() {}

func pictureInPictureDidStop() {}
}
10 changes: 9 additions & 1 deletion Demo/Sources/Players/SystemPlayer/SystemPlayerViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,12 @@ final class SystemPlayerViewModel: ObservableObject {
}
}

extension SystemPlayerViewModel: PictureInPicturePersistable {}
extension SystemPlayerViewModel: PictureInPicturePersistable {
func pictureInPictureWillStart() {}

func pictureInPictureDidStart() {}

func pictureInPictureWillStop() {}

func pictureInPictureDidStop() {}
}
6 changes: 6 additions & 0 deletions Demo/Sources/Showcase/Multi/MultiViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,13 @@ final class MultiViewModel: ObservableObject {
}

extension MultiViewModel: PictureInPicturePersistable {
func pictureInPictureWillStart() {}

func pictureInPictureDidStart() {
inactivePlayer.pause()
}

func pictureInPictureWillStop() {}

func pictureInPictureDidStop() {}
}
10 changes: 9 additions & 1 deletion Demo/Sources/Showcase/Playlist/PlaylistViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,12 @@ final class PlaylistViewModel: ObservableObject {
}
}

extension PlaylistViewModel: PictureInPicturePersistable {}
extension PlaylistViewModel: PictureInPicturePersistable {
func pictureInPictureWillStart() {}

func pictureInPictureDidStart() {}

func pictureInPictureWillStop() {}

func pictureInPictureDidStop() {}
}
9 changes: 1 addition & 8 deletions Sources/Analytics/UserInterface/PageViewTracking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,10 @@ public protocol PageViewTracking {
/// The Commanders Act page view data.
var commandersActPageView: CommandersActPageView { get }

/// A Boolean to enable or disable automatic tracking. Defaults to `true`.
/// A Boolean to enable or disable automatic tracking.
var isTrackedAutomatically: Bool { get }
}

public extension PageViewTracking {
/// The default automatic tracking setting.
var isTrackedAutomatically: Bool {
true
}
}

extension UIViewController {
static func setupViewControllerTracking() {
method_exchangeImplementations(
Expand Down
8 changes: 7 additions & 1 deletion Sources/Analytics/UserInterface/View.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import SwiftUI

private final class TrackerViewController: UIViewController, PageViewTracking {
private final class TrackerViewController: UIViewController {
let comScorePageView: ComScorePageView
let commandersActPageView: CommandersActPageView

Expand Down Expand Up @@ -46,3 +46,9 @@ public extension View {
}
}
}

extension TrackerViewController: PageViewTracking {
var isTrackedAutomatically: Bool {
true
}
}
14 changes: 0 additions & 14 deletions Sources/Player/Interfaces/PictureInPicturePersistable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,6 @@ public protocol PictureInPicturePersistable: AnyObject {
func pictureInPictureDidStop()
}

public extension PictureInPicturePersistable {
/// Default implementation. Does nothing.
func pictureInPictureWillStart() {}

/// Default implementation. Does nothing.
func pictureInPictureDidStart() {}

/// Default implementation. Does nothing.
func pictureInPictureWillStop() {}

/// Default implementation. Does nothing.
func pictureInPictureDidStop() {}
}

extension PictureInPicturePersistable {
/// The currently persisted instance, if any.
public static var persisted: Self? {
Expand Down
4 changes: 4 additions & 0 deletions Tests/AnalyticsTests/ComScore/ComScorePageViewTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ private class AutomaticMockViewController: UIViewController, PageViewTracking {
.init(name: pageName, type: "type")
}

var isTrackedAutomatically: Bool {
true
}

init(title: String? = nil) {
super.init(nibName: nil, bundle: nil)
self.title = title
Expand Down

0 comments on commit 2bb8597

Please sign in to comment.