1334554 - Handle return value of nsFilterInstance::PaintFilteredFrame

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[u459114]

We should either handle the return value of this function or change it to void if we don't want to.

Comment 1

[u459114]

Attachment: Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

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

Comment 2

[u459114]

Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

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

Comment 3

[Markus Stange [: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 4

[u459114]

Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

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

Comment 5

[u459114]

Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107804/diff/3-4/

Comment 6

[Markus Stange [:mstange]]

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.

Comment 7

[u459114]

Comment on attachment 8831240 [details]
Bug 1334554 - Handle the return value of nsFilterInstance::PaintFilteredFrame

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107804/diff/4-5/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddceecc1921e
Handle the return value of nsFilterInstance::PaintFilteredFrame r=mstange

Comment 9

[Carsten Book [:Tomcat]]

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