Closed Bug 1025994 Opened 10 years ago Closed 10 years ago

Intermittent test_webapps-manage.html | Test timed out

Categories

(Core :: Permission Manager, defect)

Product:
Core
Component:
Permission Manager
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
firefox31 --- unaffected
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: emorley, Assigned: dchanm+bugzilla)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

bug-1025994-fix.patch
2.18 KB, patch
baku
: review+
RyanVM
: feedback+
Details | Diff | Splinter Review 
b2g_emulator_vm mozilla-inbound debug test mochitest-debug-10 on 2014-06-16 03:28:56 PDT for push 409f474cc986

slave: tst-linux64-spot-701

https://tbpl.mozilla.org/php/getParsedLog.php?id=41783451&tree=Mozilla-Inbound

{
04:01:25     INFO -  253 INFO TEST-START | /tests/dom/permission/tests/test_webapps-manage.html
04:01:25     INFO -  [Child 728] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1383
04:01:26     INFO -  [Child 728] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1383
04:01:27     INFO -  [Child 728] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1383
04:01:28     INFO -  [Child 728] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1383
04:01:28     INFO -  [Child 728] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1383
04:01:29     INFO -  [Child 728] WARNING: Subdocument container has no frame: file ../../../gecko/layout/base/nsDocumentViewer.cpp, line 2509
04:01:30     INFO -  [Child 728] WARNING: NS_ENSURE_TRUE(txToRemove) failed: file ../../../../gecko/docshell/shistory/src/nsSHistory.cpp, line 1338
04:01:31     INFO -  [Child 728] WARNING: Subdocument container has no frame: file ../../../gecko/layout/base/nsDocumentViewer.cpp, line 2509
04:01:32     INFO -  -*- Webapps.jsm : getAll
04:01:42     INFO -  254 INFO TEST-INFO | dumping last 3 message(s)
04:01:42     INFO -  255 INFO TEST-INFO | if you need more context, please use SimpleTest.requestCompleteLog() in your test
04:01:42     INFO -  256 INFO TEST-PASS | /tests/dom/permission/tests/test_webapps-manage.html | Doesn't have webapps-manage
04:01:43     INFO -  257 INFO TEST-PASS | /tests/dom/permission/tests/test_webapps-manage.html | Got exception TypeError: window.navigator.mozApps.mgmt is null
04:01:43     INFO -  258 INFO TEST-PASS | /tests/dom/permission/tests/test_webapps-manage.html | Has webapps-manage
04:01:43     INFO -  259 INFO TEST-UNEXPECTED-FAIL | /tests/dom/permission/tests/test_webapps-manage.html | Test timed out
04:01:43     INFO -  [Child 728] WARNING: NS_ENSURE_TRUE(txToRemove) failed: file ../../../../gecko/docshell/shistory/src/nsSHistory.cpp, line 1338
04:01:43     INFO -  260 INFO TEST-INFO | MEMORY STAT vsize after test: 120307712
04:01:44     INFO -  261 INFO TEST-INFO | MEMORY STAT residentFast after test: 69263360
04:01:44     INFO -  262 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 21486824
04:01:44     INFO -  263 INFO TEST-END | /tests/dom/permission/tests/test_webapps-manage.html | finished in 19239ms
}
Hmm, the current testing code creates a frame with the webapps-manage permission then attempts to do a mgmt.getAll() . I wonder if the list of apps on the emulator has grown to a point that it takes too long to complete the call before the timeout is triggered.

It might be possible to rework the test to check for the existence of the mgmt object instead of calling a function of it. iirc, the original b2g design didn't allow a simple object check due to what was being returned.
This has gone near perma-fail in the last day or so. Can you please take a fresh look? Otherwise, we're going to have to consider disabling the test.
Flags: needinfo?(dchan)
Attached patch bug-1025994-fix.patch — — Splinter Review 
This reworks the permissions test for webapps-manage to test for navigator.mozApps.mgmt instead of checking to see if getAll() succeeds. The test path should be faster now without a DOMRequest + metadata cloning.

I tested that this passes locally on a Linux build, but will need someone to help me push to try.
Flags: needinfo?(dchan)
We'll want to retrigger the mochitest run lots of times to be sure it's not still failing intermittently. I'll keep an eye on it for the next few hours.
https://tbpl.mozilla.org/?tree=Try&rev=e8cff299e776
Comment on attachment 8446807 [details] [diff] [review]
bug-1025994-fix.patch

30 runs without a failure on Try sure looks promising! :)
Attachment #8446807 - Flags: feedback+
Blocks: 951976
:RyanVM

Given the green try runs, are we okay to land this on inbound / central?
Flags: needinfo?(ryanvm)
Needs review first, but yes, it would be great if we could expedite that and get it landed ASAP! :)
Assignee: nobody → dchan
Flags: needinfo?(ryanvm)
Comment on attachment 8446807 [details] [diff] [review]
bug-1025994-fix.patch

Adding :baku who did the original review for bug 815105

The main change is to the webapps-manage test to check based on IDL vs whether the API is accessible. This removes the DOMRequest which I believe was timing out the test.

A minor change was made to the test framework to allow accessing any object under window.navigator instead of just window.navigator.foo

Try run
https://tbpl.mozilla.org/?tree=Try&rev=e8cff299e776
Attachment #8446807 - Flags: review?(amarchesini)
Comment on attachment 8446807 [details] [diff] [review]
bug-1025994-fix.patch

Review of attachment 8446807 [details] [diff] [review]:
-----------------------------------------------------------------

So you don't want to check the value, but just if the property is correctly set in the object. Good for me.
Attachment #8446807 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/49e7fc49dd4e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
The timeouts should stop completely with the landing of bug 1027127 which removes the timeout functionality. Any future timeouts will be a result of the mochitest timeout triggering.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: