Skip to content

Conversation

@CaiJingLong
Copy link
Member

Signed-off-by: Caijinglong [email protected]

@CaiJingLong CaiJingLong changed the title Darwin-cancel-request feat: Add cancel token for darwin Nov 28, 2024
Signed-off-by: Caijinglong <[email protected]>
@github-actions
Copy link

Download apk from here for 76d7122

Signed-off-by: Caijinglong <[email protected]>
@github-actions
Copy link

Download apk from here for b653df7

# Conflicts:
#	CHANGELOG.md
#	pubspec.yaml
@github-actions
Copy link

Download apk from here for 400ae0d

@github-actions
Copy link

Download apk from here for ff1091f

@github-actions
Copy link

Download apk from here for 5c40973

@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Download apk from here for 12d2ddb

@AlexV525 AlexV525 requested a review from Copilot October 3, 2025 09:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds cancel token functionality for Darwin platforms (iOS/macOS), allowing users to cancel ongoing asset loading operations. The implementation introduces a PMCancelToken class and integrates it throughout the asset loading pipeline.

Key changes:

  • Introduces PMCancelToken class for managing cancellation requests
  • Adds cancelToken parameters to various asset loading methods
  • Implements platform-specific cancellation handling for iOS/macOS
  • Makes previously private methods public (getFile, getOriginBytes)

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/src/types/cancel_token.dart New file implementing the PMCancelToken class
lib/src/types/entity.dart Adds cancel token support to asset loading methods and makes some methods public
lib/src/managers/photo_manager.dart Adds static methods for cancelling requests
lib/src/internal/plugin.dart Implements cancel token handling and method channel modifications
lib/src/internal/enums.dart Adds cancel state to PMRequestState enum
lib/src/internal/constants.dart Adds cancel-related constants
lib/photo_manager.dart Exports the new cancel token class
ios/Classes/core/PMManager.m Implements iOS cancellation logic and request tracking
ios/Classes/core/PMManager.h Updates method signatures for cancel token support
ios/Classes/PMResultHandler.m Adds cancel token retrieval method
ios/Classes/PMResultHandler.h Updates interface to support cancel token functionality
ios/Classes/PMPlugin.m Integrates cancel token handling into plugin methods
README.md Documents the new cancel functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

/// [PhotoManagerPlugin.cancelRequest].
/// User don't need to use this.
@nonVirtual
String get key => _index.toString();
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key getter returns _index.toString() but should return index.toString(). This will always return '0' since _index is a static variable that gets incremented but the getter references the class-level static variable instead of the instance variable.

Suggested change
String get key => _index.toString();
String get key => index.toString();

Copilot uses AI. Check for mistakes.
Future<typed_data.Uint8List?> _getOriginBytes({
/// Obtain the raw data of the asset.
///
/// **Use it with cautious** since the original data might be epic large.
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'cautious' to 'caution'.

Suggested change
/// **Use it with cautious** since the original data might be epic large.
/// **Use it with caution** since the original data might be epic large.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Download apk from here for d0072d8

@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Download apk from here for 855b888

@AlexV525 AlexV525 merged commit 82bf2b8 into main Oct 3, 2025
13 checks passed
@AlexV525 AlexV525 deleted the darwin-cancel-request branch October 3, 2025 14:59
@HenrikH96
Copy link

HenrikH96 commented Oct 30, 2025

@AlexV525 @CaiJingLong

I have forked this repository to add some features like getAssetsGrouped.
So i can group my Assets based on Datetime.
I noticed that since this PR IOS crashed on regular bases.

[        ] [lldb]: * thread #62, queue = 'com.apple.root.user-interactive-qos', stop reason = EXC_BAD_ACCESS (code=1, address=0x11aa282b0)
[        ] [lldb]:     frame #0: 0x000000018ee3781c CoreFoundation`-[__NSDictionaryM objectForKeyedSubscript:] + 136
[        ] [lldb]: CoreFoundation`-[__NSDictionaryM objectForKeyedSubscript:]:
[        ] [lldb]: ->  0x18ee3781c <+136>: ldr    x0, [x21, x23, lsl #3]
[        ] [lldb]:     0x18ee37820 <+140>: cbz    x0, 0x18ee37870           ; <+220>
[        ] [lldb]:     0x18ee37824 <+144>: cmp    x0, x25
[        ] [lldb]:     0x18ee37828 <+148>: b.eq   0x18ee37844               ; <+176>
[        ] [lldb]:   thread #81, queue = 'com.apple.root.user-interactive-qos', stop reason = EXC_BAD_ACCESS (code=1, address=0x11aa28010)
[        ] [lldb]:     frame #0: 0x000000018edf44e4 CoreFoundation`mdict_rehashd + 160
[        ] [lldb]: CoreFoundation`mdict_rehashd:
[        ] [lldb]: ->  0x18edf44e4 <+160>: ldr    x23, [x22, x26, lsl #3]
[        ] [lldb]:     0x18edf44e8 <+164>: cmp    x23, #0x0
[        ] [lldb]:     0x18edf44ec <+168>: ccmp   x23, x27, #0x4, ne
[        ] [lldb]:     0x18edf44f0 <+172>: b.eq   0x18edf454c               ; <+264>
[        ] [lldb]: Target 0: (Runner) stopped.

I don't know exactly what is causing the problem, but it's definitely somewhere in this PR
I hope this may help development.

@AlexV525
Copy link
Member

@HenrikH96 This seems to be related to #1262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants