Merge pull request #1206

Bug 1215669 - Queue tab screenshots if BVC isn't visible

Client/Frontend/Browser/Browser.swift

@@ -48,6 +48,7 @@ class Browser: NSObject {
     var sessionData: SessionData?
     var lastRequest: NSURLRequest? = nil
     var restoring: Bool = false
+    var pendingScreenshot = false
 
     /// The last title shown by this tab. Used by the tab tray to show titles for zombie tabs.
     var lastTitle: String?

Client/Frontend/Browser/BrowserViewController.swift

@@ -115,7 +115,7 @@ class BrowserViewController: UIViewController {
     }
 
     private func didInit() {
-        screenshotHelper = BrowserScreenshotHelper(controller: self)
+        screenshotHelper = ScreenshotHelper(controller: self)
         tabManager.addDelegate(self)
         tabManager.addNavigationDelegate(self)
     }
@@ -477,9 +477,19 @@ class BrowserViewController: UIViewController {
         startTrackingAccessibilityStatus()
         presentIntroViewController()
         self.webViewContainerToolbar.hidden = false
+
+        screenshotHelper.viewIsVisible = true
+        screenshotHelper.takePendingScreenshots(tabManager.tabs)
+
         super.viewDidAppear(animated)
     }
 
+    override func viewWillDisappear(animated: Bool) {
+        screenshotHelper.viewIsVisible = false
+
+        super.viewWillDisappear(animated)
+    }
+
     override func viewDidDisappear(animated: Bool) {
         stopTrackingAccessibilityStatus()
     }
@@ -883,7 +893,7 @@ extension BrowserViewController: URLBarDelegate {
         let tabTrayController = TabTrayController(tabManager: tabManager, profile: profile)
 
         if let tab = tabManager.selectedTab {
-            tab.setScreenshot(screenshotHelper.takeScreenshot(tab, aspectRatio: 0, quality: 1))
+            screenshotHelper.takeScreenshot(tab)
         }
 
         self.navigationController?.pushViewController(tabTrayController, animated: true)
@@ -1597,14 +1607,7 @@ extension BrowserViewController: WKNavigationDelegate {
             // forward/backward. Strange, but LayoutChanged fixes that.
             UIAccessibilityPostNotification(UIAccessibilityLayoutChangedNotification, nil)
         } else {
-            // Tab is in the backgroud, but we want to be able to see it in the TabTray.
-            // Delay 100ms to let the tab render (it doesn't work without the delay)
-            // before screenshotting.
-            let time = dispatch_time(DISPATCH_TIME_NOW, Int64(100 * NSEC_PER_MSEC))
-            dispatch_after(time, dispatch_get_main_queue()) {
-                let screenshot = self.screenshotHelper.takeScreenshot(tab, aspectRatio: 0, quality: 1)
-                tab.setScreenshot(screenshot)
-            }
+            screenshotHelper.takeDelayedScreenshot(tab)
         }
 
         addOpenInViewIfNeccessary(webView.URL)
@@ -1653,7 +1656,7 @@ extension BrowserViewController: WKUIDelegate {
 
         guard let currentTab = tabManager.selectedTab else { return nil }
 
-        currentTab.setScreenshot(screenshotHelper.takeScreenshot(currentTab, aspectRatio: 0, quality: 1))
+        screenshotHelper.takeScreenshot(currentTab)
 
         // If the page uses window.open() or target="_blank", open the page in a new tab.
         // TODO: This doesn't work for window.open() without user action (bug 1124942).
@@ -1985,29 +1988,6 @@ extension BrowserViewController: ReaderModeBarViewDelegate {
     }
 }
 
-private class BrowserScreenshotHelper: ScreenshotHelper {
-    private weak var controller: BrowserViewController?
-
-    init(controller: BrowserViewController) {
-        self.controller = controller
-    }
-
-    func takeScreenshot(tab: Browser, aspectRatio: CGFloat, quality: CGFloat) -> UIImage? {
-        if let url = tab.url {
-            if AboutUtils.isAboutHomeURL(url) {
-                if let homePanel = controller?.homePanelController {
-                    return homePanel.view.screenshot(aspectRatio, quality: quality)
-                }
-            } else {
-                let offset = CGPointMake(0, -(tab.webView?.scrollView.contentInset.top ?? 0))
-                return tab.webView?.screenshot(aspectRatio, offset: offset, quality: quality)
-            }
-        }
-
-        return nil
-    }
-}
-
 extension BrowserViewController: IntroViewControllerDelegate {
     func presentIntroViewController(force: Bool = false) -> Bool{
         if force || profile.prefs.intForKey(IntroViewControllerSeenProfileKey) == nil {

Client/Frontend/Browser/ScreenshotHelper.swift

@@ -7,6 +7,53 @@ import Foundation
 /**
  * Handles screenshots for a given browser, including pages with non-webview content.
  */
-protocol ScreenshotHelper {
-    func takeScreenshot(tab: Browser, aspectRatio: CGFloat, quality: CGFloat) -> UIImage?
-}
+class ScreenshotHelper {
+    var viewIsVisible = false
+
+    private weak var controller: BrowserViewController?
+
+    init(controller: BrowserViewController) {
+        self.controller = controller
+    }
+
+    func takeScreenshot(tab: Browser) {
+        var screenshot: UIImage?
+
+        if let url = tab.url {
+            if AboutUtils.isAboutHomeURL(url) {
+                if let homePanel = controller?.homePanelController {
+                    screenshot = homePanel.view.screenshot()
+                }
+            } else {
+                let offset = CGPointMake(0, -(tab.webView?.scrollView.contentInset.top ?? 0))
+                screenshot = tab.webView?.screenshot(offset: offset)
+            }
+        }
+
+        tab.setScreenshot(screenshot)
+    }
+
+    /// Takes a screenshot after a small delay.
+    /// Trying to take a screenshot immediately after didFinishNavigation results in a screenshot
+    /// of the previous page, presumably due to an iOS bug. Adding a brief delay fixes this.
+    func takeDelayedScreenshot(tab: Browser) {
+        let time = dispatch_time(DISPATCH_TIME_NOW, Int64(100 * NSEC_PER_MSEC))
+        dispatch_after(time, dispatch_get_main_queue()) {
+            // If the view controller isn't visible, the screenshot will be blank.
+            // Wait until the view controller is visible again to take the screenshot.
+            guard self.viewIsVisible else {
+                tab.pendingScreenshot = true
+                return
+            }
+
+            self.takeScreenshot(tab)
+        }
+    }
+
+    func takePendingScreenshots(tabs: [Browser]) {
+        for tab in tabs where tab.pendingScreenshot {
+            tab.pendingScreenshot = false
+            takeDelayedScreenshot(tab)
+        }
+    }
+}

Client/Frontend/Browser/TabManager.swift

@@ -54,7 +54,7 @@ class TabManager : NSObject {
         }
     }
 
-    private var tabs: [Browser] = []
+    private(set) var tabs = [Browser]()
     private var _selectedIndex = -1
     private let defaultNewTabRequest: NSURLRequest
     private let navDelegate: TabManagerNavDelegate

Utils/Extensions/UIViewExtensions.swift

@@ -25,7 +25,7 @@ extension UIView {
      * Takes a screenshot of the view with the given aspect ratio.
      * An aspect ratio of 0 means capture the entire view.
      */
-    func screenshot(aspectRatio: CGFloat, offset: CGPoint? = nil, quality: CGFloat = 1) -> UIImage? {
+    func screenshot(aspectRatio: CGFloat = 0, offset: CGPoint? = nil, quality: CGFloat = 1) -> UIImage? {
         assert(aspectRatio >= 0)
 
         var size: CGSize