1261507 - Convert mochitest-chrome test_hover.html to be runnable with e10s

Status: NEW

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Quoting tn in bug 1254806 comment 24:
"Hover needs to be test cross-process too. I moved this to chrome to avoid a specific problem. Likely there are better ways to avoid it."

Comment 1

[Leo Schwarz]

I would like to work on this bug.

Ryan VanderMeulen already outlined to me on IRC what this would likely involve, so I guess I should be able to do this even though I am still not very familiar with the code base.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Setting you as the assignee then :). Don't hesitate to ask here or on IRC if you get stuck!

Comment 3

[Leo Schwarz]

Attachment: Bug 1261507 - Convert mochitest-chrome test_hover.html to mochitest-plain so it's runnable with e10s.

Review commit: https://reviewboard.mozilla.org/r/88418/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88418/

Comment 4

[Ryan VanderMeulen [:RyanVM]]

I'm not a layout peer, but I'll at least run this through Try to make sure it passes across all platforms. Thanks for working on this!

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8804456 [details]
Bug 1261507 - Convert mochitest-chrome test_hover.html to mochitest-plain so it's runnable with e10s.

https://reviewboard.mozilla.org/r/88418/#review87476

::: layout/style/test/mochitest.ini:208
(Diff revision 1)
>  [test_grid_item_shorthands.html]
>  [test_grid_shorthand_serialization.html]
>  [test_grid_computed_values.html]
>  [test_group_insertRule.html]
> +[test_hover.html]
> +skip-if = buildapp == 'mulet'

You can drop this skip-if line while you're here. Mulet is no longer supported.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8804456 [details]
Bug 1261507 - Convert mochitest-chrome test_hover.html to mochitest-plain so it's runnable with e10s.

Overall, this looks good on Try. I think you've successfully converted this test over to mochitest-plain! There's one problem, however. On OSX, the test appears to fail intermittently at a pretty frequent rate:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2ca491ca20f&noautoclassify&group_state=expanded

https://treeherder.mozilla.org/logviewer.html#?job_id=29889068&repo=try

15:50:34     INFO -  636 INFO TEST-START | layout/style/test/test_hover.html
15:50:34     INFO -  TEST-INFO | started process screencapture
15:50:34     INFO -  TEST-INFO | screencapture: exit 0
15:50:34     INFO -  637 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies
15:50:34     INFO -  638 INFO TEST-PASS | layout/style/test/test_hover.html | :hover does not apply
15:50:34     INFO -  639 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies hierarchically
15:50:34     INFO -  640 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies
15:50:34     INFO -  641 INFO TEST-PASS | layout/style/test/test_hover.html | :hover does not apply
15:50:34     INFO -  642 INFO TEST-PASS | layout/style/test/test_hover.html | :hover does not apply
15:50:34     INFO -  643 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies hierarchically
15:50:34     INFO -  644 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies
15:50:34     INFO -  645 INFO TEST-PASS | layout/style/test/test_hover.html | :hover does not apply (iframe body width)
15:50:34     INFO -  646 INFO TEST-PASS | layout/style/test/test_hover.html | :hover applies (iframe body width)
15:50:34     INFO -  647 INFO TEST-PASS | layout/style/test/test_hover.html | step2 called only once
15:50:34     INFO -  648 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_hover.html | :hover applies - got "transparent", expected "rgb(0, 255, 0)"
15:50:34     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:270:5
15:50:34     INFO -      step2@layout/style/test/hover_helper.html:99:5
15:50:34     INFO -      @layout/style/test/hover_helper.html:57:1
15:50:34     INFO -  Not taking screenshot here: see the one that was previously logged
15:50:34     INFO -  649 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_hover.html | :hover applies (iframe body width) - got 50, expected 100
15:50:34     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:270:5
15:50:34     INFO -      step2@layout/style/test/hover_helper.html:101:5
15:50:34     INFO -      @layout/style/test/hover_helper.html:57:1
15:50:34     INFO -  Not taking screenshot here: see the one that was previously logged
15:50:34     INFO -  650 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_hover.html | :hover does not apply (iframe body width) - got 100, expected 50
15:50:34     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:270:5
15:50:34     INFO -      step2@layout/style/test/hover_helper.html:105:5
15:50:34     INFO -      @layout/style/test/hover_helper.html:57:1
15:50:34     INFO -  651 INFO TEST-PASS | layout/style/test/test_hover.html | step3 called only once
15:50:34     INFO -  652 INFO TEST-PASS | layout/style/test/test_hover.html | :hover does not apply
15:56:03     INFO -  Not taking screenshot here: see the one that was previously logged
15:56:03     INFO -  653 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_hover.html | Test timed out.
15:56:03     INFO -      reportError@SimpleTest/TestRunner.js:114:7
15:56:03     INFO -      TestRunner._checkForHangs@SimpleTest/TestRunner.js:135:7
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:156:5
15:56:03     INFO -      TestRunner.runTests@SimpleTest/TestRunner.js:367:5
15:56:03     INFO -      RunSet.runtests@SimpleTest/setup.js:186:3
15:56:03     INFO -      RunSet.runall@SimpleTest/setup.js:165:5
15:56:03     INFO -      hookupTests@SimpleTest/setup.js:258:5
15:56:03     INFO -  parseTestManifest@http://mochi.test:8888/manifestLibrary.js:36:5
15:56:03     INFO -  getTestManifest/req.onload@http://mochi.test:8888/manifestLibrary.js:49:11
15:56:03     INFO -  EventHandlerNonNull*getTestManifest@http://mochi.test:8888/manifestLibrary.js:45:3
15:56:03     INFO -      hookup@SimpleTest/setup.js:238:5
15:56:03     INFO -  EventHandlerNonNull*@http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Fvar%2Ffolders%2Fzw%2Fk_htx6850ms3t4396777xgwh00000w%2FT:11:1
15:56:04     INFO -  MEMORY STAT | vsize 3262MB | residentFast 321MB | heapAllocated 21MB
15:56:04     INFO -  654 INFO TEST-OK | layout/style/test/test_hover.html | took 330655ms

Screenshot:
http://mozilla-releng-blobs.s3.amazonaws.com/blobs/try/sha512/c1278eba2eafbb5096e8691b0da004e6374e42c4e15c3763ef4e5599098d0501fc69f716f6bc6c8cb327da84ecc4ec82a7cc4a2227b83833d0e6e4e265f92488

Maybe dholbert has an idea of what might be going wrong?

Comment 7

[Daniel Holbert [:dholbert]]

In every single one of the failures, the screenshot shows two Mac OS notifications in the upper-right corner ("What's new in OS X Yosemite" followed by "Upgrade to macOS Sierra")

My first guess would be: when this fails, it's because one or both of those popups have *just* appeared, while this test is running, and they've (briefly) stolen focus from our "helper" popup window, and so the hover states are no longer applied. Or something like that.

Presumably this wasn't happening when this was a Mochitest-Chrome test, because the test comes at a completely different time (with respect to machine startup) in that testsiute.  And the popups weren't causing problems in pochitest-plain before this because the tests that happened to be running when the popup appeared must not care about focus. (And IIRC we regain focus at the beginning of every mochitest, or something like that.)

SO: We probably need to get RelEng to fix whatever OS X 10.10 test-machines/images we're using, so that they don't spawn these system popups.  If my theory is correct, that'll fix this.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Those popups are basically in every OSX mochitest failure screenshot you'll ever see. I wouldn't necessarily get too hung up on them.

Comment 9

[Daniel Holbert [:dholbert]]

Hmm, OK. I filed bug 1312923 on them, in any case...

So, setting that aside -- it seems like the hover styles are applying in mochitest-chrome, but not reliably doing so in mochitest-plain, specifically on this particular Mac OS platform, for some reason.

I don't have any other ideas/explanation here, offhand.  I'm just returning from vacation and am pretty backlogged, plus I've got zero history/experience with this test, so I'm not the most helpful person here at the moment.  Perhaps tn has some thoughts on possible-explanations, as it looks like he's got some experience with this test (per comment 0)?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This times out on Android as well.
https://treeherder.mozilla.org/logviewer.html#?job_id=29897672&repo=try

Probably not the end of the world to add a |skip-if = os == 'android'| if necessary since the mochitest-chrome version of this was already not running on Android anyway (skipped at globally at the top of the manifest).

Comment 11

[Timothy Nikkel (:tnikkel)]

Not sure what might be going wrong. It could be that we are seeing the same issue that caused me to convert this test to a mochitest-chrome in the first place. Which was that the reflow caused by the stop button turning into the reload button was messing up the test, which was expecting reflows to only happen based on its own generated events. So you could try running the test in hover_helper.html from setTimeout(...,1000) instead of setTimeout(...,0) and see if the problems go away.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.