1240437 - PushLayer and PopLayer APIs are not implemented for DrawTargetRecording

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bob Owen (:bobowen) PTO until 8th Oct]

Nightly crashes when I try to print the Nightly Start page to XPS via the parent process, using paper size Japanese Postcard.

This is because PushLayer and PopLayer are not implemented for DrawTargetRecording, so printing via parent will fail for any print that attempts to use them.

Comment 1

[Bob Owen (:bobowen) PTO until 8th Oct]

It looks like we just need to add two new RecordedDrawingEvent classes for PushLayer and PopLayer.

I'm guessing that IsCurrentGroupOpaque would just need to call the wrapped DrawTarget and we wouldn't need to record this.
Is that correct?

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to Bob Owen (:bobowen) from comment #1)
> It looks like we just need to add two new RecordedDrawingEvent classes for
> PushLayer and PopLayer.
> 
> I'm guessing that IsCurrentGroupOpaque would just need to call the wrapped
> DrawTarget and we wouldn't need to record this.
> Is that correct?

Absolutely right. Sorry I forgot this. Want me to write this patch?

Comment 3

[Bob Owen (:bobowen) PTO until 8th Oct]

(In reply to Bas Schouten (:bas.schouten) from comment #2)
> (In reply to Bob Owen (:bobowen) from comment #1)
> > It looks like we just need to add two new RecordedDrawingEvent classes for
> > PushLayer and PopLayer.
> > 
> > I'm guessing that IsCurrentGroupOpaque would just need to call the wrapped
> > DrawTarget and we wouldn't need to record this.
> > Is that correct?
> 
> Absolutely right. Sorry I forgot this. Want me to write this patch?

If you have time soon, that would be great, thanks.

Comment 4

[Ryo Kato [:r_kato]]

I have trouble in printing a document with XPS. Thanks for handling this issue!
Bug 1240370

Comment 5

[Bob Owen (:bobowen) PTO until 8th Oct]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db610de55515

Comment 6

[Bob Owen (:bobowen) PTO until 8th Oct]

Attachment: Implement PushLayer and PopLayer for DrawTargetRecording

One thing I'm not sure of here is RecordedPushLayer::OutputSimpleEventInfo ... writing out the details of the matrix and bounds seems pretty laborious and other events seem to skip some things, I can add them if needed.

Comment 7

[Bas Schouten (:bas.schouten)]

(In reply to Bob Owen (:bobowen) from comment #6)
> Created attachment 8711680 [details] [diff] [review]
> Implement PushLayer and PopLayer for DrawTargetRecording
> 
> One thing I'm not sure of here is RecordedPushLayer::OutputSimpleEventInfo
> ... writing out the details of the matrix and bounds seems pretty laborious
> and other events seem to skip some things, I can add them if needed.

It seems reasonable this way. Sorry for not having done this yet. Good work.

Comment 8

[Bob Owen (:bobowen) PTO until 8th Oct]

(In reply to Bas Schouten (:bas.schouten) from comment #7)
> (In reply to Bob Owen (:bobowen) from comment #6)
> > Created attachment 8711680 [details] [diff] [review]
> > Implement PushLayer and PopLayer for DrawTargetRecording
> > 
> > One thing I'm not sure of here is RecordedPushLayer::OutputSimpleEventInfo
> > ... writing out the details of the matrix and bounds seems pretty laborious
> > and other events seem to skip some things, I can add them if needed.
> 
> It seems reasonable this way. Sorry for not having done this yet. Good work.

Thanks Bas and no problem.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2575b0980d5

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a3431cbd4bb

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e2575b0980d5
https://hg.mozilla.org/mozilla-central/rev/9a3431cbd4bb

Comment 12

[Bob Owen (:bobowen) PTO until 8th Oct]

Comment on attachment 8711680 [details] [diff] [review]
Implement PushLayer and PopLayer for DrawTargetRecording

Approval Request Comment
[Feature/regressing bug #]:
Bug 1220629 essentially regressed this, even though printing via the parent wasn't turned on by default at the time.

[User impact if declined]:
We won't be able to get a low integrity content sandbox turned on for Windows on Fx46.

[Describe test coverage new/current, TreeHerder]:
This has been manually tested with prints that failed due to the regression.
There is an historical lack of automated printing tests in this area.

[Risks and why]:
Low - on it's own this code won't be used until we turn on printing via the parent. I'll make the risks for that clear in the uplift request for that.

[String/UUID change made/needed]:
None

Comment 13

[Bob Owen (:bobowen) PTO until 8th Oct]

Just noticed I somehow set this by accident a while ago.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8711680 [details] [diff] [review]
Implement PushLayer and PopLayer for DrawTargetRecording

Underlying work for fix for a regression in 46. 
Will help with sandboxing for printing in Windows.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking since this is a regression (and crash)

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ff09e358297b
https://hg.mozilla.org/releases/mozilla-aurora/rev/414cb97f6807