Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

ios: Support custom NSURLProtocol #12026

Closed
sebastianludwig opened this issue May 29, 2018 · 14 comments
Closed

ios: Support custom NSURLProtocol #12026

sebastianludwig opened this issue May 29, 2018 · 14 comments
Assignees
Labels
feature iOS Mapbox Maps SDK for iOS

Comments

@sebastianludwig
Copy link

sebastianludwig commented May 29, 2018

Platform: iOS
Mapbox SDK version: 4.0

Based on the discussion in #3597.

To modify the HTTP request headers or, in general, handle all sorts of remote requests, it's desirable to be able to use custom NSURLProtocol implementations. For this to work, we'd need a way to add NSURLProtocol classes to NSURLSessionConfiguration.protocolClasses (as described here).

How would we go about this? A delegate call? A class property?

I'm unfamiliar with the codebase but this is as I understand it so far:

  1. The NSURLSession is created in HTTPFileSource::Impl() in darwin/src/http_file_source.mm
  2. This is instantiated by DefaultFileSource::Impl() in platform/default/default_file_source.cpp
  3. which is called by SDKOfflineStorage-init in SDK/Foundation/Offline Maps/SDKOfflineStorage.mm (real path is darwin/src/MGLOfflineStorage.mm)
  4. This initialization happes due to [MGLOfflineStorage sharedOfflineStorage].mbglFileSource being accessed in
  5. MGLRendererConfiguration.fileSource in SDK/Foundation/MGLRendererConfiguration.mm (also darwin/src/)
  6. accessed in MGLMapView-commonInit in SDK/Kit/MGLMapView.mm (ios/src/)

So, MapView -> Renderer -> Offline Storage -> Default File Source -> HTTP File Source.

SDKOfflineStorage and MGLMapView do have delegates, but we can't use either because HTTPFileSource::Impl() is called during their initialization. We could add an additional parameter to an MGLMapView-init variation. However that would require to pass it through platform/default/default_source_file.cpp, which is undesirable, I guess. The only option I see would be a class property on...some class. HTTPFileSource doesn't have a ObjC pendant yet. Could we add one? Or have it access some other global state thing?

The Android solution is implemented in #10948. Would the equivalent be to have SDK/[Foundation|Kit]/MGLHTTPRequestUtil.{h,mm} and access that in HTTPFileSource::Impl()?

Edit: Is it even possible to access anything in the SDK part from the platform part?

I'm pretty much lost here. Any thoughts, @1ec5? Could you point me in a direction?

Also, are fonts/glyphs downloaded via the same mechanism?

@jmkiley jmkiley added the iOS Mapbox Maps SDK for iOS label May 29, 2018
@sebastianludwig
Copy link
Author

Having thought about it a little bit more, I think it'd be best if it'd be possible to provide the whole NSURLSessionConfiguration.

Still no clue how to go about this, though oO ARCHITECTURE.md affirms my understanding that the SDK uses and accesses the C++ core, not the other way around. Could we have a NSURLSessionConfiguration setter on...MGLOfflineStorage that somehow accesses the used HTTPFileSource::Impl and passes the given value through? HTTPFileSource::Impl would then need to re-initialize its session.

@Woody4618
Copy link

Would be really nice to have this.

@1ec5
Copy link
Contributor

1ec5 commented Jun 21, 2018

Is it even possible to access anything in the SDK part from the platform part?

Not from the SDK’s public API, because the SDK is intended to be compatible with pure Objective-C projects, whereas mbgl’s public API is entirely in C++. It is possible for the SDK’s internal code to interact with mbgl’s public API, but the networking code is internal even to mbgl.

the SDK uses and accesses the C++ core, not the other way around

This is true for the most part. mbgl does have some platform-specific Objective-C++ code at the very lowest level for working with NSURLSession.

@kkaefer, do you think we could expose a way for developers to customize NSURLSessionConfiguration? I think this would be one important step towards resolving #4291 as well.

/cc @julianrex

@julianrex
Copy link
Contributor

I think it's also worth looking at what's happening over in the Android version re customizing networking.

@stale
Copy link

stale bot commented Dec 19, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Dec 19, 2018
@sebastianludwig
Copy link
Author

This might be a way to go forward: #13491

@fabian-guerra fabian-guerra reopened this Jan 31, 2019
@stale
Copy link

stale bot commented Jan 31, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Jan 31, 2019
@friedbunny
Copy link
Contributor

Stay open, plz.

@friedbunny friedbunny reopened this Feb 1, 2019
@stale
Copy link

stale bot commented Feb 1, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Feb 1, 2019
@sebastianludwig
Copy link
Author

sebastianludwig commented Feb 1, 2019

@fabian-guerra is there a label we can add so the stale bot ignores it? (and remove the "archived" label)

Edit: No, there currently isn't. So the project would need a new label and a stale.yml PR first.

@friedbunny
Copy link
Contributor

Still, stalebot seems to be malfunctioning — unsure why it’s trying to continually close issues after mere hours of inactivity. /cc @tmpsantos

@friedbunny friedbunny reopened this Feb 1, 2019
@friedbunny friedbunny removed the archived Archived because of inactivity label Feb 1, 2019
@fabian-guerra fabian-guerra self-assigned this Feb 1, 2019
@fabian-guerra
Copy link
Contributor

fabian-guerra commented Feb 1, 2019

I'm going to assign this to me for now.

@tmpsantos
Copy link
Contributor

Still, stalebot seems to be malfunctioning — unsure why it’s trying to continually close issues after mere hours of inactivity. /cc @tmpsantos

It is because the label archived was not removed. The bot will mark as archived after detecting the inactivity and then close.

@BenderNK
Copy link

BenderNK commented Feb 19, 2019

For those of you who land on this page and looking for similar capability on the Android side, read this ticket:

#13835

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

No branches or pull requests

9 participants