Closed Bug 995697 Opened 9 years ago Closed 8 years ago

Intermittent browser_geolocation_privatebrowsing_perwindowpb.js | Test timed out | Found a browser window after previous test timed out

Categories

(Core :: DOM: Geolocation, defect)

Product:
Core
Component:
DOM: Geolocation
Platform:
x86
Windows 8
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla41
Tracking Flags:
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox-esr31 --- wontfix
firefox-esr38 --- fixed

People

(Reporter: RyanVM, Assigned: Gijs)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

MozReview Request: Bug 995697 - remove obsolete geolocation private browsing test, r?jdm
39 bytes, text/x-review-board-request
jdm
: review+
Details 
https://tbpl.mozilla.org/php/getParsedLog.php?id=37699511&tree=Mozilla-Inbound

WINNT 6.2 mozilla-inbound pgo test mochitest-browser-chrome-2 on 2014-04-12 14:51:23 PDT for push ba77af4dbd10
slave: t-w864-ix-077

15:00:46     INFO -  TEST-START | chrome://mochitests/content/browser/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js
15:00:46     INFO -  TEST-INFO | chrome://mochitests/content/browser/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js | Got load
15:01:31     INFO -  TEST-INFO | screenshot: exit status 0
15:01:31  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js | Test timed out
15:01:31     INFO -  TEST-INFO | MEMORY STAT vsize after test: 738983936
15:01:31     INFO -  TEST-INFO | MEMORY STAT vsizeMaxContiguous after test: 1893662720
15:01:31     INFO -  TEST-INFO | MEMORY STAT residentFast after test: 154562560
15:01:31     INFO -  TEST-INFO | MEMORY STAT heapAllocated after test: 54244974
15:01:31     INFO -  INFO TEST-END | chrome://mochitests/content/browser/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js | finished in 45021ms
15:01:31     INFO -  TEST-INFO | checking window state
15:01:31  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js | Found a browser window after previous test timed out
After a month without any timeouts, this test is now timing out about once per day.
David: this intermittent geolocation failure started up (again) after your try push in comment 25 for bug 1042889. The subsequent intermittent reports are all on fx-team. Have any of your changes for bug 1042889 landed in fx-team? Could they have caused network timeouts when connecting to Google's Location Service?
Flags: needinfo?(dkeeler)
Looking at the test, I see this is probably unrelated to bug 1042889. This test just talks to a local network_geolocation.sjs page, not Google's Location Service.

Curiously, all these test timeouts happen on Windows.
Flags: needinfo?(dkeeler)
Garvan, I know this is an old bug, but do you think the recent changes to use the Windows 8 Location API (bug 512407) could break the geolocation tests that assume they can override GLS by setting the "geo.wifi.uri" pref?
Flags: needinfo?(gkeeley)
All the geo mochitests are tests of the network geolocation provider, with a fake web service (as you noted, the geo.wifi.url is overridden to point to this fake local service).  
There are no geo mochitests that are affected by other providers (such as the CoreLocation or Windows provider).
Flags: needinfo?(gkeeley)
BTW, this test is super-annoying in that it fails one a day, mostly on Windows. The quick-fix (which seems to have worked with other geo tests that were timing out) was to increase wait times for responses. IIRC, this test looked correctly event-driven to me, and I didn't see any quick fixes, and didn't spend further time on it.
I should look at it again more closely when I have some time.
Chris one more comment on your question(In reply to Chris Peterson [:cpeterson] from comment #115)
> Garvan, I know this is an old bug, but do you think the recent changes to
> use the Windows 8 Location API (bug 512407) could break the geolocation
> tests that assume they can override GLS by setting the "geo.wifi.uri" pref?

Wouldn't this become non-intermittent if this was the case?

In m-c's firefox.js, the "geo.provider.ms-windows-location" pref is not in that file.
In the code, we have it defaulting to false:

https://dxr.mozilla.org/mozilla-central/source/dom/geolocation/nsGeolocation.cpp#817

#ifdef XP_WIN
  if (Preferences::GetBool("geo.provider.ms-windows-location", false)) {
    mProvider = new WindowsLocationProvider();
  }
#endif
This is failing more frequently. 
I just ran
./mach mochitest-browser --repeat 99 dom/tests/browser
on Windows 8.1 and I am getting intermittent failures.
Problem is, I get intermittent failures in other tests also, such as browser_test_new_window_from_content.js.
Going to try this on my mac now and see if I get similar intermittent failures in other tests. Point being, this may be a larger systematic problem, and not specific to this test.
And yet more notices of failure. I suspect this was failing in early 2013, because this mysterious commit happened:

http://hg.mozilla.org/mozilla-central/diff/d42a2a82f3d2/dom/tests/browser/browser_geolocation_privatebrowsing_perwindowpb.js

Which has changes that are unclear to me as to their purpose. The code prior to that commit looks more understandable to me. 
An example is the change that adds toggling the "geo.wifi.scan" pref, which is not done in any of the many geo mochitests. Indeed NetworkGeolocationProvider will try to do a scan with the default setting, but this has never had an affect on the tests that I have seen, as is largely a no-op on the test machines.

We could try to back the code prior to that commit. Not that this will necessarily fix this case which appears to be resource starvation on the test machine. But would make for a simpler-looking test case :).
Ehsan, you reviewed the commit I mentioned in comment 132. It was a while ago, but any chance you recall something about that change?
Flags: needinfo?(ehsan)
(In reply to Garvan Keeley [:garvank] from comment #264)
> Ehsan, you reviewed the commit I mentioned in comment 132. It was a while
> ago, but any chance you recall something about that change?

Olli wrote that code.
Flags: needinfo?(ehsan) → needinfo?(bugs)
That change was about, IIRC, geolocation stuff crashing locally on linux.

This bug started to happen way later SnowWhite patch landed.
Flags: needinfo?(bugs)