Handle return value of nsFilterInstance::PaintFilteredFrame

RESOLVED FIXED in Firefox 54

Status

()

Core
Layout: View Rendering
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: u459114, Assigned: u459114)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
We should either handle the return value of this function or change it to void if we don't want to.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8831240 - Flags: review?(mstange)
Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

https://reviewboard.mozilla.org/r/107804/#review109164

::: layout/svg/nsFilterInstance.cpp:520
(Diff revision 2)
> -  if (NS_FAILED(rv))
> -    return rv;
> +  if (NS_FAILED(rv)) {
> +    return NSResultToDrawResult(rv);
> +  }
>    rv = BuildSourcePaints(aDrawTarget);
> -  if (NS_FAILED(rv))
> -    return rv;
> +  if (NS_FAILED(rv)){
> +    return NSResultToDrawResult(rv);
> +  }

Can you make BuildSourceImage and BuildSourcePaints return a DrawResult instead?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

https://reviewboard.mozilla.org/r/107804/#review111284

Sorry for the delay; I missed the bugmail and mozreview doesn't email me when the review request is updated. Maybe I should have clicked r-.

::: layout/svg/nsFilterInstance.cpp:417
(Diff revision 4)
>  {
> -  nsresult rv = NS_OK;
> -
>    if (!mFillPaint.mNeededBounds.IsEmpty()) {
> -    rv = BuildSourcePaint(&mFillPaint, aTargetDT);
> -    NS_ENSURE_SUCCESS(rv, rv);
> +    DrawResult result = BuildSourcePaint(&mFillPaint, aTargetDT);
> +    if (result != DrawResult::SUCCESS){

There should be a space between ) and {. This happens in a bunch of places in this patch.

::: layout/svg/nsFilterInstance.cpp:515
(Diff revision 4)
>                   aDrawTarget->GetTransform();
>    aDrawTarget->SetTransform(newTM);
>  
>    ComputeNeededBoxes();
> +  DrawResult result = BuildSourceImage(aDrawTarget);
> +  if (DrawResult::SUCCESS != result){

Please make this "result != DrawResult::SUCCESS" (no Yoda conditions), here and below.
Attachment #8831240 - Flags: review?(mstange) → review+
Comment hidden (mozreview-request)

Comment 8

a year ago
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ddceecc1921e
Handle the return value of nsFilterInstance::PaintFilteredFrame r=mstange

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ddceecc1921e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.