Closed Bug 1390440 Opened 7 years ago Closed 7 years ago

Support plugin in layers-free mode

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

Attachments

(1 file)

Currently we do the plugin painting in layer building state. So we need to do some changes for layers-free mode.
Component: Audio/Video: Recording → Graphics: WebRender
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 on attachment 8897753 [details]
Bug 1390440 - Support nsDisplayPlugin for webrender layers-free mode.

https://reviewboard.mozilla.org/r/169058/#review180184
Attachment #8897753 - Flags: review?(matt.woodrow) → review+
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d9f65581eed4
Support nsDisplayPlugin for webrender layers-free mode. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/d9f65581eed4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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?
Flags: needinfo?(ethlin)
We already have automated tests but the preference is off by default. I think it will be covered after bug 1389000 is landed.
Flags: needinfo?(ethlin)
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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Should CreateWebRenderCommands be covered now? It looks like it still isn't.
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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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).
(In reply to Marco Castelluccio [:marco] from comment #13)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Blocks: 1624533
You need to log in before you can comment on or make changes to this bug.