turbolinks / turbolinks-ios

Native iOS adapter for building hybrid apps with Turbolinks 5
MIT License
880 stars 92 forks source link

Make sure finishRequest() is always called for cold boot visits #154

Open calleluks opened 5 years ago

calleluks commented 5 years ago

Hello and thanks for a great framework!

In a Turbolinks app I'm currently working on, we display multiple navigation controllers in a tab bar controller. Each navigation controller has their own Session but share the same WKProcessPool.

When showing and hiding the network activity indicator in response to receiving sessionDidStartRequest(_:) and sessionDidEndRequest(_:) messages, I found that sessionDidEndRequest(_:) wasn't called if I switched to a different tab before the request hda finished. Since we're using reference counting to show and hide the indicator, this caused the indicator to never be hidden.

In 11d71c3, I have made some changes that make this easier to reproduce: just start the demo app, switch to the second tab, and watch the printed messages in the Xcode debugger output pane.

We should make sure to remove this commit before merging if you decide doing so makes sense.

The reason sessionDidEndRequest(_:) isn't called is the the WKWebView won't send the webView(_:didFinish:) message to its navigationDelegate (i.e. the ColdBootVisit) after switching to a different tab. I don't know why it doesn't do so and can't find any documentation describing this behavior.

In 4515ceb, I add a call to finishRequest() in ColdBootVisit.completeVisit() to fix this problem. Since finishRequest() only calls sessionDidEndRequest(_:) once and we know that the request must have finished at this point I think this should be safe.

Let me know what you think!

henrik commented 4 years ago

Pinging the Turbolinks maintainers. I've worked with Calle on this project and we will soon pick it up again :)