Store more article view controller state when destroying and recreating them. Issue #1729

iOS/Article/ArticleViewController.swift

@@ -14,6 +14,11 @@ import SafariServices
 
 class ArticleViewController: UIViewController {
 	
+	typealias State = (extractedArticle: ExtractedArticle?,
+		isShowingExtractedArticle: Bool,
+		articleExtractorButtonState: ArticleExtractorButtonState,
+		windowScrollY: Int)
+
 	@IBOutlet private weak var nextUnreadBarButtonItem: UIBarButtonItem!
 	@IBOutlet private weak var prevArticleBarButtonItem: UIBarButtonItem!
 	@IBOutlet private weak var nextArticleBarButtonItem: UIBarButtonItem!
@@ -49,7 +54,16 @@ class ArticleViewController: UIViewController {
 			updateUI()
 		}
 	}
-	var restoreWindowScrollY = 0
+	
+	var currentState: State? {
+		guard let controller = currentWebViewController else { return nil}
+		return State(extractedArticle: controller.extractedArticle,
+					 isShowingExtractedArticle: controller.isShowingExtractedArticle,
+					 articleExtractorButtonState: controller.articleExtractorButtonState,
+					 windowScrollY: controller.windowScrollY)
+	}
+	
+	var restoreState: State?
 	
 	private let keyboardManager = KeyboardManager(type: .detail)
 	override var keyCommands: [UIKeyCommand]? {
@@ -89,7 +103,12 @@ class ArticleViewController: UIViewController {
 		])
 				
 		let controller = createWebViewController(article)
-		controller.restoreWindowScrollY = restoreWindowScrollY
+		if let state = restoreState {
+			controller.extractedArticle = state.extractedArticle
+			controller.isShowingExtractedArticle = state.isShowingExtractedArticle
+			controller.articleExtractorButtonState = state.articleExtractorButtonState
+			controller.windowScrollY = state.windowScrollY
+		}
 		articleExtractorButton.buttonState = controller.articleExtractorButtonState
 		pageViewController.setViewControllers([controller], direction: .forward, animated: false, completion: nil)
 		
@@ -239,18 +258,16 @@ class ArticleViewController: UIViewController {
 		currentWebViewController?.fullReload()
 	}
 	
+	func stopArticleExtractorIfProcessing() {
+		currentWebViewController?.stopArticleExtractorIfProcessing()
+	}
+	
 }
 
 // MARK: WebViewControllerDelegate
 
 extension ArticleViewController: WebViewControllerDelegate {
 	
-	func webViewController(_ webViewController: WebViewController, restoreWindowScrollYDidUpdate restoreWindowScrollY: Int) {
-		if webViewController === currentWebViewController {
-			self.restoreWindowScrollY = restoreWindowScrollY
-		}
-	}
-	
 	func webViewController(_ webViewController: WebViewController, articleExtractorButtonStateDidUpdate buttonState: ArticleExtractorButtonState) {
 		if webViewController === currentWebViewController {
 			articleExtractorButton.buttonState = buttonState

iOS/Article/WebViewController.swift

@@ -14,7 +14,6 @@ import Articles
 import SafariServices
 
 protocol WebViewControllerDelegate: class {
-	func webViewController(_: WebViewController, restoreWindowScrollYDidUpdate: Int)
 	func webViewController(_: WebViewController, articleExtractorButtonStateDidUpdate: ArticleExtractorButtonState)
 }
 
@@ -39,8 +38,12 @@ class WebViewController: UIViewController {
 	private var clickedImageCompletion: (() -> Void)?
 
 	private var articleExtractor: ArticleExtractor? = nil
-	private var extractedArticle: ExtractedArticle?
-	private var isShowingExtractedArticle = false {
+	var extractedArticle: ExtractedArticle? {
+		didSet {
+			windowScrollY = 0
+		}
+	}
+	var isShowingExtractedArticle = false {
 		didSet {
 			if isShowingExtractedArticle != oldValue {
 				reloadHTML()
@@ -64,18 +67,14 @@ class WebViewController: UIViewController {
 				startArticleExtractor()
 			}
 			if article != oldValue {
-				restoreWindowScrollY = 0
+				windowScrollY = 0
 				reloadHTML()
 			}
 		}
 	}
 	
 	let scrollPositionQueue = CoalescingQueue(name: "Article Scroll Position", interval: 0.3, maxInterval: 1.0)
-	var restoreWindowScrollY = 0 {
-		didSet {
-			delegate?.webViewController(self, restoreWindowScrollYDidUpdate: restoreWindowScrollY)
-		}
-	}
+	var windowScrollY = 0 
 	
 	deinit {
 		if webView != nil  {
@@ -247,6 +246,12 @@ class WebViewController: UIViewController {
 
 	}
 	
+	func stopArticleExtractorIfProcessing() {
+		if articleExtractor?.state == .processing {
+			stopArticleExtractor()
+		}
+	}
+	
 	func showActivityDialog(popOverBarButtonItem: UIBarButtonItem? = nil) {
 		guard let preferredLink = article?.preferredLink, let url = URL(string: preferredLink) else {
 			return
@@ -423,7 +428,7 @@ extension WebViewController: UIScrollViewDelegate {
 	
 	@objc func scrollPositionDidChange() {
 		webView?.evaluateJavaScript("window.scrollY") { (scrollY, _) in
-			self.restoreWindowScrollY = scrollY as? Int ?? 0
+			self.windowScrollY = scrollY as? Int ?? 0
 		}
 	}
 	
@@ -481,10 +486,10 @@ private extension WebViewController {
 		var render = "error();"
 		if let data = try? encoder.encode(templateData) {
 			let json = String(data: data, encoding: .utf8)!
-			render = "render(\(json), \(restoreWindowScrollY));"
+			render = "render(\(json), \(windowScrollY));"
 		}
 
-		restoreWindowScrollY = 0
+		windowScrollY = 0
 		
 		webView.scrollView.setZoomScale(1.0, animated: false)
 		webView.evaluateJavaScript(render)

iOS/SceneCoordinator.swift

@@ -1778,14 +1778,14 @@ private extension SceneCoordinator {
 	}
 	
 	@discardableResult
-	func installArticleController(restoreWindowScrollY: Int = 0, animated: Bool) -> ArticleViewController {
+	func installArticleController(state: ArticleViewController.State? = nil, animated: Bool) -> ArticleViewController {
 
 		isArticleViewControllerPending = true
 
 		let articleController = UIStoryboard.main.instantiateController(ofType: ArticleViewController.self)
 		articleController.coordinator = self
 		articleController.article = currentArticle
-		articleController.restoreWindowScrollY = restoreWindowScrollY
+		articleController.restoreState = state
 				
 		if let subSplit = subSplitViewController {
 			let controller = addNavControllerIfNecessary(articleController, showButton: false)
@@ -1848,12 +1848,12 @@ private extension SceneCoordinator {
 	}
 	
 	func configureThreePanelMode() {
-		let articleRestoreWindowScrollY = articleViewController?.restoreWindowScrollY ?? 0
+		articleViewController?.stopArticleExtractorIfProcessing()
+		let articleViewControllerState = articleViewController?.currentState
 		defer {
 			masterNavigationController.viewControllers = [masterFeedViewController]
 		}
 		
-		
 		if rootSplitViewController.viewControllers.last is InteractiveNavigationController {
 			_ = rootSplitViewController.viewControllers.popLast()
 		}
@@ -1863,14 +1863,15 @@ private extension SceneCoordinator {
 		masterTimelineViewController?.navigationItem.leftBarButtonItem = rootSplitViewController.displayModeButtonItem
 		masterTimelineViewController?.navigationItem.leftItemsSupplementBackButton = true
 
-		installArticleController(restoreWindowScrollY: articleRestoreWindowScrollY, animated: false)
+		installArticleController(state: articleViewControllerState, animated: false)
 		
 		masterFeedViewController.restoreSelectionIfNecessary(adjustScroll: true)
 		masterTimelineViewController!.restoreSelectionIfNecessary(adjustScroll: false)
 	}
 	
 	func configureStandardPanelMode() {
-		let articleRestoreWindowScrollY = articleViewController?.restoreWindowScrollY ?? 0
+		articleViewController?.stopArticleExtractorIfProcessing()
+		let articleViewControllerState = articleViewController?.currentState
 		rootSplitViewController.preferredPrimaryColumnWidthFraction = UISplitViewController.automaticDimension
 		
 		// Set the is Pending flags early to prevent the navigation controller delegate from thinking that we
@@ -1890,7 +1891,7 @@ private extension SceneCoordinator {
 			masterNavigationController.pushViewController(masterTimelineViewController!, animated: false)
 		}
 
-		installArticleController(restoreWindowScrollY: articleRestoreWindowScrollY, animated: false)
+		installArticleController(state: articleViewControllerState, animated: false)
 	}
 	
 	// MARK: NSUserActivity