-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iOS version deletes the data when being disabled #840
Comments
Thank you so much for the very clear explanation. I will show that to the reporter and see if that helps. Like you say, though, my best guesses too at this point are:
|
Hey there, I am said user who sent the report on Twitter. To be honest, I'm a bit dumbfounded, as I'm unable to reproduce the issue. Dictionary data currently survives both deactivating and disabling the extension, as per the terminology above, and as as per the behaviour observed by @shirakaba. I thought maybe the storage was being cleared by the 7-day cap imposed by ITP due to my infrequent usage, which would be a surprising thing to do for extensions (couldn't find any documentation on this), but the Dictionary data does actually survive changing my iOS clock to 7+ days later. Dictionary data also survives reboots. To be honest, this is a problem I had weeks ago at least (possibly on a beta build of 15.1), hadn't used the extension since, and when I checked the Extension Settings Menu the other day to see if it was still refreshing the Dictionary data and saw “No database”, I thought this was an actual proof of the problem. In reality though, this appears to be a message that is shown when the database is loading: 10ten-ja-reader/src/options.ts Lines 1066 to 1075 in 0d8c86e
From hibiki-data, I can see that the Initializing DataSeriesState may mean the application doesn't yet know whether there's a valid database or not (merely making suppositions based on a cursory look at the code here): So, in certain cases, I can get the “No database” message to show even though the database hasn't been cleared. Eventually, reloading the Extension Settings Menu does show the database versions, although I'm unable to get reliable steps to reproduce this. It only appears after I've stopped using my phone for a few minutes, or when I've been doing other things on my phone for a few minutes (probably something in RAM being cleared): 10tenbug.mp4The key step in getting the database details to show appears to be force quitting Safari. In the video, I also quit the Settings app, but this makes no difference. (Don't mind the date in the future, it's from when I toyed with the clock to test the ITP policies) Refreshing the database on my 7 Plus would usually take several minutes, so here, it doesn't look like the database is actually being refreshed at any point. It's just the message thing from the Initializing sate. Therefore, I think perhaps there was a Safari storage bug in the version of iOS I was using that has since been fixed, and the “No database” message wasn't an indication of this supposed bug. My iPhone 7 Plus has 53.3 GB storage free out of 128. |
Hi @Pacoup! Thank you very much for following up here! I checked out my notes of "things to look into later for the iOS version" and I noticed I have a note to "Investigate why DB sometimes becomes unavailable on iOS" so I guess I encountered that too at some point.
When I've observed the data apparently being unavailable it was only a few minutes or so after it was available so I agree it's probably not the 7-day time limit. Thank you for checking that though!
Yes, you're right. We should change that to say "Initializing..." instead so at least we can differentiate between the two cases.
This is the weird bit and matches what I believe I saw too on my iPhone 7 (not Plus). Thank you for all the other details and the screencast too. That's incredibly helpful. One other piece of information that I came across this morning that might be helpful is that Safari for iOS actually doesn't keep background/event pages live even with active ports:
(source w3c/webextensions#134 (comment)) So tying together all these observations, my best guess is that after some time the event page is suspended. This might happen more often when memory is low, and perhaps the fact that we've seen this on two iPhone 7's (2Gb / 3Gb) but not a more recent iPhone 11 (4Gb) points to that. After that, when we next try to communicate with the background page either by looking something up or by opening the options page, the database state is reported as "initializing". In the options screen, however, we conflate "initializing" with "no database" so we can't tell. That doesn't explain why the data is re-downloaded but it sounds like that no longer happens (which is a relief). There certainly have been some significant IndexedDB bugs fixed in Safari recently so it's possible that it was due to a bug that has been fixed 🤞 It also doesn't quite explain quite why it takes so long for the database state to update in that screencast. Perhaps Safari is just really slow at initializing IndexedDB (possible) or perhaps we're not updating the database state correctly. Next steps:
|
This really seems to be memory-related. I have new behaviours.
I was initially under the impression that the dictionary lookup always works, and that opening the options through the dictionary popup always shows the database versions. This still seems to be true (versus what I show below), which suggests something may be causing one of the events here not to fire when opening options via the Settings app. Is the options page being suspended, closing the Port perhaps, or is the onload event not fired for some reason or the code is unable to acquire a new Port? 10ten-ja-reader/src/options.ts Lines 781 to 814 in 0d8c86e
My understanding is the sate isn't updated at all. You have to force quit Safari. The Check for updates button does nothing. In both cases, the data doesn't appear to re-download. It's still there. New behavioursUpon trying to confirm the database versions are always shown when loading options via the dictionary popup, I stumbled upon something else. My procedure was to open as many apps as possible to make sure the memory-related issue was triggered without having to leave my phone aside for an as-of-yet unknown amount of time required for the issue to show up. Trying the dictionary lookup after opening all the apps, I was faced with a popup window without a tab bar: Heading into the options from the Settings app, I was faced with this new state. No database, but a Last check message is present: Tapping on the Check for updates button this time actually performs the entire database download. The DB appears to have been deleted as I had observed some weeks ago: Could something be causing the database to exceed the 10 MB limit set by the unlimitedStorage permission? Doesn't seem to be the problem, however, since the database works fine for a while. My impression is Safari doesn't have persistent storage permissions yet (or apparently, this is being added in 15.2) and the total disk space allotted to Safari storage is quite low, and possibly shared with other web views. Since a lot of apps are web view heavy, opening many apps could have caused Safari to evict some IndexedDBs in storage to make space. It wouldn't make sense that these are stored in RAM, but perhaps iOS considers web storage to be more of a cache, and loads parts of it in RAM for performance, hence the aggressive eviction policy. Or they just don't want Safari storage to blow up. This could also explain the behaviour observed on the iPhone 11, as it is not impossible that newer iPhones have larger storage quotas in line with more system memory, although I feel like the storage eviction and the No database message issues are two different problems. However, the No database message showing up could be related to total memory anyway. Nevertheless, this could all be wrong, as the dictionary still appears functional before checking for updates, so the IndexedDB wasn't evicted and something else is causing the re-download (if that's even what's happening, I'm not sure about the update process)? |
…ound page stays alive see #840 (comment)
Thanks @Pacoup! That's super helpful. I've written up a little summary below of how 10ten does lookups in case that helps interpreting the different states you see. Background: how 10ten does lookups10ten comes bundled with a snapshot of the word dictionary that it can load into memory to do lookups. This in-memory snapshot is used whenever we can't access the IndexedDB database which most often happens either (a) when the add-on has recently been installed and we haven't finished downloading the data yet, or (b) when we can't store data for some reason (e.g. Firefox users who have set their browser to "always private mode" or because we reached the storage quota). This ensures we can always return something even if it's potentially out-of-date. The snapshot doesn't include localized glosses and doesn't include the name or kanji dictionaries so we miss out on that information too. Generally we prefer to use IndexedDB because we can update the data frequently without the user having to download and install potentially large updates and because it requires less memory. That is, if we detect the IndexedDB database is available then we never load the snapshot into memory. When you see the popup with no tabs, that means it has fallen back to the snapshot in memory due to conditions described below. One complication, however, is that we have a lookup cache in the content script. It's very small (only about 10 entries or so) and exists mostly because very often the user will scan the cursor back and forth within a word and we don't want it to perform redundant lookups each time the cursor moves to a different character. We don't cache lookups when they come from the snapshot since we expect that's fast enough without the cache and will soon be replaced by the up-to-date IndexedDB version anyway. As a result, when debugging this it's entirely possible that you think you're getting results from the IndexedDB version of the dictionary (because you see all the tabs), but you're actually just getting a cached entry. Getting to your comments...
I think we can rule out the Furthermore, I think that if we failed to acquire a port in the first place--either because something earlier in that function threw, or because
I wonder if that's because we have a disconnected port that silently fails when we try to post to it, as opposed to the port not being established at all? For now I've added some code to try to detect the port being disconnected and re-connect just to see if that helps.
This state indicates that the word database is either:
as per this code: 10ten-ja-reader/src/background/jpdict.ts Lines 111 to 130 in 8a0e313
For now I've updated the code that reports the database state in the options page so we can differentiate between the above cases a bit more easily. As per your observations though, it seems like this is a case of the database being empty.
Oh, I hadn't seen that. That's quite interesting.
I didn't know that either. I really hope that helps, but given that you're on 15.2 beta 3 it seems unlikely.
That certainly seems possible. I really don't know what Safari is doing. As we discovered when implementing the puck, Safari seems to have a lot of interesting heuristics! I agree it seems likely that Safari is simply tossing out the IndexedDB data based on ✨conditions✨. If we could work out some reliable steps to reproduce this then we have a few contacts at Apple who might be able to confirm/deny our suspicions. However, if the answer is that we really can only rely on 10Mb of data, then I'm not sure what we can do except going back to bundling all the dictionaries with the add-on and loading them all into memory. That would be a shame to have to support that configuration given that for most users/platforms it's unnecessary. I am currently reworking the setup for downloading the data files to hopefully make that a bit more streamlined (avoiding downloading 60 patch updates for the name dictionary for example). Once that's done it might make re-downloading a little less painful but adding hundreds of thousands of records to IndexedDB is still going to take time (typically much longer than downloading the same records). (If only Chrome would get on board with Mozilla's "Limited Event Pages" we could use For now I'm adding the extra diagnostics I mentioned above. I will try to implement the touchscreen version of the "Copy to clipboard" function and then trigger a release some time next week. Hopefully after that we can get a clearer picture of what is going on. |
Thanks, super helpful.
That sounds correct. I'll test when I get the update.
Yeah, not sure what the expectation is with regards to getting this permission through a web extension. Perhaps this behaves like Firefox and requires some UX for the user to approve:
Do you mean the current IndexedDB exceeds 10 MB? I can see 163 MB in Chrome, but I imagine that if there was an actual 10 MB limit in Safari Web Extensions, you would get some kind of quota error. Perhaps this works similarly to Firefox and Chrome:
So if the free space on your hard drive is 500 GB, then the total storage for a browser is 250 GB. If this is exceeded, a process called origin eviction comes into play, deleting an entire origin's worth of data until the storage amount goes under the limit again. There is no trimming effect put in place to delete parts of origins — deleting one database of an origin could cause problems with inconsistency. It does sound like Safari is evicting the origin for the 10ten extension as described in Firefox's process, but the total storage allocation may be quite low comparatively: This makes it look like the default quota is 1 GB, and per origin is 100 MB, although there appears to be a mechanism for requesting more storage: I found these from the original commit that added the storage quotas for IDB: https://trac.webkit.org/changeset/237700/webkit/ Although this was later changed to a storage-generic quota: https://bugs.webkit.org/show_bug.cgi?id=196545 Besides the persistent-storage permission, losing the IDB data occasionally seems to be unavoidable. |
Thank you again for following up on this!
Yes, this is a very odd area. In Firefox, for Web Extensions even if you specify
Yes. I haven't checked what Safari's disk usage is but I believe Firefox ends up using about 400Mb of storage. The downloaded file size should be in the order of 10~20Mb but once we extract it and stick it into IndexedDB and add a few indices, the corresponding SQLite file it stores on disk is massive. I believe Safari uses SQLite for storing IndexedDB so I expect the actual storage used there is of similar magnitude. (Apparently, Blink is the only engine that doesn't use SQLite for IndexedDB and is slower as a result.)
Yes, indeed.
Yes, this is a bit annoying. There is a proposal for Storage Buckets that would allow us to, say, put the names dictionary in a separate bucket and let that be evicted first, but no one has implemented that spec yet.
Oh nice! Thanks for looking those up!
Right. It's not entirely clear what quotas are extended to Web Extensions or even if we're hitting that quota or simply some other kind of heuristic. I couldn't see any mention of handling of For what it's worth my iPhone 7 has 114Gb of free storage (it's a test phone with almost nothing else installed on it) and I've certainly seen the "database not available" issue. I don't know that I've seen it re-download the data however. |
I've finally released 1.6.0 which should have the extra diagnostics. (The touchscreen version of "Copy to clipboard" took a lot longer than anticipated 😅) |
I received the following report on twitter:
I'm trying to clarify how the user is disabling the extension but I wonder if anyone has seen this? @shirakaba perhaps?
The text was updated successfully, but these errors were encountered: