Open Bug 1573811 Opened 4 months ago Updated 14 days ago

layout/style/test/test_visited_reftests.html doesn't seem to test WebRender

Categories

(Core :: Graphics: WebRender, defect, P3)

defect

Tracking

()

People

(Reporter: jrmuizel, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

It looks like we're missing some test coverage that we should have. We should investigate.

Blocks: wr-69
Priority: -- → P3
Assignee: nobody → ktaeleman

I looked at the raw logs from the 'Windows 10 x64 QuantumRender Shippable opt' build/task and the test seems to be included in the output.
Is there anything that points to it not being tested with webrender?

Windows 10 x64 QuantumRender Shippable opt, Job: (sig): windows 10 x64 quantumrender shippable opt mochitests test-windows10-64-shippable-qr/opt-mochitest-e10s-5 m(5)

{"thread": "None", "pid": null, "source": "mochitest", "test": "layout/style/test/test_visited_reftests.html", "js_source": "TestRunner.js", "time": 1566252837578, "action": "test_start"}
{"thread": "ProcessReader", "process": "GECKO(6292)", "pid": 1780, "source": "mochitest", "time": 1566252840155, "action": "process_output", "data": "MEMORY STAT | vsize 2104167MB | vsizeMaxContiguous 69072341MB | residentFast 359MB | heapAllocated 144MB"}
{"status": "OK", "pid": null, "test": "layout/style/test/test_visited_reftests.html", "js_source": "TestRunner.js", "message": "Finished in 2648ms", "stack": "{u'runtime': 2648}", "thread": "None", "source": "mochitest", "time": 1566252840226, "action": "test_end"}

If you revert https://hg.mozilla.org/mozilla-central/rev/10439459ebd6, or really just change this line to say *aStyle->StyleBorder() rather than visitedBorder.refOr(..), you should see a few of the subtests (https://searchfox.org/mozilla-central/source/layout/reftests/css-visited/border-1.html for example) fail.

When I was writing that patch I ran that test, and I could see the output was different, but the comparison that that test does using snapshotWindow doesn't fail.

So I suspect snapshotWindow is returning a window as painted as non-webrender, even if running with webrender enabled.

Thanks Emilio!
Adding a bit more context to this (based on my limited knowledge).

These tests have their own reftest code as links are being marked as visited asynchronously. The History needs to perform a lookup on the main thread, which can cause I/O. There are some delays in place to try to ensure that the visited links are resolved before snapshotWindow() is being executed.

To be able to move these tests into the reftest harness, we could add an event to notify that links are resolved. Some extra test code could then only remove the reftest-wait when it has received this event or we could add it as an extra condition before MozReftestInvalidate gets fired.

I'm looking into the feasibility of this by digging through the Link code.

  • Implement visited link callbacks on the content thread so SpecialPowers can listen for them
  • Implement callback for non-visited link resolution to ensure we wait long enough before we run the test
  • Move all css-visited tests and related svg tests into css-visited reftest folder
  • Remove css-visited mochitest wrapper
  • Enable history during reftests only for css-visited
  • Enable SpecialPowers API for reftest harness

Todo:

  • Listen for MozReftestInvalidate before removing reftest-wait.

I'm curious why converting these tests to not use snapshotWindow is the right approach here -- there are plenty of other tests that also use snapshotWindow. Could snapshotWindow be made to use the webrender codepath?

No longer blocks: wr-69

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 from comment #5)

I'm curious why converting these tests to not use snapshotWindow is the right approach here -- there are plenty of other tests that also use snapshotWindow. Could snapshotWindow be made to use the webrender codepath?

Yes, I'm going to look into this as a solution, but haven't had the chance yet. I'll probably look at this next week.

@Sotaro: Do you have more context about this code. I tried enabling DRAWWINDOW_USE_WIDGET_LAYERS for the test, but this code seems to indicate that it is not supported as the window we're trying to call it on is an IFrame.

See:
https://searchfox.org/mozilla-central/source/layout/base/PresShell.cpp#4492-4505

https://searchfox.org/mozilla-central/source/layout/style/test/test_visited_reftests.html#165

Flags: needinfo?(sotaro.ikeda.g)

We can get the layers contents from a non-toplevel window. Can you open a new window without chrome, add an iframe in that window and run the test in the iframe, and then take the snapshots from the toplevel window you opened?

(In reply to Kris Taeleman (:kris/kris) from comment #7)

@Sotaro: Do you have more context about this code. I tried enabling DRAWWINDOW_USE_WIDGET_LAYERS for the test, but this code seems to indicate that it is not supported as the window we're trying to call it on is an IFrame.

I just modified PresShell::RenderDocument() as to handle WebRenderLayerManager case correctly as before. My understanding of it is that WebRender snapshot happens only when the following conditions are met. It is same behavior as before adding WebRenderLayerManager.

  • RenderDocumentFlags::UseWidgetLayers is set
  • Root frame has widget
  • The call is in parent process
  • Widget uses WebRender as compositor
Flags: needinfo?(sotaro.ikeda.g)
Assignee: ktaeleman → nobody
You need to log in before you can comment on or make changes to this bug.