778011 - several dom/imptests/html/dom/ranges/test_Range-xxx mochitests disabled on android

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Geoff Brown [:gbrown]]

In bug 775227, several mochitests in dom/imptests/webapps/DOMCore/tests/approved have been disabled on android (in android.json) on suspicion of causing out of memory errors on the Tegras. Additional investigation is required to verify this test's memory use and determine if it can be re-enabled on Android.

The disabled tests are:

"dom/imptests/webapps/DOMCore/tests/approved/test_Range-cloneContents.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-compareBoundaryPoints.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-deleteContents.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-extractContents.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-insertNode.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-mutations.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-set.html"
"dom/imptests/webapps/DOMCore/tests/approved/test_Range-surroundContents.html"

Comment 1

[Geoff Brown [:gbrown]]

Attachment: logcat showing test_Range-cloneContents failure

Comment 2

[Geoff Brown [:gbrown]]

I ran additional tests of test_Range-cloneContents.html on a Galaxy S, loading just this single test; OOM problems persisted (ddms system info showed a dramatic decline in free memory to near zero, the Android low memory killer killed all background processes, and fennec memory pressure was often triggered).

I found that simply reducing the range of tests made a dramatic difference; with this change:

-for (var i = 0; i < testRanges.length; i++) {
+for (var i = 0; i < testRanges.length / 2; i++) {

the test completed reliably with no sign of OOM / low memory.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I'm guessing the problem here is that a pair of iframes is created for each test.  I did this to make debugging easier, as it says at the top of the test:

"""
To debug test failures, look at the URL of the hidden iframes corresponding
to the test at the end of the document, load those individually, and use your
browser's debugging tools.
"""

It would be a lot more efficient to get rid of iframes we were done using.  test_Range-insertNode.html does this, and allow you to debug using a different technique:

"""
To debug test failures, add a query parameter with the test id (like
"?5,16").  Only that test will be run.  Then you can look at the resulting
iframes in the DOM.
"""

So porting all the iframe-using Range tests to use that technique would probably fix it.

Comment 4

[Andrew McCreight [:mccr8]]

These tests were toned down a bit, so possibly they could be reenabled on Android.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to :Aryeh Gregor from comment #3)
> So porting all the iframe-using Range tests to use that technique would
> probably fix it.

I have done this in bug 933072.

Comment 6

[Andrew McCreight [:mccr8]]

Geoff, who could look into reenabling this?  roc fixed this test so it shouldn't use up a ton of memory now.

Comment 7

[Geoff Brown [:gbrown]]

I can take that on -- will try to get to it in the next week or so.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Dammit, mid-aired.

I sent a patch to enable them to try: <https://tbpl.mozilla.org/?tree=Try&rev=43ddf52f95f4>; looks like they're pretty green.

Comment 9

[Geoff Brown [:gbrown]]

That's great -- let's enable them.

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 11

[Ed Morley [:emorley]]

Comment on attachment 833451 [details] [diff] [review]
Patch v1

lgtm :-)

(Btw a review request to a watched user sadly doesn't generate the correct bugmail, due to bug 179701 - I saw this because of the CC that it happens to perform at the same time)

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/4acc9992a56c

Comment 13

[Geoff Brown [:gbrown]]

Alas! Bug 663657 shows a big increase in mochitest-5 failures in dom/imptests beginning approx Nov 28. I think we need to disable these again.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Agreed. This is going to miss the uplift, so I'll land this on Aurora later as well.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f267d7c1148

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Not going to drive this.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Merge of backout. Let's leave this out for a few days and see if things improve or not. We can always re-land if they don't.
https://hg.mozilla.org/mozilla-central/rev/0f267d7c1148

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Geoff, should we re-land this? Your call :)

Comment 18

[Geoff Brown [:gbrown]]

Yes, let's re-land.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c4d75f2abee9

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c4d75f2abee9