Closed Bug 1240437 Opened 4 years ago Closed 4 years ago

PushLayer and PopLayer APIs are not implemented for DrawTargetRecording

Categories

(Core :: Graphics, defect)

46 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- unaffected
firefox46 + fixed
firefox47 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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?
Flags: needinfo?(bas)
(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?
Flags: needinfo?(bas)
(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.
Assignee: nobody → bobowen.code
I have trouble in printing a document with XPS. Thanks for handling this issue!
Bug 1240370
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.
Attachment #8711680 - Flags: review?(bas)
Attachment #8711680 - Flags: review?(bas) → review+
(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.
(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.
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e2575b0980d5
https://hg.mozilla.org/mozilla-central/rev/9a3431cbd4bb
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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
Attachment #8711680 - Flags: approval-mozilla-aurora?
Just noticed I somehow set this by accident a while ago.
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.
Attachment #8711680 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Tracking since this is a regression (and crash)
You need to log in before you can comment on or make changes to this bug.