TabRecency: fix a counter bug when loading recency from storage

Author: philc

background_scripts/main.js

@@ -317,9 +317,11 @@ const BackgroundCommands = {
   },
 
   visitPreviousTab({ count, tab }) {
-    const tabIds = BgUtils.tabRecency.getTabsByRecency().filter((tabId) => tabId !== tab.id);
+    let tabIds = BgUtils.tabRecency.getTabsByRecency();
+    tabIds = tabIds.filter((tabId) => tabId !== tab.id);
     if (tabIds.length > 0) {
-      selectSpecificTab({ id: tabIds[(count - 1) % tabIds.length] });
+      const id = tabIds[(count - 1) % tabIds.length];
+      selectSpecificTab({ id });
     }
   },
 

background_scripts/tab_recency.js

@@ -45,6 +45,9 @@ class TabRecency {
     for (const counter of Object.values(storage.tabRecency)) {
       if (maxCounter < counter) maxCounter = counter;
     }
+    if (this.counter < maxCounter + 1) {
+      this.counter = maxCounter + 1;
+    }
     // Tabs loaded from storage should be considered accessed less recently than any tab tracked in
     // memory, so increase all of the in-memory tabs's counters by maxCounter.
     for (const [id, counter] of Object.entries(this.tabIdToCounter)) {

tests/unit_tests/tab_recency_test.js

@@ -58,6 +58,21 @@ context("TabRecency", () => {
     assert.equal([4, 3, 2, 1], tabRecency.getTabsByRecency());
   });
 
+  should("loadFromStorage works if there are no in-memory tab tabs", async () => {
+    const tabs = [{ id: 1 }, { id: 2 }];
+    stub(chrome.tabs, "query", () => Promise.resolve(tabs));
+
+    const storage = { tabRecency: { 1: 4, 2: 1 } };
+    stub(chrome.storage.session, "get", () => Promise.resolve(storage));
+
+    await tabRecency.loadFromStorage();
+
+    // This tab's counter should be the max of the tab counters loaded from storage.
+    tabRecency.register(2);
+
+    assert.equal([2, 1], tabRecency.getTabsByRecency());
+  });
+
   should("loadFromStorage prunes out tabs which are no longer active", async () => {
     const tabs = [{ id: 1 }];
     stub(chrome.tabs, "query", () => Promise.resolve(tabs));