diff --git a/src/lib/Account.ts b/src/lib/Account.ts index cb72687745..a215633335 100644 --- a/src/lib/Account.ts +++ b/src/lib/Account.ts @@ -1,6 +1,6 @@ import AdapterFactory from './AdapterFactory' import Logger from './Logger' -import { Folder, ItemLocation, TItemLocation } from './Tree' +import { ItemLocation, TItemLocation } from './Tree' import UnidirectionalSyncProcess from './strategies/Unidirectional' import MergeSyncProcess from './strategies/Merge' import DefaultSyncProcess from './strategies/Default' @@ -208,7 +208,7 @@ export default class Account { // main sync steps: mappings = await this.storage.getMappings() - const cacheTree = localResource.constructor.name !== 'LocalTabs' ? await this.storage.getCache() : new Folder({title: '', id: 'tabs', location: ItemLocation.LOCAL}) + const cacheTree = await this.storage.getCache() let continuation = await this.storage.getCurrentContinuation() @@ -280,12 +280,10 @@ export default class Account { await this.setData({ ...this.getData(), scheduled: false, syncing: 1 }) // update cache - if (localResource.constructor.name !== 'LocalTabs') { - const cache = (await localResource.getBookmarksTree()).clone(false) - this.syncProcess.filterOutUnacceptedBookmarks(cache) - this.syncProcess.filterOutUnmappedItems(cache, await mappings.getSnapshot()) - await this.storage.setCache(cache) - } + const cache = (await localResource.getBookmarksTree()).clone(false) + this.syncProcess.filterOutUnacceptedBookmarks(cache) + this.syncProcess.filterOutUnmappedItems(cache, await mappings.getSnapshot()) + await this.storage.setCache(cache) if (this.server.onSyncComplete) { await this.server.onSyncComplete() diff --git a/src/lib/LocalTabs.ts b/src/lib/LocalTabs.ts index dbca1acefb..28b6a822ea 100644 --- a/src/lib/LocalTabs.ts +++ b/src/lib/LocalTabs.ts @@ -1,12 +1,12 @@ import browser from './browser-api' import Logger from './Logger' -import { IResource } from './interfaces/Resource' +import { OrderFolderResource } from './interfaces/Resource' import PQueue from 'p-queue' import { Bookmark, Folder, ItemLocation } from './Tree' import Ordering from './interfaces/Ordering' import uniq from 'lodash/uniq' -export default class LocalTabs implements IResource { +export default class LocalTabs implements OrderFolderResource { private queue: PQueue<{ concurrency: 10 }> private storage: unknown @@ -65,6 +65,7 @@ export default class LocalTabs implements IResource { active: false, }) ) + await awaitTabsUpdated() return node.id } @@ -85,6 +86,7 @@ export default class LocalTabs implements IResource { index: -1 // last }) ) + await awaitTabsUpdated() } async removeBookmark(bookmark:Bookmark): Promise { @@ -95,6 +97,7 @@ export default class LocalTabs implements IResource { return } await this.queue.add(() => browser.tabs.remove(bookmarkId)) + await awaitTabsUpdated() } async createFolder(folder:Folder): Promise { @@ -131,6 +134,7 @@ export default class LocalTabs implements IResource { throw new Error('Failed to reorder folder ' + id + ': ' + e.message) } } + await awaitTabsUpdated() } async updateFolder(folder:Folder):Promise { @@ -140,7 +144,7 @@ export default class LocalTabs implements IResource { async removeFolder(folder:Folder):Promise { const id = folder.id Logger.log('(tabs)REMOVEFOLDER', id) - await this.queue.add(() => browser.tabs.remove(id)) + await this.queue.add(() => browser.window.remove(id)) } async isAvailable(): Promise { @@ -150,3 +154,15 @@ export default class LocalTabs implements IResource { return Boolean(tabs.length) } } + +function awaitTabsUpdated() { + return Promise.race([ + new Promise(resolve => { + browser.tabs.onUpdated.addListener(() => { + browser.tabs.onUpdated.removeListener(resolve) + setTimeout(() => resolve(), 1000) + }) + }), + new Promise(resolve => setTimeout(resolve, 1100)) + ]) +} diff --git a/src/lib/browser/BrowserController.js b/src/lib/browser/BrowserController.js index 2f1498ee3e..b2d02459d5 100644 --- a/src/lib/browser/BrowserController.js +++ b/src/lib/browser/BrowserController.js @@ -167,6 +167,9 @@ export default class BrowserController { } } }) + + // Run some things on browser startup + browser.runtime.onStartup.addListener(this.onStartup) } async _receiveEvent(data, sendResponse) { @@ -412,7 +415,7 @@ export default class BrowserController { } } - async onLoad() { + async onStartup() { const accounts = await Account.getAllAccounts() await Promise.all( accounts.map(async acc => { @@ -423,9 +426,14 @@ export default class BrowserController { scheduled: acc.getData().enabled, }) } + if (acc.getData().localRoot === 'tabs') { + await acc.init() + } }) ) + } + async onLoad() { browser.storage.local.get('telemetryEnabled').then(async d => { if (!d.telemetryEnabled) { return diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index dd70fe72d0..0dc5972aa6 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -27,6 +27,7 @@ import { CancelledSyncError, FailsafeError } from '../../errors/Error' import NextcloudBookmarksAdapter from '../adapters/NextcloudBookmarks' import CachingAdapter from '../adapters/Caching' +import LocalTabs from '../LocalTabs' const ACTION_CONCURRENCY = 12 @@ -424,27 +425,55 @@ export default class SyncProcess { const newMappings = [] - // if we have the cache available, Diff cache and both trees - const localScanner = new Scanner( - this.cacheTreeRoot, - this.localTreeRoot, - (oldItem, newItem) => - (oldItem.type === newItem.type && String(oldItem.id) === String(newItem.id)), - this.preserveOrder - ) - const serverScanner = new Scanner( - this.cacheTreeRoot, - this.serverTreeRoot, - // We also allow canMergeWith here, because e.g. for NextcloudFolders the id of moved bookmarks changes (because their id is ";") - (oldItem, newItem) => { - if ((oldItem.type === newItem.type && Mappings.mappable(mappingsSnapshot, oldItem, newItem)) || (oldItem.type === 'bookmark' && oldItem.canMergeWith(newItem))) { - newMappings.push([oldItem, newItem]) - return true - } - return false - }, - this.preserveOrder - ) + let localScanner, serverScanner + if (this.localTree instanceof LocalTabs) { + // if we have the cache available, Diff cache and both trees + localScanner = new Scanner( + this.cacheTreeRoot, + this.localTreeRoot, + // We also allow canMergeWith for folders here, because Window IDs are not stable + (oldItem, newItem) => + (oldItem.type === newItem.type && String(oldItem.id) === String(newItem.id)) || (oldItem.type === 'folder' && oldItem.canMergeWith(newItem)), + this.preserveOrder, + ) + serverScanner = new Scanner( + this.cacheTreeRoot, + this.serverTreeRoot, + // We also allow canMergeWith here + // (for bookmarks, because e.g. for NextcloudFolders the id of moved bookmarks changes (because their id is ";") + // (for folders because Window IDs are not stable) + (oldItem, newItem) => { + if ((oldItem.type === newItem.type && Mappings.mappable(mappingsSnapshot, oldItem, newItem)) || (oldItem.canMergeWith(newItem))) { + newMappings.push([oldItem, newItem]) + return true + } + return false + }, + this.preserveOrder, + ) + } else { + // if we have the cache available, Diff cache and both trees + localScanner = new Scanner( + this.cacheTreeRoot, + this.localTreeRoot, + (oldItem, newItem) => + (oldItem.type === newItem.type && String(oldItem.id) === String(newItem.id)), + this.preserveOrder + ) + serverScanner = new Scanner( + this.cacheTreeRoot, + this.serverTreeRoot, + // We also allow canMergeWith here, because e.g. for NextcloudFolders the id of moved bookmarks changes (because their id is ";") + (oldItem, newItem) => { + if ((oldItem.type === newItem.type && Mappings.mappable(mappingsSnapshot, oldItem, newItem)) || (oldItem.type === 'bookmark' && oldItem.canMergeWith(newItem))) { + newMappings.push([oldItem, newItem]) + return true + } + return false + }, + this.preserveOrder + ) + } Logger.log('Calculating diffs for local and server trees relative to cache tree') const localScanResult = await localScanner.run() const serverScanResult = await serverScanner.run() diff --git a/src/test/test.js b/src/test/test.js index e191ad6f27..0917d04f39 100644 --- a/src/test/test.js +++ b/src/test/test.js @@ -4904,10 +4904,10 @@ describe('Floccus', function() { windowType: 'normal' // no devtools or panels or popups }) await browser.tabs.remove(tabs.filter(tab => tab.url.startsWith('http')).map(tab => tab.id)) - await awaitTabsUpdated() } catch (e) { console.error(e) } + await awaitTabsUpdated() if (ACCOUNT_DATA.type === 'git') { await account.server.clearServer() } else if (ACCOUNT_DATA.type !== 'fake') { @@ -5115,14 +5115,14 @@ describe('Floccus', function() { serverMark2 = { title: 'Example Domain', url: 'https://example.org/#test3', - parentId: windowFolderId, + parentId: tree.children[0].id, location: ItemLocation.SERVER } await adapter.createBookmark( new Bookmark(serverMark2) ) - await adapter.updateBookmark({ ...serverMark, id: serverMarkId, url: 'https://example.org/#test2', title: 'Example Domain', parentId: windowFolderId }) + await adapter.updateBookmark({ ...serverMark, id: serverMarkId, url: 'https://example.org/#test2', title: 'Example Domain', parentId: tree.children[0].id }) }) await account.setData({...account.getData(), strategy: 'slave'}) @@ -5148,6 +5148,92 @@ describe('Floccus', function() { false ) }) + it('should sync tabs correctly when merging server and local changes', async function() { + if (ACCOUNT_DATA.noCache) { + return this.skip() + } + const adapter = account.server + const serverTree = await getAllBookmarks(account) + let windowFolderId, serverMark, serverMarkId + await withSyncConnection(account, async() => { + windowFolderId = await adapter.createFolder(new Folder({ + parentId: serverTree.id, + title: 'Window 0', + location: ItemLocation.SERVER + })) + serverMark = { + title: 'Example Domain', + url: 'https://example.org/#test1', + parentId: windowFolderId, + location: ItemLocation.SERVER + } + + serverMarkId = await adapter.createBookmark( + new Bookmark(serverMark) + ) + }) + + await account.sync() + expect(account.getData().error).to.not.be.ok + + let tree = await getAllBookmarks(account) + expectTreeEqual( + tree, + new Folder({ + title: tree.title, + children: [ + new Folder({ + title: 'Window 0', + children: [ + new Bookmark({ title: 'Example Domain', url: 'https://example.org/#test1' }), + ] + }) + ] + }), + false + ) + + let serverMark2 + await withSyncConnection(account, async() => { + serverMark2 = { + title: 'Example Domain', + url: 'https://example.org/#test3', + parentId: tree.children[0].id, + location: ItemLocation.SERVER + } + await adapter.createBookmark( + new Bookmark(serverMark2) + ) + + await adapter.updateBookmark({ ...serverMark, id: serverMarkId, url: 'https://example.org/#test2', title: 'Example Domain', parentId: tree.children[0].id }) + }) + + await browser.tabs.create({url: 'https://example.org/#test4'}) + await awaitTabsUpdated() + + await account.sync() + expect(account.getData().error).to.not.be.ok + + tree = await getAllBookmarks(account) + expectTreeEqual( + tree, + new Folder({ + title: tree.title, + children: [ + new Folder({ + title: 'Window 0', + children: [ + new Bookmark({ title: 'Example Domain', url: 'https://example.org/#test3' }), + new Bookmark({ title: 'Example Domain', url: 'https://example.org/#test2' }), + new Bookmark({ title: 'Example Domain', url: 'https://example.org/#test4' }), + ] + }) + ] + }), + false, + false, // We're merging which doesn't guarantee an order + ) + }) }) }) })