feat: update media content time spent calculations with ad breaks

[nickolas-dimitrakas]

Background

• Per this doc, Currently, the mParticle Media SDK requires developers to manually manage content time tracking during ad breaks, creating additional complexity and potential for error. If not managed appropriately, the media content time spent value is overstated with ad break time. This PR adds an (optional) ability to allow pausing mediaContentTimeSpent when an Ad Break starts and ends

What Has Changed

• Added excludeAdBreaksFromContentTime flag to MPMediaSession.
• Updated logAdBreakStart and logAdBreakEnd to pause/resume content time tracking when the flag is enabled.
• Added private helpers pauseContentTimeIfAdBreakExclusionEnabled() and resumeContentTimeIfAdBreakExclusionEnabled().
• Added unit tests for flag behavior and time exclusion accuracy.

Checklist

• I have performed a self-review of my own code.
• I have made corresponding changes to the documentation.
• I have added tests that prove my fix is effective or that my feature works.
• I have tested this locally.

Additional Notes

• Related to feat: Pause mediaContentTimeSpent calculation on ad breaks mparticle-web-media-sdk#777
• Same feature will be added to Android and Roku

Reference Issue (For employees only. Ignore if you are an outside contributor)

• Closes https://rokt.atlassian.net/browse/SDKE-447

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[denischilik]

LGTM

[BrandonStalnaker]

Looks good

[samdozor]

Likely issues introduced

• Auto-resume on ad break end can start content time when it wasn’t playing
  • If content was already paused before an ad break starts, ending the ad break will incorrectly start content time tracking.

// MARK: private helpers (ad break)
private func pauseContentTimeIfAdBreakExclusionEnabled() {
    guard excludeAdBreaksFromContentTime, currentPlaybackStartTimestamp != nil else { return }
    storedPlaybackTime += Date().timeIntervalSince(currentPlaybackStartTimestamp!)
    currentPlaybackStartTimestamp = nil
}

private func resumeContentTimeIfAdBreakExclusionEnabled() {
    guard excludeAdBreaksFromContentTime, currentPlaybackStartTimestamp == nil else { return }
    currentPlaybackStartTimestamp = Date()
}

• Problem: resume... only checks that currentPlaybackStartTimestamp == nil, not that the pause was caused by an ad break. If the user paused manually before the ad break, logAdBreakEnd() will set a new start timestamp and begin counting content time even though playback is still paused.

• Fix: Track an explicit state flag (e.g., pausedByAdBreak) that is set by pauseContentTimeIfAdBreakExclusionEnabled() and only resume if that flag is true, then clear it.

• Ad time is double-counted

  • mediaTotalAdTimeSpent is incremented both on ad end/skip and again in the ad summary logger.

@objc public func logAdEnd(options: Options?  = nil) {
    if (self.adContent?.adStartTimestamp != nil) {
        self.adContent?.adEndTimestamp = Date()
        self.adContent?.adCompleted = true
        self.mediaTotalAdTimeSpent += self.adContent!.adEndTimestamp!.timeIntervalSince(self.adContent!.adStartTimestamp!)
    }
    ...
    self.logAdSummary()
}

private func logAdSummary() {
    if (self.adContent != nil) {
        if (self.adContent?.adStartTimestamp != nil) {
            self.adContent?.adEndTimestamp = Date()
            self.mediaTotalAdTimeSpent += self.adContent!.adEndTimestamp!.timeIntervalSince(self.adContent!.adStartTimestamp!)
        }
        ...
    }
}

• Impact: Ad time gets added twice. Remove the increment in logAdSummary() or skip if already accounted for.

• Minor: redundant assignment

  • excludeAdBreaksFromContentTime is set twice in the long initializer.

self.excludeAdBreaksFromContentTime = excludeAdBreaksFromContentTime
...
if ( 100 >= completeLimit && completeLimit > 0) {
    self.mediaContentCompleteLimit = completeLimit
}
self.excludeAdBreaksFromContentTime = excludeAdBreaksFromContentTime

• Fix: Remove the duplicate set.

Tests

• The new tests properly use short sleeps with tolerances and validate both include/exclude behaviors:

func testAdBreakExclusionDisabledDoesNotPauseContentTime() { ... }

func testAdBreakExclusionEnabledExcludesAdTime() { ... }

• They are unlikely to be flaky due to the accuracy windows used (0.08–0.1).

• Add a test covering “ad break while paused” to catch the auto-resume bug:

  • Pause content, start ad break, end ad break, assert currentPlaybackStartTimestamp is still nil and mediaContentTimeSpent unchanged.

• Optional: Consider removing timing from unit tests by injecting a time source, but the current tolerances are reasonable.

Summary:

• Ad-break resume should only happen if the ad break actually paused content; add a pausedByAdBreak flag to gate resume.
• mediaTotalAdTimeSpent is double-counted; remove one of the increments (recommend removing the one in logAdSummary()).
• Clean up a redundant assignment in the long initializer.
• Tests look solid with tolerances; add a case for “ad break while paused” to prevent regressions.

[nickolas-dimitrakas]

@samdozor - Thanks for the detailed review and for calling out these issues.

private func logAdSummary() {
    if (self.adContent != nil) {
        if (self.adContent?.adStartTimestamp != nil) {
            self.adContent?.adEndTimestamp = Date()
            self.mediaTotalAdTimeSpent += self.adContent!.adEndTimestamp!.timeIntervalSince(self.adContent!.adStartTimestamp!)
        }
        ...
        customAttributes[adContentStartTimestampKey] = self.adContent?.adStartTimestamp
    }
}

One thing I noticed as well: in the scenario where adContent.adStartTimestamp is nil, I’m wondering whether we should be running any of the logAdSummary logic at all. In the current implementation, logAdSummary() will still set an end timestamp and attempt to track ad time even though there was never a recorded start. It seems like we may want to fully skip the summary logic when adStartTimestamp is nil rather than partially executing it.

cc @BrandonStalnaker

[BrandonStalnaker]

@samdozor - Thanks for the detailed review and for calling out these issues.

private func logAdSummary() {
    if (self.adContent != nil) {
        if (self.adContent?.adStartTimestamp != nil) {
            self.adContent?.adEndTimestamp = Date()
            self.mediaTotalAdTimeSpent += self.adContent!.adEndTimestamp!.timeIntervalSince(self.adContent!.adStartTimestamp!)
        }
        ...
        customAttributes[adContentStartTimestampKey] = self.adContent?.adStartTimestamp
    }
}

One thing I noticed as well: in the scenario where adContent.adStartTimestamp is nil, I’m wondering whether we should be running any of the logAdSummary logic at all. In the current implementation, logAdSummary() will still set an end timestamp and attempt to track ad time even though there was never a recorded start. It seems like we may want to fully skip the summary logic when adStartTimestamp is nil rather than partially executing it.

cc @BrandonStalnaker

That's tough to say because it really depends on how the customer implements this. I'd lean towards logging what we can considering the only places we call the method is if adContent is set and if the class is deallocated or the ad is skipped or ended. In those cases I would assume the customer would want some reference of the ad they initialized even if they're not implementing the timestamps.