Closed Bug 1271029 Opened 4 years ago Closed 4 years ago

Fix and re-enable test_bug465448.xul on e10s

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
e10s + ---
firefox47 --- wontfix
firefox48 --- fixed
firefox49 --- fixed

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
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)
(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
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.
Thanks! That's good news.
Assignee: nobody → dholbert
Priority: -- → P1
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)
(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
(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
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
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
https://hg.mozilla.org/mozilla-central/rev/69977c5df1c6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.