Refactor how the database is read in ViewHistory #4705
Conversation
| // // See comment in try-catch block above. | ||
| // sessionStorage.setItem('pdfjsHistory', databaseStr); | ||
| // } catch (ex) {} | ||
| // sessionStorage.setItem('pdfjsHistory', databaseStr); |
There was a problem hiding this comment.
Actually removing try-catch here might not have been such a great idea, given that the preference network.cookie.lifetimePolicy could change after the viewer was loaded. (There should probably be a try-catch around the localStorage.setItem call 4 lines down too.)
Another, better (?), solution might be to add a Promise in _writeToStorage as well. Opinions?
There was a problem hiding this comment.
solution might be to add a Promise in _writeToStorage as well
yes we should
There was a problem hiding this comment.
I've addressed this, but the question is how far we want to go?
If we want to make ViewHistory completely async that might be better to do in yet another PR, since the diff in this one is becoming quite messy already.
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/c1ea9310a9f399f/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/c1ea9310a9f399f/output.txt Total script time: 0.38 mins Published
|
Refactor how the database is read in ViewHistory
|
Thank you for the patch |
This PR re-factors
ViewHistoryso that reading stored values returns a Promise. It should thus handle any possible errors when eithersessionStorageorlocalStoragefails (e.g. bug 1000777 and #4690).My goal with this PR was to address #4679 (comment), so the
ViewHistorywill now fail to initialize when the database cannot be read.As part of the re-factoring I've also taken the opportunity to simplify the, in my opinion, quite clunky initialization code.
I didn't want to touch too much code, so I refrained from rewriting the entire
ViewHistoryto be Promise friendly and just settled for the necessary part.Also fixes #4690 (by courtesy of the first commit).