Skip to content

Commit

Permalink
Code review of IndexedDB usage for cache storage purpose
Browse files Browse the repository at this point in the history
Use Promise.prototype.catch() to deal with potential exceptions.

Related issue:
- uBlockOrigin/uBlock-issues#416
  • Loading branch information
gorhill committed Mar 21, 2019
1 parent bbdb08c commit 26c57fe
Showing 1 changed file with 15 additions and 18 deletions.
33 changes: 15 additions & 18 deletions src/js/cachestorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,6 @@
let dbPromise;
let dbTimer;

const genericErrorHandler = function(ev) {
const error = ev.target && ev.target.error;
if ( error && error.name === 'QuotaExceededError' ) {
api.error = error.name;
}
console.error('[%s]', STORAGE_NAME, error && error.name);
};

const noopfn = function () {
};

Expand Down Expand Up @@ -216,20 +208,22 @@
return resolve(null);
}
req.onupgradeneeded = function(ev) {
req = undefined;
const db = ev.target.result;
db.onerror = db.onabort = genericErrorHandler;
const table = db.createObjectStore(
STORAGE_NAME,
{ keyPath: 'key' }
);
table.createIndex('value', 'value', { unique: false });
if ( ev.oldVersion === 1 ) { return; }
try {
const db = ev.target.result;
const table = db.createObjectStore(
STORAGE_NAME,
{ keyPath: 'key' }
);
table.createIndex('value', 'value', { unique: false });
} catch(ex) {
req.onerror();
}
};
req.onsuccess = function(ev) {
if ( resolve === undefined ) { return; }
req = undefined;
db = ev.target.result;
db.onerror = db.onabort = genericErrorHandler;
dbPromise = undefined;
resolve(db);
resolve = undefined;
Expand Down Expand Up @@ -296,7 +290,7 @@
const visitAllFromDb = function(visitFn) {
getDb().then(db => {
if ( !db ) { return visitFn(); }
const transaction = db.transaction(STORAGE_NAME);
const transaction = db.transaction(STORAGE_NAME, 'readonly');
transaction.oncomplete =
transaction.onerror =
transaction.onabort = ( ) => visitFn();
Expand Down Expand Up @@ -333,6 +327,9 @@
keyvalStore[result.key] = result.value;
})
);
}).catch(reason => {
console.info(`cacheStorage.getAllFromDb() failed: ${reason}`);
callback();
});
};

Expand Down

12 comments on commit 26c57fe

@uBlock-user
Copy link
Contributor

@uBlock-user uBlock-user commented on 26c57fe Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the latest dev build, this commit show outdated filterlists on startup only on Firefox with cacheStorageAPI set to indexedDB, even though they were updated a day ago.

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox version is 67.0 if it's any help.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything in the browser console? If you use indexedDB and for some reasons it fails to instantiate properly, the lists will never be up to date. Using unset will let uBO fallback onto browser.storage.local.

I am using Nightly and not experiencing any issue here.

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing. Empty as usual, this happens after I close the browser and run it again.

for some reasons it fails to instantiate properly, the lists will never be up to date. Using unset will let uBO fallback

If that's considered acceptable, then I will set to unset in Firefox.

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unset will let uBO fallback onto browser.storage.local.

no change using unset either, situation is the same.

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to the latest Nightly and browser console has this in it --

Nothing related to uBO as it seems.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure, can you also look into uBO's dev console also? (you can reach uBO's dev console from about:debugging#addons)

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above picture is from about:debugging -> Debug for uBlock Origin.

Well, it doesn't happen if I reset to default settings, but I do have backup of my uBO configuration and if I use that to restore it, it happens.
uBO.txt

Uploaded my uBO configuration file for you to reproduce.

Anyways, I managed to fix it, I resetted back to default setting for uBO and one by one activated settings to reach my uBO configuration, restarted every time and I can't reproduce anymore, must have been corrupted cache storage or something.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be a free disk space issue? uBO's indexedDB may be evicted if free disk space reaches a threshold -- forgot exactly, maybe 1/5 of 50% free space.

@uBlock-user
Copy link
Contributor

@uBlock-user uBlock-user commented on 26c57fe Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox is on D drive on my Windows 10 installation with a free space of 211 GB out of 212 GB, so no that's not the issue.

I downgraded to 1.18.9rc2 and it happened there too, so something screwed up in Firefox apparently.

Can you reproduce with my uBO configuration ?

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to reproduce with your config, including after waiting for a selfie.

@uBlock-user
Copy link
Contributor

@uBlock-user uBlock-user commented on 26c57fe Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not happening anymore so it's all good now.

Edit: It seems I can no longer use Restore from file functionality anymore as the issue is reproduced again with that.

Please sign in to comment.