Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Avoid duplicate offline pack completion notifications #421

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ZiZasaurus
Copy link
Contributor

Fixes #419

@1ec5 1ec5 changed the title Updating existing offline example to fix #419 Avoid duplicate offline pack completion notifications Nov 17, 2020
deinit {
// Remove offline pack observers.
print("Removing offline pack notification observers")
NotificationCenter.default.removeObserver(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

It’s still important to clean up observations when deinitializing (or when the view controller is about to disappear, depending). The application may allow the user to navigate away from the offline pack–downloading view controller before the download completes. If that happens, the view controller shouldn’t leak memory due to a lingering observation.

Comment on lines 105 to 107
NotificationCenter.default.removeObserver(self, name: NSNotification.Name.MGLOfflinePackProgressChanged, object: nil)
NotificationCenter.default.removeObserver(self, name: NSNotification.Name.MGLOfflinePackError, object: nil)
NotificationCenter.default.removeObserver(self, name: NSNotification.Name.MGLOfflinePackMaximumMapboxTilesReached, object: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Each of these observations would be worth removing during deinitialization too. Similar to what you did with the calls to addObserver(_:selector:name:object:), you can factor these calls to removeObserver(_:name:object:) into a separate method for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oddly enough, changing completedResources == expectedResources to pack.state == .complete resolves the duplicate notification in this example

@ZiZasaurus ZiZasaurus requested a review from 1ec5 November 18, 2020 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Completed Resources test for Offline Pack fires more than once in OfflinePackExample.swift
2 participants