Closed
Bug 1253935
Opened 8 years ago
Closed 8 years ago
Frequent debug e10s browser_styleeditor_fetch-from-cache.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/styleeditor/test/head.js:65 - Error: cross-process JS call failed
Categories
(DevTools :: Style Editor, defect, P2)
Tracking
(e10s+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: RyanVM, Assigned: pbro)
References
(Blocks 1 open bug)
Details
(Keywords: intermittent-failure, Whiteboard: [btpp-fix-later])
Attachments
(1 file)
We ran into this failure a lot over the weekend, and as best we can tell, it's tied to where in the chunk the test is run. In other words, we would push a change to the devtools manifests and this test would suddenly go permafail, then another change would make it go green again. My best theory as to what might explain it is that while we use run-by-dir in these tests (launching a fresh Firefox instance between each directory), I *think* we recycle the profile between runs (because creating a new profile for each directory in run-by-dir mode caused a non-negligible runtime hit IIRC). So my suspicion is that this test somehow depends on state from other tests, which makes it susceptible to failures when those conditions aren't met due to chunking changes. At least, that's the most-plausible theory I've got :-) Anyway, I'm disabling the test on Windows debug e10s for now to avoid noise in the trees (and other patches being backed out for inadvertently pushing this into permafail again). https://treeherder.mozilla.org/logviewer.html#?job_id=23112616&repo=mozilla-inbound 20:38:04 INFO - 4 INFO TEST-START | devtools/client/styleeditor/test/browser_styleeditor_fetch-from-cache.js 20:38:04 INFO - ++DOCSHELL 0E56C400 == 3 [pid = 1428] [id = 3] 20:38:04 INFO - ++DOMWINDOW == 6 (0F5F5400) [pid = 1428] [serial = 6] [outer = 00000000] 20:38:04 INFO - ++DOMWINDOW == 7 (0F5F8C00) [pid = 1428] [serial = 7] [outer = 0F5F5400] 20:38:04 INFO - ++DOCSHELL 0E61A400 == 4 [pid = 1428] [id = 4] 20:38:04 INFO - ++DOMWINDOW == 8 (0F5F6800) [pid = 1428] [serial = 8] [outer = 00000000] 20:38:04 INFO - ++DOMWINDOW == 9 (0FA05800) [pid = 1428] [serial = 9] [outer = 0F5F6800] 20:38:04 INFO - ++DOMWINDOW == 10 (0FA09800) [pid = 1428] [serial = 10] [outer = 0F5F6800] 20:38:04 INFO - ++DOCSHELL 0F64B400 == 10 [pid = 2676] [id = 14] 20:38:04 INFO - ++DOMWINDOW == 26 (0F64BC00) [pid = 2676] [serial = 33] [outer = 00000000] 20:38:04 INFO - ++DOMWINDOW == 27 (0F652000) [pid = 2676] [serial = 34] [outer = 0F64BC00] 20:38:04 INFO - ++DOMWINDOW == 28 (0F66D000) [pid = 2676] [serial = 35] [outer = 0F64BC00] 20:38:05 INFO - ++DOCSHELL 0F864C00 == 11 [pid = 2676] [id = 15] 20:38:05 INFO - ++DOMWINDOW == 29 (0F8AA800) [pid = 2676] [serial = 36] [outer = 00000000] 20:38:05 INFO - ++DOMWINDOW == 30 (0F8B0400) [pid = 2676] [serial = 37] [outer = 0F8AA800] 20:38:05 INFO - ++DOCSHELL 18365800 == 12 [pid = 2676] [id = 16] 20:38:05 INFO - ++DOMWINDOW == 31 (1836CC00) [pid = 2676] [serial = 38] [outer = 00000000] 20:38:05 INFO - ++DOMWINDOW == 32 (18A6FC00) [pid = 2676] [serial = 39] [outer = 1836CC00] 20:38:06 INFO - TEST-INFO | started process screenshot 20:38:06 INFO - TEST-INFO | screenshot: exit 0 20:38:06 INFO - 5 INFO checking window state 20:38:06 INFO - 6 INFO Entering test bound 20:38:06 INFO - 7 INFO Opening netmonitor 20:38:06 INFO - 8 INFO Adding a new tab with URL: 'about:blank' 20:38:06 INFO - 9 INFO URL 'about:blank' loading complete 20:38:06 INFO - 10 INFO Navigating to test page 20:38:06 INFO - 11 INFO TEST-UNEXPECTED-FAIL | devtools/client/styleeditor/test/browser_styleeditor_fetch-from-cache.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/styleeditor/test/head.js:65 - Error: cross-process JS call failed 20:38:06 INFO - Stack trace: 20:38:06 INFO - navigateTo@chrome://mochitests/content/browser/devtools/client/styleeditor/test/head.js:65:3 20:38:06 INFO - @chrome://mochitests/content/browser/devtools/client/styleeditor/test/browser_styleeditor_fetch-from-cache.js:20:9 20:38:06 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:790:9 20:38:06 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:710:7 20:38:06 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59 20:38:06 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:790:9 20:38:06 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:710:7 20:38:06 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59 20:38:06 INFO - 12 INFO Leaving test bound 20:38:06 INFO - [Child 1428] WARNING: site security information will not be persisted: file c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/security/manager/ssl/nsSiteSecurityService.cpp, line 260 20:38:06 INFO - ++DOMWINDOW == 11 (0EF7EC00) [pid = 1428] [serial = 11] [outer = 0F5F6800] 20:38:06 INFO - --DOCSHELL 10A87C00 == 11 [pid = 2676] [id = 11] 20:38:07 INFO - --DOMWINDOW == 31 (10AD6000) [pid = 2676] [serial = 27] [outer = 00000000] [url = about:blank] 20:38:07 INFO - --DOCSHELL 18365800 == 10 [pid = 2676] [id = 16] 20:38:07 INFO - --DOMWINDOW == 30 (10A88800) [pid = 2676] [serial = 26] [outer = 00000000] [url = chrome://devtools/content/framework/toolbox.xul] 20:38:07 INFO - --DOMWINDOW == 29 (1836CC00) [pid = 2676] [serial = 38] [outer = 00000000] [url = about:blank] 20:38:07 INFO - --DOMWINDOW == 28 (18A70400) [pid = 2676] [serial = 29] [outer = 00000000] [url = chrome://devtools/content/styleeditor/styleeditor.xul] 20:38:07 INFO - --DOMWINDOW == 27 (1C61D800) [pid = 2676] [serial = 31] [outer = 00000000] [url = data:text/html;charset=utf8,<!DOCTYPE%20html><html%20dir='ltr'>%20%20<head>%20%20%20%20<style>%20%20%20%20%20%20html,%20body%20{%20height:%20100%;%20}%20%20%20%20%20%20body%20{%20margin:%200;%20overflow:%20hidden;%20}%20%20%20%20%20%20.CodeMirror%20{%20width:%20100%;%20height:%20100%%20!important;%20line-height:%201.25%20!important;}%20%20%20%20</style><link%20rel='stylesheet'%20href='chrome://devtools/content/sourceeditor/codemirror/lib/codemirror.css'><link%20rel='stylesheet'%20href='chrome://devtools/content/sourceeditor/codemirror/addon/dialog/dialog.css'><link%20rel='stylesheet'%20href='chrome://devtools/content/sourceeditor/codemirror/mozilla.css'>%20%20</head>%20%20<body%20class='theme-body%20devtools-monospace'></body></html>] 20:38:07 INFO - MEMORY STAT | vsize 632MB | vsizeMaxContiguous 541MB | residentFast 272MB | heapAllocated 71MB 20:38:07 INFO - 13 INFO TEST-OK | devtools/client/styleeditor/test/browser_styleeditor_fetch-from-cache.js | took 3012ms
Reporter | ||
Comment 2•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8f2cc9188850
Assignee | ||
Comment 3•8 years ago
|
||
I don't know why changing the way this test runs in chunks would affect it, but the error message is pretty telling:
> devtools/client/styleeditor/test/head.js:65 - Error: cross-process JS call failed
This line is `content.location = url` in this head.js helper:
function navigateTo(url) {
let navigating = promise.defer();
gBrowser.selectedBrowser.addEventListener("load", function onload() {
gBrowser.selectedBrowser.removeEventListener("load", onload, true);
navigating.resolve();
}, true);
content.location = url;
return navigating.promise;
}
This is obviously a CPOW usage that should be cleaned up, and an easy fix too.
ContentTask.spawn can be used to navigate in the content process instead.
I suggest taking this opportunity import shared-head.js in this head.js to get rid of some of the code, and getting rid of all CPOW usages in these tests.
Priority: -- → P2
Whiteboard: [btpp-fix-later]
Updated•8 years ago
|
tracking-e10s:
--- → +
Reporter | ||
Comment 4•8 years ago
|
||
Happens on OSX too.
OS: Windows → All
Summary: Frequent Windows debug e10s browser_styleeditor_fetch-from-cache.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/styleeditor/test/head.js:65 - Error: cross-process JS call failed → Frequent debug e10s browser_styleeditor_fetch-from-cache.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/styleeditor/test/head.js:65 - Error: cross-process JS call failed
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0220f77f4cd4
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 8•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42979/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42979/
Attachment #8735852 -
Flags: review?(poirot.alex)
Comment 9•8 years ago
|
||
Comment on attachment 8735852 [details] MozReview Request: Bug 1253935 - Remove all CPOW usages in styleeditor tests and use ContentTask instead of custom frame-script; r=ochameau https://reviewboard.mozilla.org/r/42979/#review39483 Thanks for this cleanup. I'm happy to see another framescript being removed. Before reenabling tests, have you run many try to verify it is no longer intermittent? ::: devtools/client/styleeditor/test/browser_styleeditor_transition_rule.js:27 (Diff revision 1) > editor.sourceEditor.setText(NEW_RULE); > editor.sourceEditor.setText(NEW_RULE + " "); > > yield styleChanges; > > - let rules = yield executeInContent("Test:cssRules", { > + let rules = yield ContentTask.spawn(tab.linkedBrowser, 0, function*(index) { In browser_styleeditor_sync.js you are using gBrowser.selectedBrowser, here you are using tab.linkedBrowser. It would be great to be consistent. ::: devtools/client/styleeditor/test/head.js:62 (Diff revision 1) > }, true); > - content.location = url; > - return navigating.promise; > -} > > -function* cleanup() { > + yield ContentTask.spawn(browser, url, "u => content.location.href = u"); I would have done this instead of doing something in child: browser.loadURI(url); (xul:browser code is then in charge of doing whatever is necessary to load this url) Also don't we already have such helper elsewhere? ::: devtools/client/styleeditor/test/head.js:134 (Diff revision 1) > - return yield executeInContent("Test:GetComputedStylePropertyValue", > - {selector, > - pseudo, > - name: propName}); > + return yield ContentTask.spawn(gBrowser.selectedBrowser, args, > + `function*({selector, pseudo, name}) { > + let element = content.document.querySelector(selector); > + let style = content.getComputedStyle(element, pseudo); > + return style.getPropertyValue(name); > + }` It would be easier to read without using a string here. Is there any reason to not use a regular function?
Attachment #8735852 -
Flags: review?(poirot.alex) → review+
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #9) > Thanks for this cleanup. I'm happy to see another framescript being removed. > Before reenabling tests, have you run many try to verify it is no longer > intermittent? Yeah, this is still pending. Here's the try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=546d7e3942f2&group_state=expanded > In browser_styleeditor_sync.js you are using gBrowser.selectedBrowser, here > you are using tab.linkedBrowser. It would be great to be consistent. Ok, will do. > ::: devtools/client/styleeditor/test/head.js:62 > > -function* cleanup() { > > + yield ContentTask.spawn(browser, url, "u => content.location.href = u"); > > I would have done this instead of doing something in child: > browser.loadURI(url); > > (xul:browser code is then in charge of doing whatever is necessary to load > this url) Righ, looks better. > Also don't we already have such helper elsewhere? I'll look, but I'm not sure. We have bug 1181856 for using shared-head.js in styleeditor tests, but I'd like to avoid doing this big change in this patch. > ::: devtools/client/styleeditor/test/head.js:134 > (Diff revision 1) > > - return yield executeInContent("Test:GetComputedStylePropertyValue", > > - {selector, > > - pseudo, > > - name: propName}); > > + return yield ContentTask.spawn(gBrowser.selectedBrowser, args, > > + `function*({selector, pseudo, name}) { > > + let element = content.document.querySelector(selector); > > + let style = content.getComputedStyle(element, pseudo); > > + return style.getPropertyValue(name); > > + }` > > It would be easier to read without using a string here. Is there any reason > to not use a regular function? Yes, there was a reason when I did it, but I can't remember for sure. I'll try again.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8735852 [details] MozReview Request: Bug 1253935 - Remove all CPOW usages in styleeditor tests and use ContentTask instead of custom frame-script; r=ochameau Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42979/diff/1-2/
Attachment #8735852 -
Attachment description: MozReview Request: Bug 1253935 - Remove all CPOW usages in styleeditor tests and use ContentTask instead of custom frame-script → MozReview Request: Bug 1253935 - Remove all CPOW usages in styleeditor tests and use ContentTask instead of custom frame-script; r=ochameau
Reporter | ||
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b994a14879e1
Assignee | ||
Comment 14•8 years ago
|
||
Not sure why this is leave-open. Something landed on m-c in March and there has been no reports of test failures since. So I'm going to close it.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•