Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GTMSessionUploadFetcher: respect GTMSESSION_RECONNECT_BACKGROUND_SESSIONS_ON_LAUNCH flag #219

Merged
merged 2 commits into from
Dec 1, 2020

Conversation

maksymmalyhin
Copy link
Contributor

@maksymmalyhin maksymmalyhin commented Nov 27, 2020

Fix #218.

  • replicate approach used in GTMSessionFetcher to reduce app startup performance footprint.

@mwyman I didn't really do a lot of testing, so additional verification from your end is appreciated.

@maksymmalyhin
Copy link
Contributor Author

maksymmalyhin commented Nov 27, 2020

@mwyman I also have a question as for the default value of the flag GTMSESSION_RECONNECT_BACKGROUND_SESSIONS_ON_LAUNCH defined here. What is the reason for the flag being set to 1 only for iOS 13+ devices? From the first glance it looks like it should be preferable behaviour for all iOS, tvOS, and macOS devices and probably watchOS as well (though may require additional code changes to support watchOS case).

@mwyman
Copy link
Contributor

mwyman commented Nov 30, 2020

@mwyman I also have a question as for the default value of the flag GTMSESSION_RECONNECT_BACKGROUND_SESSIONS_ON_LAUNCH defined here. What is the reason for the flag being set to 1 only for iOS 13+ devices? From the first glance it looks like it should be preferable behaviour for all iOS, tvOS, and macOS devices and probably watchOS as well (though may require additional code changes to support watchOS case).

Since this was a change in behavior, we gated it on using the latest target OS at the time to avoid an unexpected change in launch behavior; was considered (right or wrong) that projects that had targeted the latest OS at that time were staying more on-top of things.

@maksymmalyhin
Copy link
Contributor Author

Since this was a change in behavior, we gated it on using the latest target OS at the time to avoid an unexpected change in launch behavior; was considered (right or wrong) that projects that had targeted the latest OS at that time were staying more on-top of things.

I see, that makes sense.

We would prefer to have it as a default for all Firebase users but it's a bit difficult (not sure if possible at all) to set up without an extra step for developers. I guess, the default behaviour may be updated in conjunction with the major the version bump which will indicate the braking change for the library users. I assume most of the users lock the dependency on a particular major version. WDYT?

@@ -2,7 +2,7 @@
# to import GTMSessionFetcher via the CocoaPods dependency Manager.
Pod::Spec.new do |s|
s.name = 'GTMSessionFetcher'
s.version = '1.5.0'
s.version = '1.6.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't bump the version number in this PR. We like to keep that separate from individual code changes.

@mwyman mwyman merged commit 3e6489e into google:master Dec 1, 2020
@maksymmalyhin maksymmalyhin deleted the mm/load-optimization branch December 1, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

+[GTMSessionUploadFetcher load] method takes too much time at app startup
2 participants