1. Run the "storage_test_coverage_2" patch from bug #1382598 through Try on all the environments 2. Check the results Expected results: No debug failures are triggered. Actual results: Debug failures like https://treeherder.mozilla.org/logviewer.html#?job_id=126449819&repo=try&lineNumber=9320 are triggered in the Gecko code. Please see the entire job https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd4ae50f904676c18363e90d028a88fbc7e2a6d7
The failure happens in QuotaManager.
No longer blocks: 1382598
Component: DOM: Service Workers → DOM: Quota Manager
Sorry, I somehow change the dependency...
Yeah, we shouldn't delete itself while freeing disk space.
The failure happens in eviction on all platforms. I guess storage.estimate() creates the temporary type of originInfo object and indexedDB creates the default type of originInfo object. After a while, the temporary originInfo is collected to remove when the QM get inactive origins to be evicted. Thus, the test hits the assertion (for having the same origin string). I'll test to verify whether it's correct or not.
It seems that it only happens on try. I cannot reproduce it on my desktop (ubuntu 14.04). try with a quick patch to verify my guess for this issue: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6ab450dcf8bbf4ad15bbb6bb5347c6a85b5c3ab
I was in opt build, but I find out I can reproduce it on debug build. So, the reason is that: First, testing script uses storage estimate() to create temporary origin and it also uses indexedDB to create default origin. (both origins are the same string) Then, the testing script keeps storing objects by indexedDB until triggering eviction. Next, GetInactiveOriginInfos() collects inactive origins by checking if the origin holds the directory lock. (at this time storage estimate() have released its directory lock) Finally, the assertion checks if the evicted origin (created by storage.estimate(), temporary) is the same origin string with the directory lock(holding by indexedDB, default) and this hit the assertion. I believe there are two issues in this bug. 1. storage.estimate() shouldn't create an empty temporary originInfo, and I've filed bug 1373183 for it. 2. Not sure if this is an issue. Should we evict an origin if the complementary type (default <-> temporary) of origin holds the directory lock? If so, we should modify the assertion on MaybeUpdateSize()  to check the persistence type as well. If not, we should skip this kind of origin if the complementary type of origin holds a directory lock when collecting inactive origins.  http://searchfox.org/mozilla-central/source/dom/quota/ActorsParent.cpp#3075-3076
(In reply to Tom Tung [:tt] from comment #5) > try with a quick patch to verify my guess for this issue: try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=609a61b9083ee1d27a2de87d0ceb9e7d4cbc4792&selectedJob=126964941
Created attachment 8902652 [details] [diff] [review] Modify assertion to allow QM to evict the origin in the different persistence type.
Attachment #8902652 - Flags: review?(jvarga)
try result for all platform with patches here and in bug 1382598: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b39ee585cc5f8ef2a4fa26ec14513f3ae805bbcb
10 months ago
Summary: Debug failures in Gecko code when running patches through Try → Evicting the same origin string but in the different persistence type hits the assertion in MaybeUpdateSize()
Created attachment 8903004 [details] [diff] [review] [Final] Bug 1394745: Modify the assertion to allow the QM to evict the origin in the different persistence type. r=janv Update the commit message.
Mark as checkin-needed since the try looks ok
10 months ago
Attachment #8903004 - Attachment description: Bug 1394745: Modify the assertion to allow the QM to evict the origin in the different persistence type. r=janv → [Final] Bug 1394745: Modify the assertion to allow the QM to evict the origin in the different persistence type. r=janv
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/c71b4f46c8e1 Modify the assertion to allow the QM to evict the origin in the different persistence type. r=janv
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
tracking this in storage meta bug
You need to log in before you can comment on or make changes to this bug.