-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ios][macos] Add MGLNetworkConfiguration.sessionDelegate #15128
Conversation
id<NSURLSessionDelegate> sessionDelegate = | ||
[MGLNetworkConfiguration sharedManager].sessionDelegate; | ||
|
||
session = [NSURLSession sessionWithConfiguration:sessionConfig delegate:sessionDelegate delegateQueue:nil]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since sessionWithConfiguration:delegate:delegateQueue:
keeps a strong reference to the delegate, I think we'll need to call invalidateAndCancel
or finishTasksAndInvalidate
in the destructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I can add a destructor to HTTPFileSource::Impl
and call invalidateAndCancel
on theNSURLSession
there, but how can I test it? Is there a way to tear down the stack? I have tried to stop the app, but the destructor does not seem to be called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay in responding - I believe we'll need to create a test for this. There are cpp tests for HTTPFileSource
and OnlineFileSource
which we might be able to take advantage of, but I think we'll need to create an integration test for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc: @tmpsantos for an interesting testing scenario (platform <-> core).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added: #15592
Thanks for this PR @halset - we will discuss and review! /cc @chloekraw @zugaldia |
@julianrex There is a merge-conflict in CHANGELOG.md, but I will await your discussion/review. |
Hi @halset, one question, would this help to recreate the session with a new configuration? But after the token is refreshed, I'm getting auth issues since I'm not able to recreate the session with a new configuration. |
Requires #15592 |
any news on this, or how to set a custom header to the request done by mapbox? On Android it is easy, on iOS somehow not.. |
This PR is still valid, but needs to be renewed/reviewed in light of recent changes. /cc @jmkiley |
Closing in favour of mapbox/mapbox-gl-native-ios#228 |
This is an attempt to solve #11888 by expanding
MGLNetworkConfiguration
with a property to set a customNSURLSessionDelegate
.My main motivation for doing this is to be able to use tile services that uses HTTP Basic Authentication.
This is a new version of #13491 as that was made before
MGLNetworkConfiguration
went public.