The browser_geolocation-prompt-noperm.js webapprt-test-chrome test is failing because of a timeout: 0:05.39 TEST-START | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js 0:05.91 TEST-PASS | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Geolocation permission: unknown. 0:35.39 Failed to retrieve MOZ_UPLOAD_DIR env var 0:35.39 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Test timed out 0:35.39 TEST-INFO | MEMORY STAT vsize after test: 3223707648 0:35.39 TEST-INFO | MEMORY STAT residentFast after test: 173965312 0:35.39 TEST-INFO | MEMORY STAT heapAllocated after test: 37711032 0:35.39 INFO TEST-END | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | finished in 30007ms

On Windows, this wasn't failing a week ago (revision 73b3906ba285, Feb 15).

(I can't test with a recent revision because I'm having a build issue on Windows)

Looks like the problem is that the geolocation permission gets denied before we create the mutation observer. I'm now checking whether the permission was denied before creating the observer (it may be possible that we don't need the mutation observer anymore, but as we don't know what changeset triggered this test failure and it's just testing code, I think we can leave it as is).

What happens if we create the mutation observer on DOMContentLoaded instead of load? If the behavior is really indeterminate, then it makes sense to register the mutation observer while also checking to see if the content has already been mutated. But if the behavior has changed in a consistent way, then I'd rather we only test the expected behavior, so we know when something changes.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #4) > What happens if we create the mutation observer on DOMContentLoaded instead > of load? If the behavior is really indeterminate, then it makes sense to > register the mutation observer while also checking to see if the content has > already been mutated. But if the behavior has changed in a consistent way, > then I'd rather we only test the expected behavior, so we know when > something changes. Looks like the permission is being constistently denied before we get to define the mutation observer (no matter if we listen for "DOMContentLoaded" or "load"). I left the mutation observer because I can't be sure it's actually consistent (even if it's consistent on my machines). If we want to find what happened, I could find what caused the regression (at least we know it is something between Feb 15 and Feb 24). I have to say now I'm quite curious. (In these cases you really miss automated testing :D)

In the geolocation service initialization function, |sGeoInitPending| used to be |true| before bug 974100, now |sGeoInitPending| is |false| because we don't use the settings service anymore. When a location is requested, if |sGeoInitPending| is true we add the request to a queue (mPendingRequests), otherwise we create a runnable and dispatch it to the main thread. So I think the call to |nsIContentPermissionPrompt::Prompt| is still asynchronous, unless there's something else I didn't catch in my quick look at the geolocation code. Doug, is this analysis correct?

(In reply to Marco Castelluccio [:marco] from comment #5) > Looks like the permission is being constistently denied before we get to > define the mutation observer (no matter if we listen for "DOMContentLoaded" > or "load"). I left the mutation observer because I can't be sure it's > actually consistent (even if it's consistent on my machines). Could we make the app wait until "load" before trying to get the current position? Then, if the test script registers the mutation observer on DOMContentLoaded, we can be sure it will do so before the DOM node gets mutated.

Done!

Comment on attachment 8392553 [details] [diff] [review] Patch v2 Review of attachment 8392553 [details] [diff] [review]: ----------------------------------------------------------------- Looks and works great!