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

#5 do not suspend webserver when sent to background if background mode is enabled in config.xml #41

Closed
wants to merge 6 commits into from

Conversation

ghenry22
Copy link
Contributor

@ghenry22 ghenry22 commented Nov 6, 2017

This does not prevent the operating system from suspending the app (which will in turn suspend the GCDwebserver).

What this does is stop the app from immediately suspending the webserver when sent to the background, so if you have an app that needs access to files / images / whatever when in the background it can still access them.

In my case I am working on a music player app which needs access to cover art images when playing music in the background for the OS remote controls and lock screen so I don't want the webserver to suspend in the background immediately or I get broken images.

I'm not sure how to link this to a variable in config.xml but this PR adds in the native option to not automatically suspend.

@BuddyLReno
Copy link

@ghenry22 I seem to be having this issue which is probably what is causing my own issues with the cordova music controls plugin. I'm going to try your branch here to see if it resolves it.

@ghenry22
Copy link
Contributor Author

ghenry22 commented Nov 7, 2017

@BuddyLReno I also submitted a PR for the music controls project yesterday which fixed a bunch of other issues I had with it on ios11.

But yes without this setting on wkwebview you won’t get any artwork when in background.

@ghenry22
Copy link
Contributor Author

ghenry22 commented Nov 9, 2017

@BuddyLReno upon further investigation the music controls plugin actually fetches the cover art using native code so it doesn't need this, have just been running some debugging.

However, I have been testing this PR and have found that after a certain IDLE time in the background the GCDwebserver will still stop accepting requests. So if you are making requested to the webserver every now and then it's all good and stays alive but after a period with no requests it seems to suspend the webserver even though the app is still running.

There are a couple of issues logged with GCDWebserver project around how to handle this state when the app is brought back to the foreground so I am going to:

  1. test the proposed solutions for when an app is brought back to foreground to check state and reset sockets if needed.
  2. update this PR to tie background mode to a config.xml variable as this should probably not be enabled by default.
  3. implement the ConnectedStateCoalescingInterval with a sensible value so that when the webview goes to the background it will wait for say 30 seconds of inactivity before suspending rather than doing so the second there isn't an active connection.

Apply patches to GCDWebserver.m for detecting socket errors on return to foreground due to change in state / suspend / error when in background.
@ghenry22
Copy link
Contributor Author

ghenry22 commented Nov 9, 2017

Upon testing I found that with suspendautomaticallyinbackground disabled, IF the app was suspended by the OS while in the background the GCDwebserver would also be suspended and when the app is brought back into the foreground GCDwebserver has a socket error and will not accept new inbound connections.

There is a lot of discussion of exactly this behaviour when using wkwebview plugin with meteor and there were a couple of proposed PR's to detect socket errors and restart the webserver if needed.

To start with I updated to latest GCDwebserver version 4.3.2 (as at 9.Nov.17).

I took one of these, cleaned it up a little further and implemented it against the latest GCDwebserver code base and it completely resolves this issue. Now if automaticallysuspendinbackground is set to NO and the app is eventually suspended due to inactivity when it comes back to the foreground GCDwebserver checks for socket errors and if found restarts the server and everything works as expected!

if WKBackgroundEnabled is set to true in config.xml then the webserver will not automatically suspend when app is sent to background.
If it is not set then the webserver will suspend in background.
add ConnectedStateCoalescingInterval with a default of 10 meaning the webserver will wait for 10 seconds after the last HTTP request before stopping its background task. Gives requests a little leeway to finish when app is sent to background.
When background mode is enabled this is extended to 60s.
Moved GCDwebserver init to it's own function which can be called after config values are available.
This will also facilitate user selectable port and host name in config.xml in future
@ghenry22
Copy link
Contributor Author

ghenry22 commented Nov 9, 2017

Ok so this should be good to go. This PR now does the following:

Add preference to config.xml
WKEnableBackground
defaults to false

if set to true then set GCDWebserver to NOT suspend when the app is sent to the background
If not set or set to false then set GCDWebserver to suspend when the app is sent to the background

Added ConnectedStateCoalescingInterval value for GCDWebserver with a value of 10s by default and 60s if background mode is enabled. By default this value is 0.

What this means is that after the last HTTP connection closes GCDwebserver will wait 10 seconds before ending it's background task. This just gives a little more leeway upon being send to the background for any requests to complete even without background mode.

Was changed for testing purposes, changing back to secure default
add preference WKPort in your config.xml and set it to the port number you want to use.
If not set it will default to 8080.
Remember to update your allows-navigation entry in config.xml to also reflect the new port number.
@ghenry22
Copy link
Contributor Author

add port selection as a preference WKPort in config.xml
just ser preference WKPort = 8181 for example. Defaults to 8080 if nothing is supplied.

this change is dependant upon the earlier changes for background mode GCDwebserver so I have included it in this PR.

Resolves #40

@ghenry22 ghenry22 changed the title #5 do not suspend webserver when sent to background #5 do not suspend webserver when sent to background if background mode is enabled in config.xml Nov 27, 2017
@cfmjohn cfmjohn mentioned this pull request Jan 17, 2018
@imsingh
Copy link

imsingh commented Feb 19, 2018

Allowing to configure port is a really handy thing. I would love this feature to be added to the plugin. Can anyone give an update on it?

Thanks.

@ghenry22
Copy link
Contributor Author

@imsingh you can just use my fork of this plugin and it has the changes merged for port selection and background operation and a bunch of other fixes.

add the plugin from this git url: https://github.com/ghenry22/cordova-plugin-ionic-webview

@mlynch
Copy link
Contributor

mlynch commented Jun 8, 2018

Can someone make a new PR that cleans up this commit and then removes those files that changed size but didn't change content?

@ghenry22
Copy link
Contributor Author

ghenry22 commented Jun 9, 2018

As per the other thread where we were discussing, I will create a new PR to clean things up so it can be merged

@jacquesdev
Copy link
Contributor

@ghenry22 - I will need your changes in the next few days, so just wanted to check how far you are with the new PR. I'm happy to create one based on these changes unless you are just about done with it. Please let me know.

@ghenry22
Copy link
Contributor Author

Pretty much done, I will be able to submit finalised PR by tomorrow no problem.

@mlynch
Copy link
Contributor

mlynch commented Jun 18, 2018

Cool, closing this PR for now then

@mlynch mlynch closed this Jun 18, 2018
@ghenry22
Copy link
Contributor Author

@mlynch FYI PR #117 is submitted. All previous issues resolved, no conflicts and all up to date good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants