1390440 - Support plugin in layers-free mode

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Ethan Lin[:ethlin]]

Currently we do the plugin painting in layer building state. So we need to do some changes for layers-free mode.

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Bug 1390440 - Support nsDisplayPlugin for webrender layers-free mode.

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

Comment 2

[Ethan Lin[:ethlin]]

Currently I only handle nsDisplayPlugin. I'm not sure which test will use nsDisplayPluginReadback. With the patch, 'layout/reftests/bugs/632423-1.html' could pass in layers-free mode.

Comment 3

[Ethan Lin[:ethlin]]

Comment on attachment 8897753 [details]
Bug 1390440 - Support nsDisplayPlugin for webrender layers-free mode.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169058/diff/1-2/

Comment 4

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8897753 [details]
Bug 1390440 - Support nsDisplayPlugin for webrender layers-free mode.

https://reviewboard.mozilla.org/r/169058/#review180184

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9f65581eed4
Support nsDisplayPlugin for webrender layers-free mode. r=mattwoodrow

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d9f65581eed4

Comment 7

[Marco Castelluccio [:marco]]

According to the coverage report, the code you added is not covered by automated tests: https://codecov.io/gh/marco-c/gecko-dev/src/master/layout/generic/nsPluginFrame.cpp#L1411.

Is it expected? Would it be possible to write tests covering the code?

Comment 8

[Ethan Lin[:ethlin]]

We already have automated tests but the preference is off by default. I think it will be covered after bug 1389000 is landed.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

We can turn on some tests that cover this code via the pref(gfx.webrender.layers-free,true) skip-if(!webrender) annotation (e.g. in bug 1392523) as a stopgap until bug 1389000 lands.

Comment 10

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #9)
> We can turn on some tests that cover this code via the
> pref(gfx.webrender.layers-free,true) skip-if(!webrender) annotation (e.g. in
> bug 1392523) as a stopgap until bug 1389000 lands.

Okay, I'll add 'layout/reftests/bugs/632423-1.html' (plugin test) in bug 1392523 as well.

Comment 11

[Marco Castelluccio [:marco]]

Should CreateWebRenderCommands be covered now? It looks like it still isn't.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Yeah it should be covered by the test at http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/layout/reftests/bugs/reftest.list#1654

Note that the test only runs on the linux64-qr platform ("Linux x64 QuantumRender" on TreeHerder), which might not be included in the code coverage analysis. That might explain why it's not showing up as covered.

Comment 13

[Marco Castelluccio [:marco]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> Yeah it should be covered by the test at
> http://searchfox.org/mozilla-central/rev/
> 6326724982c66aaeaf70bb7c7ee170f7a38ca226/layout/reftests/bugs/reftest.
> list#1654
> 
> Note that the test only runs on the linux64-qr platform ("Linux x64
> QuantumRender" on TreeHerder), which might not be included in the code
> coverage analysis. That might explain why it's not showing up as covered.

Yes, that's why. I thought we were running those tests on the normal linux64 platform (and thus linux64-ccov).

Comment 14

[Ethan Lin[:ethlin]]

(In reply to Marco Castelluccio [:marco] from comment #13)
> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> > Yeah it should be covered by the test at
> > http://searchfox.org/mozilla-central/rev/
> > 6326724982c66aaeaf70bb7c7ee170f7a38ca226/layout/reftests/bugs/reftest.
> > list#1654
> > 
> > Note that the test only runs on the linux64-qr platform ("Linux x64
> > QuantumRender" on TreeHerder), which might not be included in the code
> > coverage analysis. That might explain why it's not showing up as covered.
> 
> Yes, that's why. I thought we were running those tests on the normal linux64
> platform (and thus linux64-ccov).

Right, should be the platform problem. I just checked on my local device and I think it's covered now.