1. https://www.amarujala.com./ on Windows

Assertion failure: StringBeginsWith(path, directoryPath), at z:/build/build/src/dom/quota/ActorsParent.cpp:4617
#01: mozilla::dom::quota::QuotaManager::GetQuotaObject(__int64,nsTSubstring<char16_t> const &) [dom/quota/ActorsParent.cpp:4729]
#02: static struct already_AddRefed<mozilla::dom::quota::QuotaObject> `anonymous namespace'::GetQuotaObjectFromNameAndParameters(const char *, const char *) [storage/TelemetryVFS.cpp:316]
#03: static int `anonymous namespace'::xOpen(struct sqlite3_vfs *, const char *, struct sqlite3_file *, int, int *) [storage/TelemetryVFS.cpp:627]
#04: static int sqlite3BtreeOpen(struct sqlite3_vfs *, const char *, struct sqlite3 *, struct Btree * *, int, int) [third_party/sqlite3/src/sqlite3.c:65869]
#05: static int openDatabase(const char *, struct sqlite3 * *, unsigned int, const char *) [third_party/sqlite3/src/sqlite3.c:159438]
#06: mozilla::storage::Connection::initialize(nsIFileURL *) [storage/mozStorageConnection.cpp:687]
#07: mozilla::storage::Service::OpenDatabaseWithFileURL(nsIFileURL *,mozIStorageConnection * *) [storage/mozStorageService.cpp:667]
#08: mozilla::dom::cache::OpenDBConnection(mozilla::dom::cache::QuotaInfo const &,nsIFile *,mozIStorageConnection * *) [dom/cache/DBAction.cpp:234]
#09: mozilla::dom::cache::DBAction::OpenConnection(mozilla::dom::cache::QuotaInfo const &,nsIFile *,mozIStorageConnection * *) [dom/cache/DBAction.cpp:147]
#10: mozilla::dom::cache::DBAction::RunOnTarget(mozilla::dom::cache::Action::Resolver *,mozilla::dom::cache::QuotaInfo const &,mozilla::dom::cache::Action::Data *) [dom/cache/DBAction.cpp:102]
#11: mozilla::dom::cache::Context::QuotaInitRunnable::Run() [dom/cache/Context.cpp:326]

I'll look into this. It's probably related to recent change of tunneling quota info to SQLite.

It's a known issue in Bug 1516333. So ending char '.' is not allowed in Windows and it seems it would create a file/directory without ending char '.' while we are creating one.

I haven't attached a debugger in Windows, but I assume the condition is like below:
So in QuotaManager::GetQuotaObject the path from the file is without '.' ending and directoryPath is made based on aOrigin (with '.' ending).

It's not related to my recent fix for using DriectoryLockId for tunneling quota info. It's a bug on Windows platform doesn't allow a filename ends with '.'.

Yeah, we fixed a similar problem in IndexedDB by adding a suffix to the directory name.
So for example https+++www.amarujala.com. would become https+++www.amarujala.com..data on disk

I'm not saying this is the only solution, maybe it can be fixed differently.
Anyway, the suffix would require a new major storage upgrade.

One more note: It seems this issue is on DOM Cache and LocalStorage. We might need somehow align it with all quota clients if we eventually find another way to solve this.

The issue can potentially happen with any quota client because the origin directory (shared by all quota clients) misses the dot on Windows. Once the issue is fixed, all quota clients will be fixed, no ?
IDB will just continue using the .files suffix for stored blobs/files.

(In reply to Jan Varga [:janv] from comment #5)

The issue can potentially happen with any quota client because the origin directory (shared by all quota clients) misses the dot on Windows. Once the issue is fixed, all quota clients will be fixed, no ?
IDB will just continue using the .files suffix for stored blobs/files.

Ah, I see. I thought you meant IDB somehow avoid this issue by adding suffix. Then, please ignore my comment in comment #4

I think this can be a problem for quota objects only when we lose .metadata-v2 file and we need to recover origin string from origin directory name.

(In reply to Jan Varga [:janv] from comment #7)

I think this can be a problem for quota objects only when we lose .metadata-v2 file and we need to recover origin string from origin directory name.

Hmm, I need to look into the code more and run a custom build on Windows to understand the code path more.

I added this signature because I created two crash reports in that signature on my Windows machine yesterday when I tried to reproduce the issue (by visiting the website). So my understanding is that: if it's a debug build, we would hit this assertion. If it's not, we might get either a wrong quotaObject or a nullptr. And thus crash at the point of signature. And while I reproduced the issue, I didn't check if the .metadata-v2 file is lost. I will verify this.

Yeah, maybe MozURL removes the dot. I don't know, it needs to be investigated.

(In reply to Jan Varga [:janv] from comment #3)

Yeah, we fixed a similar problem in IndexedDB by adding a suffix to the directory name.
So for example https+++www.amarujala.com. would become https+++www.amarujala.com..data on disk

I haven't looked into Chromium's code yet, but my profile for Chrome Canary has something like this approach. (https_www.amarujala.com._0.indexeddb.leveldb for indexedDB; https_mail.google.com_0 for databases)

So, there's a suffix, right ?

(In reply to Jan Varga [:janv] from comment #11)

So, there's a suffix, right ?

Yes, it looks like they have suffixes for files/directories for origins (I assumed you meant Chrome's implementation)

So, there's another crash report for https://www.amarujala.com./
https://crash-stats.mozilla.org/report/index/caabe351-7ee2-4e56-aad5-0c9770191030

So it shows up in bug 1576593 too.

While I tested the website mentioned in comment #0 with a debug build on Windows, there is no warning/failure related to QuotaManager before we hit the assertion (StringBeginWith).

The origin was: "https://www.amarujala.com."
The path was: "/path/to/profile/storage/default/http+++www,amarujala.com/cache/cache.sqlite"
The directoryPath was: "/path/to/profile/storage/default/http+++www,amarujala.com./cache"

Note that

• the leafName in EnsureOriginDirectory was "https+++www.amarujala.com."
• If I get the leafName after the creation of the directory in EnsureDirectory, it's "https+++www.amarujala.com."
  These indicate aDirectory->Create() doesn't provide any hint/warning but return an NS_OK with creating the "https+++www.amarujala.com" folder.

At this moment, add some suffix to the origin folder seems to be a reasonable way to fix the issue. But it requires a major upgrade. Though we can only add a suffix to these special origin folders, that might increase complexity to our existing system.

If we want to have a major upgrade, I guess that it would be probably better to do that after enabling LSNG and fixing existing initialization issues.

If there are not many sites like this one, we can just wait for the new QM storage.

I think there's a general platform level issue here. It's our convention that we assume all domains are absolute and there's no trailing period, but we're obviously allowing this ambiguity to exist. This has come up before like in https://bugzilla.mozilla.org/show_bug.cgi?id=1342282#c8.

:annevk pointed me at bug 134402. My take-away is that we do need to support the trailing period case and it may be appropriate for the megabar/navigation UX to try and help the user lose the trailing dot when appropriate, but QuotaManager shouldn't be second guessing.

This should be fixed by Bug 1593365 if there is no other urgent request.

Aggregating the crashes on major version number gives me:

1 	91 	2581 	55.09 %
2 	102 	1815 	38.74 %
3 	100 	193 	4.12 %
4 	104 	57 	1.22 %
5 	101 	11 	0.23 %
6 	103 	9 	0.19 %
7 	105 	8 	0.17 %
8 	99 	6 	0.13 %
9 	98 	5 	0.11 %

No crashes with 106+, seems to be mostly an ESR problem now.

Edit: No, the signature changed. Still we have much less of them now.