All tabs in STG are lost if icon is clicked prior to loading sequence

[zkhcohen]

Describe the bug If you start Firefox and click the STG icon in the extensions bar before the initial loading sequence (black, rotating loading icon), all of the tabs in the STGs will be lost. The STGs themselves will remain, but the tab count will appear as "0" in each STG. Clicking on the tab groups or restarting Firefox does not restore the tabs. The only solution is to restore from backup.

To Reproduce Steps to reproduce the behavior:

1. Open Firefox. Wait a moment for STG to load.
2. Close Firefox.
3. Quickly open Firefox and click on the STG icon as fast as possible. If done successfully, when you click on STG again, all of your tab groups will be empty.

Expected behavior Following the steps above would not result in the loss of your saved tabs.

Desktop (please complete the following information):

• OS and version: Windows 11
• Firefox version: 112.0.1
• Simple Tab Groups version: 5.1

Additional context I'll try to do some additional testing to narrow down the root cause. Let me know if logs or settings would assist in the troubleshooting process.

[Drive4ik]

Yes, you can enable debug mode in the settings and try to reproduce the problem. You can email me the log with a link to this ticket. I can't reproduce the problem yet. You can even record a GIF animation)) https://www.cockos.com/licecap/

[zkhcohen]

Yes, you can enable debug mode in the settings and try to reproduce the problem. You can email me the log with a link to this ticket. I can't reproduce the problem yet. You can even record a GIF animation)) https://www.cockos.com/licecap/

Sent logs.

Download GIF here

[rifat0153]

I am facing a similar issue. After opening a tab group, if I close Firefox during loading I will lose all my saved tabs.

Gif Download

Firefox: 112.0.2 (64-bit) STG: 5.2 OS: Windows 11

[zkhcohen]

Update 5/12/2024: The issue occurs without any other extensions added to Firefox, and without any of the default Firefox settings changed. I've also found that a reliable way to replicate the issue is to start and then close Firefox over and over again until the tabs disappear from the groups.

I think I'm getting closer to tracking down the specific culprit in the handling of storage. I also have a working dev build that I'm modifying, deploying and debugging locally. Note for anyone else who tries to build this extension locally, you need NPM 17.9.1 - ver. 22.1.0 doesn't work.

I think the stack looks something like this (with my own presumptive comments added):

At the top-level, the highlighted line in the following snippet returns nothing, causing the tab restoration process to abort:

    let [
        { tabsToRestore: tabsToRestoreNotModified },
        windows,
    ] = await Promise.all([
         // set tabsToRestoreNotModified equal to tabsToRestore
>       Storage.get({ tabsToRestore: [] }), 
        Windows.load(),
    ]);

Another level deeper, the highlighted line removes the "tabsToRestore" key from the local storage database. Commenting this out fixes the issue. Since this either requires "tabsToRestoreNotModified" == "tabsToRestore", and "tabsToRestoreNotModified" is defined by "tabsToRestore" above, prior to the restoration process, this either means that "tabsToRestore" still equals "tabsToRestoreNotModified", or, based on the logic below, "tabsToRestore" was emptied:

// set tabsInDB equal to tabsToRestore from local storage. we know tabsToRestore can't be null, otherwise we wouldn't have gotten this far into the function.
let { tabsToRestore: tabsInDB } = await Storage.get({ tabsToRestore: [] });
    // filter tabsInDB based on values in tabsToRestoreNotModified
    tabsInDB = tabsInDB.filter(t => {
        // filter out all of the tabs that exist in both lists, leaving just the unique tabs
        return !tabsToRestoreNotModified.some(tab => t.groupId === tab.groupId && t.url === tab.url && t.cookieStoreId === tab.cookieStoreId);
    });

    // if the lists are the same or tabsInDB is an empty array (due to tabsToRestore being empty), the length will be 0
    if (tabsInDB.length) {
        // if there are tabs that haven't been restored, set tabsToRestore to those tabs
        await Storage.set({ tabsToRestore: tabsInDB });
    } else {
           // if there are no tabs left to restore (because the lists match), then remove the tabsToRestore from the local storage DB
>        await Storage.remove('tabsToRestore');
    }

The lack of documented code is making it extremely difficult to determine exactly what the root cause is, but I'm fairly confident it was introduced with the following commit, at least symptomatically: https://github.com/Drive4ik/simple-tab-groups/commit/0f9e1850480ed7011ad88dee87be8a0419b34fc5

Cross-referencing it with the logs, it appears that even on runs where Storage.remove('tabsToRestore'); was called, subsequent runs don't necessarily fail. Maybe this suggests that the mechanism which is supposed to re-set the tabsToRestore fails in certain cases, and by not removing tabsToRestore from the database, it serves as a fallback?

Testing that theory, it appears to hold partially true. By opening Firefox, adding a tab to a group, closing Firefox and reopening it, repeating these steps multiple times, I am able to confirm that in some cases the new tab is lost. Interestingly, removing tabs doesn't prevent them from being reopened. In other words, you can only add tabs, not remove them from the list that will be restored.

Looking back at the old commits and cross-referencing it with the logs / behavior of the current 5.2 code, regardless of the new "missed tabs" functionality, the "tabsToRestore" needs to get reset at the end of the restore operation. This means that the "fix" of commenting-out the "await Storage.remove('tabsToRestore');" was purely obscuring the underlying issue.

This makes me believe that there's some race condition between the following two functions associated with event listeners. If "onRemovedWindow" was called before "onRemovedTab" (and/or "isWindowClosing" == false) and "tabsToRestore" had already been zeroed, then nothing would get appended to "tabsToRestore" so it would stay zeroed for the next session and trigger the failure.

function onRemovedTab(tabId, { isWindowClosing, windowId }) {
    const log = logger.start('onRemovedTab', tabId, { isWindowClosing, windowId });

    if (isWindowClosing) {
>       reCreateTabsOnRemoveWindow.push(tabId);
        log.stop('add to reCreateTabsOnRemoveWindow');
    } else {
        Cache.removeTab(tabId);
        log.stop('tab removed from Cache');
    }
}

const onRemovedWindow = catchFunc(async function (windowId) {
    const log = logger.start(['info', 'onRemovedWindow'], windowId);

    let groupId = Cache.getWindowGroup(windowId);

    if (groupId) {
        sendMessage('window-closed', { windowId });
    }

    Cache.removeWindow(windowId);

>   let tabsToRestore = Cache.getTabsSessionAndRemove(reCreateTabsOnRemoveWindow);

    reCreateTabsOnRemoveWindow = [];

    if (tabsToRestore.length) {
        log.info('start merge tabs');
        let { tabsToRestore: prevRestore } = await Storage.get({ tabsToRestore: [] });
        tabsToRestore = tabsToRestore.filter(tab => !prevRestore.some(t => t.groupId === tab.groupId && t.url === tab.url && t.cookieStoreId === tab.cookieStoreId));
>       await Storage.set({
            tabsToRestore: [...prevRestore, ...tabsToRestore]
        });

        log.info('stop merge tabs > start restoring tabs');

        restoringMissedTabsPromise = tryRestoreMissedTabs();
        await restoringMissedTabsPromise.catch(log.onCatch('tryRestoreMissedTabs'));
        restoringMissedTabsPromise = null;
        log.info('stop restoring tabs');
    }

    log.stop();
});

[achernyakevich-sc]

@zkhcohen Thank you for the detailed research.

I can add from my side (maybe it will help) that:

• I have experienced the issue twice.
• both times during Firefox restart my laptop was under high load (CPU and SSD utilization was near to 100%).

[curtiola]

All the tabs in my groups were lost yesterday after I enabled Firefox Sync. I was just trying to sync everything from windows to a fresh kubuntu (snap) install. I had a backup of my tab groups and my profile folder, which I needed to restore, disabling sync and using a STG backup wasn't enough. I confirmed it starts when enabling sync and signing in this morning & it's happening on both windows and kubuntu.

For some reason, it only happened to one profile, my 2nd profile synced just fine. I changed nothing other than enabling sync on both profiles and signing in to 2 fresh mozilla accounts I had just made. I have no idea why this one profile refuses to work, is there any useful information I can look for in settings or the profile folders? ("Open previous windows and tabs" is checked)

Edit: I have backups of the profile folder before the bug, after enabling sync (which was fine by itself), and then after signing in to yet another fresh mozilla account (just in case my other one was tainted). I'm going to keep these folders separated and a WinMerge project showing the differences.

I'm down to investigate but I don't know where to start looking, this is new territory for me so a pointer or 2 would be appreciated. Screenshot of the base profile folder differences in winmerge:

Prefs from bugged profile and working sync profile: prefs.js-from-both-profiles.zip

Yet another edit: I think this is a firefox bug, not STG. It loses tabs that aren't in any groups as well, with or without "sync open tabs" enabled.

[zkhcohen]

Yet another edit: I think this is a firefox bug, not STG. It loses tabs that aren't in any groups as well, with or without "sync open tabs" enabled.

There may be another bug with Firefox sync, but this error occurs without it enabled.

I'm certain that the problem is due to a race condition, I just haven't had the time to fork the project and fix it.