From f62d866b361fcdee1947ca2e8484545a75487d89 Mon Sep 17 00:00:00 2001 From: Raymond Hill Date: Thu, 28 Mar 2019 10:17:47 -0300 Subject: [PATCH] Code review implementation of cacheStorage.clear() Possibly related issue: - https://old.reddit.com/r/firefox/comments/b3u4nj/what_is_the_estimated_time_period_for_reviewing_a/ @gwarser has been able to reproduce at will, while I have been unable to reproduce at all. The change here is to clear the content of the database instead of outright deleting it before restoring backed up settings. --- src/js/cachestorage.js | 20 ++++++++++---------- src/js/messaging.js | 22 +++++++++++++--------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/js/cachestorage.js b/src/js/cachestorage.js index 6538732ac..2f9b16968 100644 --- a/src/js/cachestorage.js +++ b/src/js/cachestorage.js @@ -211,10 +211,7 @@ if ( ev.oldVersion === 1 ) { return; } try { const db = ev.target.result; - const table = db.createObjectStore( - STORAGE_NAME, - { keyPath: 'key' } - ); + db.createObjectStore(STORAGE_NAME, { keyPath: 'key' }); } catch(ex) { req.onerror(); } @@ -422,15 +419,18 @@ if ( typeof callback !== 'function' ) { callback = noopfn; } - disconnect(); - try { - const req = indexedDB.deleteDatabase(STORAGE_NAME); - req.onsuccess = req.onerror = ( ) => { + getDb().then(db => { + let transaction = db.transaction(STORAGE_NAME, 'readwrite'); + transaction.oncomplete = + transaction.onerror = + transaction.onabort = ( ) => { callback(); }; - } catch(ex) { + transaction.objectStore(STORAGE_NAME).clear(); + }).catch(reason => { + console.info(`cacheStorage.clearDb() failed: ${reason}`); callback(); - } + }); }; return getDb().then(db => { diff --git a/src/js/messaging.js b/src/js/messaging.js index 9904df11a..c0ec034c8 100644 --- a/src/js/messaging.js +++ b/src/js/messaging.js @@ -799,17 +799,21 @@ var backupUserData = function(callback) { µb.assets.get(µb.userFiltersPath, onUserFiltersReady); }; -var restoreUserData = function(request) { - var userData = request.userData; +const restoreUserData = function(request) { + const userData = request.userData; - var restart = function() { + const restart = function() { vAPI.app.restart(); }; - var onAllRemoved = function() { + let countdown = 2; + + const onAllRemoved = function() { + countdown -= 1; + if ( countdown !== 0 ) { return; } µBlock.saveLocalSettings(); vAPI.storage.set(userData.userSettings); - var hiddenSettings = userData.hiddenSettings; + let hiddenSettings = userData.hiddenSettings; if ( hiddenSettings instanceof Object === false ) { hiddenSettings = µBlock.hiddenSettingsFromString( userData.hiddenSettingsString || '' @@ -840,7 +844,7 @@ var restoreUserData = function(request) { // If we are going to restore all, might as well wipe out clean local // storage - µb.cacheStorage.clear(); + µb.cacheStorage.clear().then(( ) => { onAllRemoved(); }); vAPI.storage.clear(onAllRemoved); vAPI.localStorage.removeItem('immediateHiddenSettings'); }; @@ -856,9 +860,9 @@ const resetUserData = function() { vAPI.app.restart(); } }; - µb.cacheStorage.clear().then(( ) => countdown()); // 1 - vAPI.storage.clear(countdown); // 2 - µb.saveLocalSettings(countdown); // 3 + µb.cacheStorage.clear().then(( ) => countdown()); // 1 + vAPI.storage.clear(countdown); // 2 + µb.saveLocalSettings(countdown); // 3 vAPI.localStorage.removeItem('immediateHiddenSettings'); };