Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[iOS] Improve new playlist UI responsiveness #26726

Merged
merged 1 commit into from
Nov 25, 2024
Merged

[iOS] Improve new playlist UI responsiveness #26726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistLegacyCarplayController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ extension PlaylistLegacyCarplayController {
let cacheState = PlaylistManager.shared.state(for: item.tagId)
if cacheState != .invalid {
if let index = PlaylistManager.shared.index(of: item.tagId),
let asset = PlaylistManager.shared.assetAtIndex(index)
let asset = PlaylistManager.shared.assetAtIndexSynchronous(index)
{

do {
Expand Down
2 changes: 1 addition & 1 deletion ...rave-ios/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ extension PlaylistViewController: VideoViewDelegate {
let cacheState = PlaylistManager.shared.state(for: item.tagId)
if cacheState != .invalid {
if let index = PlaylistManager.shared.index(of: item.tagId),
let asset = PlaylistManager.shared.assetAtIndex(index)
let asset = PlaylistManager.shared.assetAtIndexSynchronous(index)
{

do {
Expand Down
42 changes: 31 additions & 11 deletions ios/brave-ios/Sources/Playlist/PlaylistDownloadManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ protocol PlaylistDownloadManagerDelegate: AnyObject {

private protocol PlaylistStreamDownloadManagerDelegate: AnyObject {
// TODO: Should be async, fix when removing legacy playlist UI
func localAsset(for itemId: String) -> AVURLAsset?
func localAssetSynchronous(for itemId: String) -> AVURLAsset?
func localAsset(for itemId: String) async -> AVURLAsset?
func onDownloadProgressUpdate(streamDownloader: Any, id: String, percentComplete: Double)
func onDownloadStateChanged(
streamDownloader: Any,
Expand Down Expand Up @@ -229,7 +230,8 @@ public class PlaylistDownloadManager: PlaylistStreamDownloadManagerDelegate {

// MARK: - PlaylistStreamDownloadManagerDelegate

func localAsset(for itemId: String) -> AVURLAsset? {
@available(*, deprecated, renamed: "localAsset(for:)", message: "Use async version")
func localAssetSynchronous(for itemId: String) -> AVURLAsset? {
guard let item = PlaylistItem.getItem(uuid: itemId),
let cachedData = item.cachedData,
!cachedData.isEmpty
Expand All @@ -253,6 +255,30 @@ public class PlaylistDownloadManager: PlaylistStreamDownloadManagerDelegate {
}
}

func localAsset(for itemId: String) async -> AVURLAsset? {
let cachedData = await MainActor.run {
return PlaylistItem.getItem(uuid: itemId)?.cachedData
}
guard let cachedData = cachedData, !cachedData.isEmpty else { return nil }

var bookmarkDataIsStale = false
do {
let url = try URL(
resolvingBookmarkData: cachedData,
bookmarkDataIsStale: &bookmarkDataIsStale
)

if bookmarkDataIsStale {
return nil
}

return AVURLAsset(url: url, options: AVAsset.defaultOptions)
} catch {
Logger.module.error("\(error.localizedDescription)")
return nil
}
}

fileprivate func onDownloadProgressUpdate(
streamDownloader: Any,
id: String,
Expand Down Expand Up @@ -536,9 +562,7 @@ private class PlaylistHLSDownloadManager: NSObject, AVAssetDownloadDelegate {
// HLS streams can be in two spots, we need to delete from both
// just in case the download process was in the middle of transferring the asset
// to its proper location
if let cacheLocation = await MainActor.run(body: {
delegate?.localAsset(for: asset.id)?.url
}) {
if let cacheLocation = await delegate?.localAsset(for: asset.id)?.url {
do {
try await AsyncFileManager.default.removeItem(at: cacheLocation)
} catch {
Expand Down Expand Up @@ -741,9 +765,7 @@ private class PlaylistFileDownloadManager: NSObject, URLSessionDownloadDelegate
if let error = error as NSError? {
switch (error.domain, error.code) {
case (NSURLErrorDomain, NSURLErrorCancelled):
if let cacheLocation = await MainActor.run(body: {
delegate?.localAsset(for: asset.id)?.url
}) {
if let cacheLocation = await delegate?.localAsset(for: asset.id)?.url {
do {
try await AsyncFileManager.default.removeItem(at: cacheLocation)
PlaylistItem.updateCache(uuid: asset.id, pageSrc: asset.pageSrc, cachedData: nil)
Expand Down Expand Up @@ -1064,9 +1086,7 @@ private class PlaylistDataDownloadManager: NSObject, URLSessionDataDelegate {
if let error = error as NSError? {
switch (error.domain, error.code) {
case (NSURLErrorDomain, NSURLErrorCancelled):
if let cacheLocation = await MainActor.run(body: {
delegate?.localAsset(for: asset.id)?.url
}) {
if let cacheLocation = await delegate?.localAsset(for: asset.id)?.url {
Task {
do {
try await AsyncFileManager.default.removeItem(at: cacheLocation)
Expand Down
Loading
Loading