Created attachment 539045 [details] [diff] [review] Fix indexeddb mochitests to run green in Fennec.
A very serious regression (bug 715074) just arose because we're not running these tests. Anything preventing this patch from landing?
How do I enable these tests on Fennec (in a patch for testing on the try server) ?
The tests should just work on native Fennec without this patch. I have no idea why we're not running them.
if the tests run fine, please add them to the http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/android.json file.
jmaher tells me that there's no way to say "these tests work in native fennec but not xul fennec"
hmm, it appears that android.json isn't supported yet in releng as per bug 695351. I filed bug 664029 to track this.
Why should they not run in xul fennec?
the tests failed there, I believe due to e10s issues with indexedDB and/or the tests.
If I recall correctly, we had something of a hacky remoting stopgap that should have worked. If it doesn't, we need to fix it. Will chat with bent/sicking. There were longer-range plans for a better one but scheduling that is harder. In this case, we should enable the tests. But I don't remember correctly and there wasn't a hacky remoting stopgap, OK, makes sense to have them off.
IDL (allegedly) works in e10s, in a single process (I haven't actually tried it myself, wouldn't be surprised if we broke it). The tests set preferences and permissions though, which is why they don't work.
That's IDB, of course.
We have support for cross-process friendly pref-setting now. ISTR we did the same for permissions. We really need to get these tests back on: one product (firefox/mobile/tablet) is currently shipping the code (apparently completely untested) and will continue to do so for the foreseeable future, and another product (b2g) will soon be shipping it.
Feel free to add directories to the testing/mochitest/android.json file and run them on try. If they work land them and you have new tests turned on.
https://tbpl.mozilla.org/?tree=Try&rev=5b12dfdfcccb https://tbpl.mozilla.org/php/getParsedLog.php?id=8788674&tree=Try&full=1 19870 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_deleteDatabase.html | indexedDB error, code 12 19884 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_deleteDatabase_interactions.html | indexedDB error, code 12 20184 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_os_delete.html | OS file deleted - got 864000, expected 76400 we need to fix that
ok, it seems sqlite3_quota_remove() doesn't work on mobile https://hg.mozilla.org/try/rev/253a99839007
we need this for unprefixing IDB
A SpecialPowers permissions API corresponding to pushPrefEnv was never created. We need to create that, then update the patch in this bug to use it.
At least, the previous comment is in regards to B2G's e10s model. I don't exactly understand what unprefixing IDB has to do with this patch.
We don't want to unprefix an API which is basically completely untested on mobile.
Jan: Does your comment 17 mean that IndexedDB doesn't currently work in Fennec? If so, can you file a separate bug on that?
Comment on attachment 539045 [details] [diff] [review] Fix indexeddb mochitests to run green in Fennec. This should have landed as part of e10s stuff. Tests are still orange, but for a different reason.
I have ran these recently without this patch and 3 tests timeout and the rest take a long time, before we turn these on, can we make a pass through to ensure these tests are running without major kinks to slow them down?
There are a few tests which run noticeably slower on android: test_add_put.html test_complex_keyPaths.html test_clear.html test_index_update_delete.html And there are a few that timeout: test_deleteDatabase.html test_deleteDatabase_interactions.html test_file_os_delete.html These are all unrelated to the patch, but related to getting indexedDB tests running on android.
Joel, when I pushed a patch to try which enabled the IDB tests, the test you mention as timing out straight up failed for me, with what looked as legitimate failures. Bent is looking into this though.
Created attachment 632030 [details] [diff] [review] Fix. This was our fault. Fixed.
(In reply to ben turner [:bent] from comment #27) > Created attachment 632030 [details] [diff] [review] > Fix. > > This was our fault. Fixed. So databases and stored files are not quota tracked on Android ?
when I run these locally on my tegra, I get this: 2269 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_deleteDatabase.html | indexedDB error, 'VersionError' 2270 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_deleteDatabase.html | an unexpected uncaught JS exception reported through window.onerror - [object Event] at undefined:undefined 2273 INFO TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_deleteDatabase_interactions.html | finished in a non-clean fashion (in /tests/dom/indexedDB/test/test_deleteDatabase.html) 2572 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_os_delete.html | OS file deleted - got 864000, expected 764000 I had mentioned these specific test files before. this is while running with --test-path=dom/indexedDB/test.
Wait, they're passing on tinderbox right?
yes, they are passing, I double checked that. I was just confused as I can't see them passing on my local tegra with a build from tinderbox.
This fixes a bug where we didn't properly clear databases in some racy conditions. It bit android tests in particular, but it can happen on any platform.
Comment on attachment 632030 [details] [diff] [review] Fix. [Approval Request Comment] Bug caused by (feature/regressing bug #): IndexedDB (been around forever) User impact if declined: Some databases may persist after pages request their deletion. Testing completed (on m-c, etc.): Good tests on m-c, even fennec. Risk to taking this patch (and alternatives if risky): This is a very small and very safe patch. I don't expect any risk, to be honest. String or UUID changes made by this patch: None.
Comment on attachment 632030 [details] [diff] [review] Fix. Please land on mozilla-aurora and mozilla-beta, default branch (not 14.0 release branch).