From cf8dbb26f7aaa3333c26bde227927ccd957eedea Mon Sep 17 00:00:00 2001
From: Maurice Parker <mo@vincode.io>
Date: Tue, 1 Sep 2020 18:54:46 -0500
Subject: [PATCH] Resolve issue where we could have a web view deallocated
before getting displayed.
---
.../Shared/Article/PreloadedWebView.swift | 8 +--
.../Shared/Article/WebViewProvider.swift | 8 +--
.../iOS/Article/WebViewController.swift | 56 ++++++++++---------
.../macOS/Article/WebViewController.swift | 41 +++++++-------
iOS/Article/PreloadedWebView.swift | 8 +--
iOS/Article/WebViewController.swift | 56 ++++++++++---------
iOS/Article/WebViewProvider.swift | 8 +--
7 files changed, 94 insertions(+), 91 deletions(-)
diff --git a/Multiplatform/Shared/Article/PreloadedWebView.swift b/Multiplatform/Shared/Article/PreloadedWebView.swift
index baa99a48b..2baca9e73 100644
--- a/Multiplatform/Shared/Article/PreloadedWebView.swift
+++ b/Multiplatform/Shared/Article/PreloadedWebView.swift
@@ -13,7 +13,7 @@ import RSWeb
class PreloadedWebView: WKWebView {
private var isReady: Bool = false
- private var readyCompletion: ((PreloadedWebView) -> Void)?
+ private var readyCompletion: (() -> Void)?
init(articleIconSchemeHandler: ArticleIconSchemeHandler) {
let preferences = WKPreferences()
@@ -44,7 +44,7 @@ class PreloadedWebView: WKWebView {
loadFileURL(ArticleRenderer.blank.url, allowingReadAccessTo: ArticleRenderer.blank.baseURL)
}
- func ready(completion: @escaping (PreloadedWebView) -> Void) {
+ func ready(completion: @escaping () -> Void) {
if isReady {
completeRequest(completion: completion)
} else {
@@ -89,10 +89,10 @@ extension PreloadedWebView: WKNavigationDelegate {
private extension PreloadedWebView {
- func completeRequest(completion: @escaping (PreloadedWebView) -> Void) {
+ func completeRequest(completion: @escaping () -> Void) {
isReady = false
navigationDelegate = nil
- completion(self)
+ completion()
}
}
diff --git a/Multiplatform/Shared/Article/WebViewProvider.swift b/Multiplatform/Shared/Article/WebViewProvider.swift
index 0eff3cd4f..64d834c0c 100644
--- a/Multiplatform/Shared/Article/WebViewProvider.swift
+++ b/Multiplatform/Shared/Article/WebViewProvider.swift
@@ -86,9 +86,7 @@ class WebViewProviderDequeueOperation: MainThreadOperation {
func run() {
if let webView = queue.lastObject as? PreloadedWebView {
- webView.ready { preloadedWebView in
- self.completion(preloadedWebView)
- }
+ self.completion(webView)
self.queue.remove(webView)
self.operationDelegate?.operationDidComplete(self)
return
@@ -98,9 +96,7 @@ class WebViewProviderDequeueOperation: MainThreadOperation {
let webView = PreloadedWebView(articleIconSchemeHandler: articleIconSchemeHandler)
webView.preload()
- webView.ready { preloadedWebView in
- self.completion(preloadedWebView)
- }
+ self.completion(webView)
self.operationDelegate?.operationDidComplete(self)
}
diff --git a/Multiplatform/iOS/Article/WebViewController.swift b/Multiplatform/iOS/Article/WebViewController.swift
index 1630124bd..151084f75 100644
--- a/Multiplatform/iOS/Article/WebViewController.swift
+++ b/Multiplatform/iOS/Article/WebViewController.swift
@@ -452,37 +452,41 @@ private extension WebViewController {
sceneModel?.webViewProvider?.dequeueWebView() { webView in
- // Add the webview
- webView.translatesAutoresizingMaskIntoConstraints = false
- self.view.insertSubview(webView, at: 0)
- NSLayoutConstraint.activate([
- self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
- self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
- self.view.topAnchor.constraint(equalTo: webView.topAnchor),
- self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
- ])
-
- // UISplitViewController reports the wrong size to WKWebView which can cause horizontal
- // rubberbanding on the iPad. This interferes with our UIPageViewController preventing
- // us from easily swiping between WKWebViews. This hack fixes that.
- webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: -1, bottom: 0, right: 0)
+ webView.ready {
+
+ // Add the webview
+ webView.translatesAutoresizingMaskIntoConstraints = false
+ self.view.insertSubview(webView, at: 0)
+ NSLayoutConstraint.activate([
+ self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
+ self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
+ self.view.topAnchor.constraint(equalTo: webView.topAnchor),
+ self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
+ ])
+
+ // UISplitViewController reports the wrong size to WKWebView which can cause horizontal
+ // rubberbanding on the iPad. This interferes with our UIPageViewController preventing
+ // us from easily swiping between WKWebViews. This hack fixes that.
+ webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: -1, bottom: 0, right: 0)
- webView.scrollView.setZoomScale(1.0, animated: false)
+ webView.scrollView.setZoomScale(1.0, animated: false)
- self.view.setNeedsLayout()
- self.view.layoutIfNeeded()
+ self.view.setNeedsLayout()
+ self.view.layoutIfNeeded()
- // Configure the webview
- webView.navigationDelegate = self
- webView.uiDelegate = self
- webView.scrollView.delegate = self
-// self.configureContextMenuInteraction()
+ // Configure the webview
+ webView.navigationDelegate = self
+ webView.uiDelegate = self
+ webView.scrollView.delegate = self
+ // self.configureContextMenuInteraction()
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
- self.renderPage(webView)
+ self.renderPage(webView)
+
+ }
}
diff --git a/Multiplatform/macOS/Article/WebViewController.swift b/Multiplatform/macOS/Article/WebViewController.swift
index e37ab4b8b..c0ebfc927 100644
--- a/Multiplatform/macOS/Article/WebViewController.swift
+++ b/Multiplatform/macOS/Article/WebViewController.swift
@@ -246,28 +246,31 @@ private extension WebViewController {
sceneModel?.webViewProvider?.dequeueWebView() { webView in
- // Add the webview
- self.webView = webView
+ webView.ready {
+
+ // Add the webview
+ self.webView = webView
+
+ webView.translatesAutoresizingMaskIntoConstraints = false
+ self.view.addSubview(webView, positioned: .below, relativeTo: self.statusBarView)
+ NSLayoutConstraint.activate([
+ self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
+ self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
+ self.view.topAnchor.constraint(equalTo: webView.topAnchor),
+ self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
+ ])
+
+ webView.navigationDelegate = self
- webView.translatesAutoresizingMaskIntoConstraints = false
- self.view.addSubview(webView, positioned: .below, relativeTo: self.statusBarView)
- NSLayoutConstraint.activate([
- self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
- self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
- self.view.topAnchor.constraint(equalTo: webView.topAnchor),
- self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
- ])
-
- webView.navigationDelegate = self
-
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.mouseDidEnter)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.mouseDidExit)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.mouseDidEnter)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.mouseDidExit)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
- self.renderPage(webView)
+ self.renderPage(webView)
+ }
}
}
diff --git a/iOS/Article/PreloadedWebView.swift b/iOS/Article/PreloadedWebView.swift
index c5e0c2c90..3bf76a2ca 100644
--- a/iOS/Article/PreloadedWebView.swift
+++ b/iOS/Article/PreloadedWebView.swift
@@ -12,7 +12,7 @@ import WebKit
class PreloadedWebView: WKWebView {
private var isReady: Bool = false
- private var readyCompletion: ((PreloadedWebView) -> Void)?
+ private var readyCompletion: (() -> Void)?
init(articleIconSchemeHandler: ArticleIconSchemeHandler) {
let preferences = WKPreferences()
@@ -38,7 +38,7 @@ class PreloadedWebView: WKWebView {
loadFileURL(ArticleRenderer.blank.url, allowingReadAccessTo: ArticleRenderer.blank.baseURL)
}
- func ready(completion: @escaping (PreloadedWebView) -> Void) {
+ func ready(completion: @escaping () -> Void) {
if isReady {
completeRequest(completion: completion)
} else {
@@ -66,10 +66,10 @@ extension PreloadedWebView: WKNavigationDelegate {
private extension PreloadedWebView {
- func completeRequest(completion: @escaping (PreloadedWebView) -> Void) {
+ func completeRequest(completion: @escaping () -> Void) {
isReady = false
navigationDelegate = nil
- completion(self)
+ completion()
}
}
diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift
index c6bd80fbf..ca3c0ff8e 100644
--- a/iOS/Article/WebViewController.swift
+++ b/iOS/Article/WebViewController.swift
@@ -483,37 +483,41 @@ private extension WebViewController {
coordinator.webViewProvider.dequeueWebView() { webView in
- // Add the webview
- webView.translatesAutoresizingMaskIntoConstraints = false
- self.view.insertSubview(webView, at: 0)
- NSLayoutConstraint.activate([
- self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
- self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
- self.view.topAnchor.constraint(equalTo: webView.topAnchor),
- self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
- ])
-
- // UISplitViewController reports the wrong size to WKWebView which can cause horizontal
- // rubberbanding on the iPad. This interferes with our UIPageViewController preventing
- // us from easily swiping between WKWebViews. This hack fixes that.
- webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: -1, bottom: 0, right: 0)
+ webView.ready {
+
+ // Add the webview
+ webView.translatesAutoresizingMaskIntoConstraints = false
+ self.view.insertSubview(webView, at: 0)
+ NSLayoutConstraint.activate([
+ self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor),
+ self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor),
+ self.view.topAnchor.constraint(equalTo: webView.topAnchor),
+ self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor)
+ ])
+
+ // UISplitViewController reports the wrong size to WKWebView which can cause horizontal
+ // rubberbanding on the iPad. This interferes with our UIPageViewController preventing
+ // us from easily swiping between WKWebViews. This hack fixes that.
+ webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: -1, bottom: 0, right: 0)
- webView.scrollView.setZoomScale(1.0, animated: false)
+ webView.scrollView.setZoomScale(1.0, animated: false)
- self.view.setNeedsLayout()
- self.view.layoutIfNeeded()
+ self.view.setNeedsLayout()
+ self.view.layoutIfNeeded()
- // Configure the webview
- webView.navigationDelegate = self
- webView.uiDelegate = self
- webView.scrollView.delegate = self
- self.configureContextMenuInteraction()
+ // Configure the webview
+ webView.navigationDelegate = self
+ webView.uiDelegate = self
+ webView.scrollView.delegate = self
+ self.configureContextMenuInteraction()
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
- webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown)
+ webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector)
- self.renderPage(webView)
+ self.renderPage(webView)
+
+ }
}
diff --git a/iOS/Article/WebViewProvider.swift b/iOS/Article/WebViewProvider.swift
index 50c6e2780..4c49d3ed5 100644
--- a/iOS/Article/WebViewProvider.swift
+++ b/iOS/Article/WebViewProvider.swift
@@ -86,9 +86,7 @@ class WebViewProviderDequeueOperation: MainThreadOperation {
func run() {
if let webView = queue.lastObject as? PreloadedWebView {
- webView.ready { preloadedWebView in
- self.completion(preloadedWebView)
- }
+ self.completion(webView)
self.queue.remove(webView)
self.operationDelegate?.operationDidComplete(self)
return
@@ -98,9 +96,7 @@ class WebViewProviderDequeueOperation: MainThreadOperation {
let webView = PreloadedWebView(articleIconSchemeHandler: articleIconSchemeHandler)
webView.preload()
- webView.ready { preloadedWebView in
- self.completion(preloadedWebView)
- }
+ self.completion(webView)
self.operationDelegate?.operationDidComplete(self)
}