Closed
Bug 1271029
Opened 8 years ago
Closed 8 years ago
Fix and re-enable test_bug465448.xul on e10s
Categories
(Core :: Layout, defect, P1)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: RyanVM, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
https://treeherder.mozilla.org/logviewer.html#?job_id=20501520&repo=try#L6585 20:07:33 INFO - 1914 INFO TEST-START | layout/base/tests/test_bug465448.xul 20:07:34 INFO - JavaScript error: http://mochi.test:8888/tests/layout/base/tests/test_bug465448.xul, line 23: NS_ERROR_NOT_IMPLEMENTED: 20:12:44 INFO - TEST-INFO | started process screentopng 20:12:45 INFO - TEST-INFO | screentopng: exit 0 20:12:45 INFO - 1915 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug465448.xul | Test timed out. 20:12:45 INFO - reportError@SimpleTest/TestRunner.js:114:7 20:12:45 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:135:7 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5 20:12:45 INFO - TestRunner.runTests@SimpleTest/TestRunner.js:367:5 20:12:45 INFO - RunSet.runtests@SimpleTest/setup.js:186:3 20:12:45 INFO - RunSet.runall@SimpleTest/setup.js:165:5 20:12:45 INFO - hookupTests@SimpleTest/setup.js:258:5 20:12:45 INFO - parseTestManifest@http://mochi.test:8888/manifestLibrary.js:36:5 20:12:45 INFO - getTestManifest/req.onload@http://mochi.test:8888/manifestLibrary.js:49:11 20:12:45 INFO - EventHandlerNonNull*getTestManifest@http://mochi.test:8888/manifestLibrary.js:45:3 20:12:45 INFO - hookup@SimpleTest/setup.js:238:5 20:12:45 INFO - EventHandlerNonNull*@http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Ftmp:11:1 20:12:45 INFO - MEMORY STAT | vsize 393MB | residentFast 81MB | heapAllocated 16MB 20:12:45 INFO - 1916 INFO TEST-OK | layout/base/tests/test_bug465448.xul | took 312329ms
Updated•8 years ago
|
tracking-e10s:
--- → +
Comment 1•8 years ago
|
||
Daniel, layout/base/tests/test_bug465448.xul is disabled for e10s because it times out on all platforms when e10s is enabled. How important is this test? Can you please flag it to be triaged by the Layout team?
Flags: needinfo?(dholbert)
Assignee | ||
Comment 2•8 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #1) > How important is this test? I don't know, offhand. From looking at the bug that added it, it seems to be a regression test for a bug in the sizeToContent API; I'm not familiar with that API. > Can you please flag it to be triaged by the Layout team? Sure! I'll take a closer look early next week. Leaving needinfo open.
Depends on: 465448
Assignee | ||
Comment 3•8 years ago
|
||
I think this might be WORSKFORME, actually (and want a re-enabling). At least, this passes locally: ./mach mochitest layout/base/tests/test_bug465448.xul (And e10s is enabled for mochitests by default now, so IIUC it's running in e10s mode.) So, maybe we've recently implemented something that gets us past the NS_ERROR_NOT_IMPLEMENTED failure in comment 0! I'm running a test-reenable patch through Try to check if this is all fine now.
Assignee | ||
Comment 5•8 years ago
|
||
Yup, looks like it's expected that this would have been fixed by bug 1255138. Quoting bug 1255138 comment 0: > Doing things such as setting window.innerWidth, window.innerHeight, or > calling window.sizeToContent() doesn't work in e10s mode, since > TabChild::SizeBrowserTo() is unimplemented. This very function (SizeBrowserTo) is what was responsible for the NS_ERROR_NOT_IMPLEMENTED error message here.) So, it definitely makes sense that this test is passing now, and we can safely re-enable it.
Depends on: 1255138
Flags: needinfo?(dholbert)
Assignee | ||
Comment 6•8 years ago
|
||
(Adding dependency on bug 982828, which is where this test was originally disabled for e10s. At that point it was already skipped for b2g, presumably due to this same issue -- so I'm going to remove the "skip" line entirely.) Try run with this test re-enabled ("skip" line removed): https://treeherder.mozilla.org/#/jobs?repo=try&revision=f16135b39b92 This test is part of Linux opt Mochitest-4 there, which is green and definitely includes this test in its log output: > 12:29:45 INFO - 1922 INFO TEST-START | layout/base/tests/test_bug465448.xul > 12:29:46 INFO - MEMORY STAT | vsize 400MB | residentFast 107MB | heapAllocated 30MB > 12:29:46 INFO - 1923 INFO TEST-OK | layout/base/tests/test_bug465448.xul | took 1331ms http://archive.mozilla.org/pub/firefox/try-builds/dholbert@mozilla.com-f16135b39b92d69132460eda35f1fe829a514454/try-linux/try_ubuntu32_vm_test-mochitest-e10s-4-bm03-tests1-linux32-build47.txt.gz
Depends on: 982828
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #6) > (Adding dependency on bug 982828, which is where this test was originally > disabled for e10s. (Sorry, I'll revert this actually -- on closer inspection, it looks like no other individual "re-enable" bugs are marked as blocking bug 982828 -- we track them via the e10s-tests metabug, which this is already blocking.)
No longer depends on: 982828
Assignee | ||
Comment 8•8 years ago
|
||
Historical note -- for b2g, this test was originally marked for skipping (twice actually) here, for bug 815416: > + "layout/base/tests/test_bug465448.xul" : "uncaught expception, window.onerror", > + "layout/base/tests/test_bug465448.xul": "times out", https://hg.mozilla.org/try/rev/7c4c1bbf4d88#l1.18
Assignee | ||
Comment 10•8 years ago
|
||
The bug that fixed this (bug 1255138) was backported to Aurora48, so we should be able to reenable the test there, as well. Aurora try run to sanity-check that, before I land there: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1940e984c84
Assignee | ||
Comment 11•8 years ago
|
||
Aurora try run looks good. Landed: https://hg.mozilla.org/releases/mozilla-aurora/rev/f8e493cd1e87
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/69977c5df1c6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•