Bug 1395265 - Make sure Activity stream queries do not run on the mai…

…n thread (#3131) r=farhan

Now, we make sure the query is run async off the main thread. We also don't refresh the Panel as much making sure the number of times the recalculation happens is low.

Client/Frontend/Home/ActivityStreamPanel.swift

@@ -65,7 +65,6 @@ class ActivityStreamPanel: UICollectionViewController, HomePanel {
     fileprivate let flowLayout: UICollectionViewFlowLayout = UICollectionViewFlowLayout()
 
     fileprivate let topSitesManager = ASHorizontalScrollCellManager()
-    fileprivate var pendingCacheUpdate = false
     fileprivate var showHighlightIntro = false
     fileprivate var sessionStart: Timestamp?
 
@@ -482,8 +481,11 @@ extension ActivityStreamPanel: DataObserverDelegate {
         }
         accumulate([self.getHighlights, self.getTopSites]).uponQueue(.main) { _ in
             // If there is no pending cache update and highlights are empty. Show the onboarding screen
-            self.showHighlightIntro = self.highlights.isEmpty && !self.pendingCacheUpdate
+            self.showHighlightIntro = self.highlights.isEmpty
             self.collectionView?.reloadData()
+
+            // Refresh the AS data in the background so we'll have fresh data next time we show.
+            self.profile.panelDataObservers.activityStream.refreshIfNeeded(forceHighlights: false, forceTopSites: false)
         }
     }
 
@@ -510,7 +512,7 @@ extension ActivityStreamPanel: DataObserverDelegate {
 
     func getTopSites() -> Success {
         return self.profile.history.getTopSitesWithLimit(16).both(self.profile.history.getPinnedTopSites()).bindQueue(.main) { (topsites, pinnedSites) in
-            guard let mySites = topsites.successValue?.asArray(), let pinned = pinnedSites.successValue?.asArray(), !self.pendingCacheUpdate else {
+            guard let mySites = topsites.successValue?.asArray(), let pinned = pinnedSites.successValue?.asArray() else {
                 return succeed()
             }
 
@@ -558,14 +560,15 @@ extension ActivityStreamPanel: DataObserverDelegate {
             return succeed()
         }
     }
-    func willInvalidateDataSources() {
-        self.pendingCacheUpdate = true
-    }
 
     // Invoked by the ActivityStreamDataObserver when highlights/top sites invalidation is complete.
-    func didInvalidateDataSources() {
-        self.pendingCacheUpdate = false
-        reloadAll()
+    func didInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool) {
+        // Do not reload panel unless we're currently showing the highlight intro or if we
+        // force-reloaded the highlights or top sites. This should prevent reloading the
+        // panel after we've invalidated in the background on the first load.
+        if showHighlightIntro || highlights || topSites {
+            reloadAll()
+        }
     }
 
     func hideURLFromTopSites(_ site: Site) {
@@ -741,7 +744,7 @@ extension ActivityStreamPanel: HomePanelContextMenu {
         } else {
             bookmarkAction = PhotonActionSheetItem(title: Strings.BookmarkContextMenuTitle, iconString: "action_bookmark", handler: { action in
                 let shareItem = ShareItem(url: site.url, title: site.title, favicon: site.icon)
-                self.profile.bookmarks.shareItem(shareItem)
+                _ = self.profile.bookmarks.shareItem(shareItem)
                 var userData = [QuickActions.TabURLKey: shareItem.url]
                 if let title = shareItem.title {
                     userData[QuickActions.TabTitleKey] = title

Client/Frontend/Home/HomePanelViewController.swift

@@ -134,8 +134,6 @@ class HomePanelViewController: UIViewController, UITextFieldDelegate, HomePanelD
         let dismissKeyboardGestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(HomePanelViewController.SELhandleDismissKeyboardGestureRecognizer(_:)))
         dismissKeyboardGestureRecognizer.cancelsTouchesInView = false
         buttonContainerView.addGestureRecognizer(dismissKeyboardGestureRecognizer)
-
-        self.profile.panelDataObservers.activityStream.refreshIfNeeded(forceHighlights: false, forceTopSites: true)
     }
 
     fileprivate func updateAppState() {

Client/Frontend/Home/PanelDataObservers.swift

@@ -17,9 +17,15 @@ protocol DataObserver {
     func refreshIfNeeded(forceHighlights highlights: Bool, forceTopSites topSites: Bool)
 }
 
-@objc protocol DataObserverDelegate: class {
-    func didInvalidateDataSources()
-    func willInvalidateDataSources()
+protocol DataObserverDelegate: class {
+    func didInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool)
+    func willInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool)
+}
+
+// Make these delegate methods optional by providing default implementations
+extension DataObserverDelegate {
+    func didInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool) {}
+    func willInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool) {}
 }
 
 open class PanelDataObservers {
@@ -68,13 +74,13 @@ class ActivityStreamDataObserver: DataObserver {
             return
         }
 
-        self.delegate?.willInvalidateDataSources()
-        self.profile.recommendations.repopulate(invalidateTopSites: shouldInvalidateTopSites, invalidateHighlights: shouldInvalidateHighlights).uponQueue(.main) { _ in
+        self.delegate?.willInvalidateDataSources(forceHighlights: highlights, forceTopSites: topSites)
+        self.profile.recommendations.repopulate(invalidateTopSites: shouldInvalidateTopSites, invalidateHighlights: shouldInvalidateHighlights).uponQueue(DispatchQueue.main) { _ in
             if shouldInvalidateTopSites {
                 self.profile.prefs.setBool(true, forKey: PrefsKeys.KeyTopSitesCacheIsValid)
             }
             self.lastInvalidation = shouldInvalidateHighlights ? Timestamp.uptimeInMilliseconds() : self.lastInvalidation
-            self.delegate?.didInvalidateDataSources()
+            self.delegate?.didInvalidateDataSources(forceHighlights: highlights, forceTopSites: topSites)
         }
     }
 

ClientTests/PanelDataObserversTests.swift

@@ -11,11 +11,11 @@ private class MockDataObserverDelegate: DataObserverDelegate {
     var didInvalidateCount = 0
     var willInvalidateCount = 0
 
-    func didInvalidateDataSources() {
+    func didInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool) {
         didInvalidateCount += 1
     }
 
-    func willInvalidateDataSources() {
+    func willInvalidateDataSources(forceHighlights highlights: Bool, forceTopSites topSites: Bool) {
         willInvalidateCount += 1
     }
 }

Storage/SQL/BrowserDB.swift

@@ -13,16 +13,6 @@ private let log = Logger.syncLogger
 
 public typealias Args = [Any?]
 
-protocol Changeable {
-    func run(_ sql: String, withArgs args: Args?) -> Success
-    func run(_ commands: [String]) -> Success
-    func run(_ commands: [(sql: String, args: Args?)]) -> Success
-}
-
-protocol Queryable {
-    func runQuery<T>(_ sql: String, args: Args?, factory: @escaping (SDRow) -> T) -> Deferred<Maybe<Cursor<T>>>
-}
-
 open class BrowserDB {
     fileprivate let db: SwiftData
 
@@ -198,9 +188,7 @@ open class BrowserDB {
     public func reopenIfClosed() {
         db.reopenIfClosed()
     }
-}
 
-extension BrowserDB: Changeable {
     func run(_ sql: String, withArgs args: Args? = nil) -> Success {
         return run([(sql, args)])
     }
@@ -237,9 +225,31 @@ extension BrowserDB: Changeable {
 
         return succeed()
     }
-}
 
-extension BrowserDB: Queryable {
+    func runAsync(_ commands: [(sql: String, args: Args?)]) -> Success {
+        if commands.isEmpty {
+            return succeed()
+        }
+
+        let deferred = Success()
+
+        var error: NSError?
+        error = self.transaction(synchronous: false, err: &error) { (conn, err) -> Bool in
+            for (sql, args) in commands {
+                err = conn.executeChange(sql, withArgs: args)
+                if let err = err {
+                    deferred.fill(Maybe(failure: DatabaseError(err: err)))
+                    return false
+                }
+            }
+
+            deferred.fill(Maybe(success: ()))
+            return true
+        }
+
+        return deferred
+    }
+
     func runQuery<T>(_ sql: String, args: Args?, factory: @escaping (SDRow) -> T) -> Deferred<Maybe<Cursor<T>>> {
         return runWithConnection { (connection, _) -> Cursor<T> in
             return connection.executeQuery(sql, factory: factory, withArgs: args)

Storage/SQL/SQLiteHistoryRecommendations.swift

@@ -144,7 +144,7 @@ extension SQLiteHistory: HistoryRecommendations {
         if shouldInvalidateHighlights {
             queries.append(contentsOf: self.repopulateHighlightsQuery())
         }
-        return self.db.run(queries)
+        return self.db.runAsync(queries)
     }
 
     public func getRecentBookmarks(_ limit: Int = 3) -> Deferred<Maybe<Cursor<Site>>> {