[ios, macos]Remove iOS/macOS codes from native codes #16031

m-stephen · 2019-12-09T16:47:01Z

Refactor of http_file_source.mm, using a delegate & interface to separate platform codes(macOS, iOS) from native codes, to reach our goal of modular SDK.

platform/darwin/src/http_file_source.mm

fabian-guerra

I'm going to summarize our hangout conversation into the following changes:

@tmpsantos could you please advice how can we use the new build system to build this library?
Move MGLNativeInterfaceReceiver.h/m from ios/macos to platform/darwin and import this file into each project.
Rename MGLNativeInterfaceReceiver to: MGLNetworkIntegrationManager
Rename MGLNativeAppleInterfaceTransmitter to: MGLNativeNetworkDelegate
Once this PR lands CP the changes related to the platform code to gl-native-ios

tmpsantos · 2019-12-10T12:39:56Z

@tmpsantos could you please advice how can we use the new build system to build this library?

Just update the list of files here:

next/platform/macos/macos.cmake
next/platform/ios/ios.cmake

julianrex · 2019-12-10T15:06:14Z

platform/darwin/src/http_file_source.mm

@@ -88,29 +83,29 @@ void cancel() {
 public:
    Impl() {
        @autoreleasepool {
-            NSURLSessionConfiguration *sessionConfig = [MGLNetworkConfiguration sharedManager].sessionConfiguration;
+            NSURLSessionConfiguration *sessionConfig = MGLNativeAppleInterfaceTransmitter.shared.sessionConfiguration;


Longer-term, I would prefer to avoid having & going via a singleton here. @tmpsantos is there an existing core object that we could set a property on?

Perhaps using a notification (& object) to request the client sets a weak delegate here?

This comment is general, and shouldn't block the PR.

Not at the moment. But if we move at some point this abstraction to not be at the "request" level and rather at the "network manager" level it will probably solve the problem, because session becomes a property of the "network manager".

platform/darwin/include/mbgl/interface/native_apple_interface.h

m-stephen · 2019-12-12T10:28:02Z

Just update the list of files here:

next/platform/macos/macos.cmake
next/platform/ios/ios.cmake

@tmpsantos Could help to show how can I use this new build system to avoid generating sdk-files.json? It seems ios.cmake only configure files in mbgl-core.
cc: @fabian-guerra

fabian-guerra

This is looking good. Please address the comments I left. The test is not passing because the test target is not setting the delegate and returns a nil NSURLSessionConfiguration.

fabian-guerra · 2019-12-13T00:08:23Z

next/platform/macos/macos.cmake

@@ -88,6 +88,8 @@ target_sources(
        ${MBGL_ROOT}/platform/darwin/src/run_loop.cpp
        ${MBGL_ROOT}/platform/darwin/src/string_nsstring.mm
        ${MBGL_ROOT}/platform/darwin/src/timer.cpp
+        ${MBGL_ROOT}/platform/darwin/src/native_apple_interface.m


Please add this files to next/platform/ios/ios.cmake

If we are using two building systems on iOS & mac OS, it shouldn't be added.

Hmm..it looks like this was needed after all?

Yep, looks like that. We need add all *.m files into ios.cmake, should we make another PR for this?

platform/darwin/include/mbgl/interface/native_apple_interface.h

platform/darwin/src/MGLNetworkIntegrationManager.m

platform/darwin/src/http_file_source.mm

platform/ios/CHANGELOG.md

…un tests

fabian-guerra

Thank you for making these changes. The next step is bring the darwing changes into gl-native-ios.
/cc @julianrex

* add source/header * add ios files * add configs * modify name * http_file_source * add interface delegate when map init * fix name * fix delegate name * support mac os * add mac os support * make optional delegate when mac os * mac/ios difference * add ios change log * cancel iOS/mac OS judgement * cancel iOS/mac OS judgement * cancel judgement in .m * update * update * update http_file_source * update ios * update mac os * add mac os file * add mac os file to `.cmake` * change names * add log & fix format * reset changelog commit * update changelog * rename iOS network manager * Add a test configuration(same as default configuration) when mac os run tests * re-add account type into `http_file_source` * refactor

m-stephen added 8 commits December 9, 2019 23:49

add source/header

3d91301

add ios files

32b55c1

add configs

99f0738

modify name

bc3fb0b

http_file_source

ca72384

add interface delegate when map init

c09c622

fix name

810cefd

fix delegate name

37d88fe

m-stephen requested review from tmpsantos, fabian-guerra and julianrex December 9, 2019 16:47

m-stephen requested a review from 1ec5 as a code owner December 9, 2019 16:47

m-stephen requested a review from a team December 9, 2019 16:47

m-stephen self-assigned this Dec 9, 2019

m-stephen added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor labels Dec 9, 2019

support mac os

f9dfa38

m-stephen removed request for 1ec5 and a team December 9, 2019 16:56

m-stephen added 4 commits December 10, 2019 01:01

add mac os support

65e7c29

make optional delegate when mac os

dfa567e

mac/ios difference

c1301ec

add ios change log

ecf76fe

m-stephen mentioned this pull request Dec 9, 2019

Remove platform code from the network platform. mapbox/mapbox-gl-native-ios#51

Closed

m-stephen added 3 commits December 10, 2019 01:39

cancel iOS/mac OS judgement

5cd5aff

cancel iOS/mac OS judgement

f0df7ba

cancel judgement in .m

357c8f7

tmpsantos reviewed Dec 9, 2019

View reviewed changes

platform/darwin/src/http_file_source.mm Outdated Show resolved Hide resolved

fabian-guerra reviewed Dec 10, 2019

View reviewed changes

julianrex reviewed Dec 10, 2019

View reviewed changes

m-stephen added 4 commits December 12, 2019 18:04

update

1c4c436

update

29397ff

update http_file_source

a72e651

update ios

f72d5f5

m-stephen added 3 commits December 12, 2019 18:50

update mac os

0f07dd3

add mac os file

82cf430

add mac os file to .cmake

0a5e2d1

fabian-guerra suggested changes Dec 13, 2019

View reviewed changes

m-stephen added 8 commits December 13, 2019 15:36

change names

6aeb46c

add log & fix format

17d3e53

reset changelog commit

2b79561

update changelog

d5c9171

rename iOS network manager

2c5415d

Add a test configuration(same as default configuration) when mac os r…

3afd98b

…un tests

re-add account type into http_file_source

84af097

refactor

aabc7e9

m-stephen requested review from fabian-guerra, tmpsantos and julianrex December 13, 2019 10:17

fabian-guerra approved these changes Dec 14, 2019

View reviewed changes

m-stephen merged commit f9b7284 into master Dec 14, 2019

m-stephen deleted the stephen-remove-ios-codes branch December 14, 2019 05:38

julianrex mentioned this pull request Dec 18, 2019

Cherry pick #16031 to release-tequila #16076

Closed

captainbarbosa mentioned this pull request Jan 8, 2020

Preparations for ios-v5.7.0-alpha.1 mapbox/mapbox-gl-native-ios#139

Merged

jmkiley mentioned this pull request Jan 9, 2020

Add files from #16031 mapbox/mapbox-gl-native-ios#140

Merged

1ec5 mentioned this pull request Feb 27, 2020

Mapbox GL Native v1.3.0 mapbox/mapbox-gl-native-ios#181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios, macos]Remove iOS/macOS codes from native codes #16031

[ios, macos]Remove iOS/macOS codes from native codes #16031

m-stephen commented Dec 9, 2019

fabian-guerra left a comment

tmpsantos commented Dec 10, 2019

julianrex Dec 10, 2019

tmpsantos Dec 11, 2019

m-stephen commented Dec 12, 2019

fabian-guerra left a comment

fabian-guerra Dec 13, 2019

m-stephen Dec 13, 2019

captainbarbosa Jan 9, 2020

m-stephen Jan 9, 2020

fabian-guerra left a comment

[ios, macos]Remove iOS/macOS codes from native codes #16031

[ios, macos]Remove iOS/macOS codes from native codes #16031

Conversation

m-stephen commented Dec 9, 2019

fabian-guerra left a comment

Choose a reason for hiding this comment

tmpsantos commented Dec 10, 2019

julianrex Dec 10, 2019

Choose a reason for hiding this comment

tmpsantos Dec 11, 2019

Choose a reason for hiding this comment

m-stephen commented Dec 12, 2019

fabian-guerra left a comment

Choose a reason for hiding this comment

fabian-guerra Dec 13, 2019

Choose a reason for hiding this comment

m-stephen Dec 13, 2019

Choose a reason for hiding this comment

captainbarbosa Jan 9, 2020

Choose a reason for hiding this comment

m-stephen Jan 9, 2020

Choose a reason for hiding this comment

fabian-guerra left a comment

Choose a reason for hiding this comment