This repository has been archived by the owner on Sep 25, 2021. It is now read-only.
Make sure finishRequest() is always called for cold boot visits #154
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When doing a cold boot visit, `finishRequest()` was previously only called in response to receiving the `webView(_:didFinish:)` `WKNavigationDelegate` message. If the web view wasn't on screen when the navigation finished—for example after the user switched to another tab in a parent `UITabViewController`—`webView(_:didFinish:)` would not be called, causing the `VisitDelegate` to never be sent the `visitRequestDidFinish(_:)` message. Since `finishRequest()` will only call `visitRequestDidFinish(_:)` once, it should be safe to call it again in `completeVisit()` at which point the request must have finished since the whole visit has already completed.
|
Pinging the Turbolinks maintainers. I've worked with Calle on this project and we will soon pick it up again :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Sessionbut share the sameWKProcessPool.When showing and hiding the network activity indicator in response to receiving
sessionDidStartRequest(_:)andsessionDidEndRequest(_:)messages, I found thatsessionDidEndRequest(_:)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 theWKWebViewwon't send thewebView(_:didFinish:)message to itsnavigationDelegate(i.e. theColdBootVisit) 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()inColdBootVisit.completeVisit()to fix this problem. SincefinishRequest()only callssessionDidEndRequest(_:)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!