Closed Bug 1274966 Opened 6 years ago Closed 6 years ago
Broken browser e10s user agent tests because of Content
Parent/Content Child and Build User Agent timing
https://treeherder.mozilla.org/#/jobs?repo=pine&filter-tier=1&filter-tier=2&filter-tier=3&fromchange=9785ffc4b1ad&selectedJob=28236 > 812 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_explicit_user_agent.html | The resulting user-agent is the navigator's UA - got "Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0", expected "Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 /" > 814 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_explicit_user_agent.html | The user-agent is the navigator's UA - got "Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0", expected "Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 /"
printf() in attachment 8755421 [details] shows that we have the child calling BuildUserAgent() BEFORE ContentChild::RecvAppInfo(). Hence, the proper AppInfo sent by the parent are not yet in the child. That is not the case on attachment 8755420 [details].
This is reproduced on pine since at least march 3rd, as one can check with: https://tools.taskcluster.net/task-inspector/#Q8_j9FnmRTq0NHitS-iI3A/
After investigation, I have been able to see that the offending call was related to nsPermissionManager::FetchPermissions(). I have not yet any proper explanation of why, so I am sharing this for now.
Looks like the race condition was caused by this cset, which uses the permission manager _very_ early in the initialization of the content process: http://hg.mozilla.org/projects/pine/rev/d5da8b62d465#l10.1 That code ends up calling FetchPermissions which does a synchronous SendReadPermission call to the parent process, and messes with the timing of the child seeing RecvAppInfo before nsHttpHandler::BuildUserAgent gets called. My first impression is that we're abusing the permission manager for enabling formatToParts. I'll keep looking for a way to fix this. Attachment 8755590 [details] [diff] will probably regress bug 1129315 (do we care about that?).
(In reply to Reuben Morais [:reuben] from comment #7) > Looks like the race condition was caused by this cset Poor wording on my part here. That cset transformed the race condition into this bug, it didn't create the race condition.
Fabrice, as far as I can tell from the current Gecko on Pine + your Gaia pine branch on GitHub, the "previously-certified-app" permission test is essentially a test for the system app. Is that supposed to be extended in the future to other apps, or can we substitute the permission check for something like a hardcoded test on the principal?
Thanks reuben! But as you can see in comment 6, just removing FetchPermissions() call does break a lot of stuff. Reverting the offending patch fixes the problem and is much more green, according to https://treeherder.mozilla.org/#/jobs?repo=try&revision=5364afea8669&filter-tier=1&filter-tier=2&filter-tier=3 but it breaks some features in Gaia (like, lockscreen clock).
I have a new tentative patch that just allows experimental formatsToPart() to anything with a system principal: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3dabfeefdc19&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8755590 - Attachment is obsolete: true
Comment on attachment 8755786 [details] [diff] [review] Allow experimental ECMA formatToParts in chrome r=... This seems good enough for getting mulet to work AND keeping browser test green. I have checked but the URI of the principal is either empty or "about:blank", so I don't have a better suggestion for now :/.
Attachment #8755786 - Flags: review?(fabrice)
Attachment #8755786 - Flags: review?(fabrice) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.