Merge pull request #13129 from keymanapp/change/ios/prevent-observer-leaks

change(ios): prevent memory leaks from keyboard-related notification observers

ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift

@@ -165,6 +165,8 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
   // For now, should be mostly upon keymanWeb.view.heightAnchor.
   var portraitConstraint: NSLayoutConstraint?
   var landscapeConstraint: NSLayoutConstraint?
+  
+  var outerWidthConstraint: NSLayoutConstraint?
 
   private var keymanWeb: KeymanWebViewController
   private var swallowBackspaceTextChange: Bool = false
@@ -206,7 +208,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
     keymanWeb = KeymanWebViewController(storage: Storage.active)
     super.init(nibName: nibNameOrNil, bundle: nibBundleOrNil)
 
-    var message = self.hasFullAccess ? "hasFullAccess: true" : "hasFullAccess: false"
+    let message = self.hasFullAccess ? "hasFullAccess: true" : "hasFullAccess: false"
     os_log("%{public}s", log: KeymanEngineLogger.settings, type: .default, message)
     SentryManager.breadcrumb(message)
 
@@ -300,6 +302,16 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
       keymanWeb.shouldReload = true
     }
   }
+  
+  open override func viewDidDisappear(_ animated: Bool) {
+    super.viewDidDisappear(animated)
+    
+    if outerWidthConstraint != nil {
+      outerWidthConstraint?.isActive = false
+      self.inputView?.removeConstraint(self.outerWidthConstraint!)
+      outerWidthConstraint = nil
+    }
+  }
 
   open override func textDidChange(_ textInput: UITextInput?) {
     // Swallows self-triggered calls from emptying the context due to keyboard rules
@@ -527,11 +539,15 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
   }
 
   // These require the view to appear - parent and our relationship with it must exist!
+  // ... wait, is THIS possibly a leak source?
   private func setOuterConstraints() {
-    var baseWidthConstraint: NSLayoutConstraint
-    baseWidthConstraint = self.inputView!.widthAnchor.constraint(equalTo: parent!.view.safeAreaLayoutGuide.widthAnchor)
-    baseWidthConstraint.priority = UILayoutPriority(rawValue: 999)
-    baseWidthConstraint.isActive = true
+    guard outerWidthConstraint == nil else {
+      outerWidthConstraint!.isActive = true
+      return
+    }
+    outerWidthConstraint = self.inputView!.widthAnchor.constraint(equalTo: parent!.view.safeAreaLayoutGuide.widthAnchor)
+    outerWidthConstraint!.priority = UILayoutPriority(rawValue: 999)
+    outerWidthConstraint!.isActive = true
   }
 
   public var kmwHeight: CGFloat {
@@ -589,7 +605,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
   }
 
   // KeymanWebViewController maintenance methods
-  func reload() {
+  public func reload() {
     keymanWeb.reloadKeyboard()
   }
 
@@ -623,10 +639,18 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate {
   func showHelpBubble() {
     keymanWeb.showHelpBubble()
   }
+  
+  func dismissHelpBubble() {
+    keymanWeb.dismissHelpBubble()
+  }
 
   func showHelpBubble(afterDelay delay: TimeInterval) {
     keymanWeb.showHelpBubble(afterDelay: delay)
   }
+  
+  internal func enforceKeyboardSize() {
+    keymanWeb.resizeKeyboard()
+  }
 
   func clearText() {
     setContextState(text: nil, range: NSRange(location: 0, length: 0))

ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift

@@ -26,7 +26,9 @@ class KeyboardMenuView: UIView, UITableViewDelegate, UITableViewDataSource, UIGe
   private var tableView: UITableView?
   private let closeButtonTitle: String?
 
-  private var _inputViewController: InputViewController?
+  // Is populated during init from a weak ref by KeymanWebViewController,
+  // and instances of this class are owned by the same.
+  private weak var _inputViewController: InputViewController?
   override var inputViewController: InputViewController? {
     return _inputViewController
   }

ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift

@@ -72,14 +72,6 @@ class KeymanWebViewController: UIViewController {
     super.init(nibName: nil, bundle: nil)
 
     _ = view
-
-    NotificationCenter.default.addObserver(
-      forName: UIApplication.willEnterForegroundNotification,
-      object: nil,
-      queue: OperationQueue.main
-    ) { _ in
-      self.reloadKeyboard()
-    }
   }
 
   required init?(coder aDecoder: NSCoder) {
@@ -154,11 +146,6 @@ class KeymanWebViewController: UIViewController {
 
     view = webView
 
-    NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillShow),
-                                           name: UIResponder.keyboardWillShowNotification, object: nil)
-    NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillHide),
-                                           name: UIResponder.keyboardWillHideNotification, object: nil)
-
     reloadKeyboard()
   }
 
@@ -990,18 +977,3 @@ extension KeymanWebViewController {
     return keyboardMenuView != nil
   }
 }
-
-// MARK: - Keyboard Notifications
-extension KeymanWebViewController {
-  @objc func keyboardWillShow(_ notification: Notification) {
-    resizeKeyboard()
-
-    if Manager.shared.isKeymanHelpOn {
-      showHelpBubble(afterDelay: 1.5)
-    }
-  }
-
-  @objc func keyboardWillHide(_ notification: Notification) {
-    dismissHelpBubble()
-  }
-}

ios/engine/KMEI/KeymanEngine/Classes/Manager.swift

@@ -239,10 +239,35 @@ public class Manager: NSObject, UIGestureRecognizerDelegate {
         SentryManager.capture(error, message:message)
       }
     }
+    
+    // Keep these within our singleton Manager instance.  As we never remove these,
+    // setting them within something prone to replacement (like the keyboard when in
+    // app-extension mode) can lead to memory leaks.  See #12216.
+    NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillShow), //
+                                           name: UIResponder.keyboardWillShowNotification, object: nil)
+    NotificationCenter.default.addObserver(self, selector: #selector(self.keyboardWillHide), //
+                                           name: UIResponder.keyboardWillHideNotification, object: nil)
 
     // We used to preload the old KeymanWebViewController, but now that it's embedded within the
     // InputViewController, that's not exactly viable.
   }
+  
+  // MARK: - Keyboard Notifications
+  // Do NOT place these anywhere within InputViewController or its children.
+  // So far as we can tell, the OS causes the app extension to sometimes throw
+  // instances away, and maintaining references via notification to them can
+  // cause memory leaks.  Refer to #12216.
+  @objc func keyboardWillShow(_ notification: Notification) {
+    inputViewController?.enforceKeyboardSize()
+
+    if Manager.shared.isKeymanHelpOn {
+      inputViewController?.showHelpBubble(afterDelay: 1.5)
+    }
+  }
+
+  @objc func keyboardWillHide(_ notification: Notification) {
+    inputViewController?.dismissHelpBubble()
+  }
 
   // MARK: - Keyboard management
   

ios/keyman/Keyman/Keyman/Classes/MainViewController/MainViewController.swift

@@ -112,6 +112,14 @@ class MainViewController: UIViewController, TextViewDelegate, UIActionSheetDeleg
       forName: Notifications.keyboardRemoved,
       observer: self,
       function: MainViewController.keyboardRemoved)
+    
+    NotificationCenter.default.addObserver(
+      forName: UIApplication.willEnterForegroundNotification,
+      object: nil,
+      queue: OperationQueue.main
+    ) { _ in
+      Manager.shared.inputViewController.reload()
+    }
 
     // Unfortunately, it's the main app with the file definitions.
     // We have to gerry-rig this so that the framework-based SettingsViewController