1581315 - Add MOZ_MUST_RETURN_FROM_CALLER to more ErrorResult methods

Status: RESOLVED FIXED

(Core :: DOM: Bindings (WebIDL), task, P1)

Description

[Boris Zbarsky [:bzbarsky]]

ThrowTypeError/ThrowRangeError should probably have the annotation too. Maybe others as well (NoteJSContextException, for example).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1581315 part 1. Adjust MOZ_MUST_RETURN_FROM_CALLER to only apply to method calls on arguments. r=nika

It doesn't really make sense to enforce this behavior for calls on stack
variables (which presumably we then want to do something with) and doesn't
necessarily make sense to enforce it for member variables either.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1581315 part 2. Fix incorrect behavior of PannerNode::SetRolloffFactor. r=karlt

Our implementation modified the state, in addition to throwing the exception.
And the tests not only didn't test for this, but wouldn't have reached that
code anyway, due to the harness giving up as soon as anything failed.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1581315 part 3. Add MOZ_MUST_RETURN_FROM_CALLER_IF_THIS_IS_ARG to more ErrorResult methods. r=smaug

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4968ed697e0a
part 1.  Adjust MOZ_MUST_RETURN_FROM_CALLER to only apply to method calls on arguments.  r=nika
https://hg.mozilla.org/integration/autoland/rev/be26864d5a00
part 2.  Fix incorrect behavior of PannerNode::SetRolloffFactor.  r=karlt
https://hg.mozilla.org/integration/autoland/rev/395ae88909c6
part 3.  Add MOZ_MUST_RETURN_FROM_CALLER_IF_THIS_IS_ARG to more ErrorResult methods.  r=smaug

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/19224 for changes under testing/web-platform/tests

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/19224
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/dNeJ9c6jRIypfSJ0x5Ag-A)

Comment 7

[Boris Zbarsky [:bzbarsky]]

The instability in Firefox causing the upstream test failure is a known issue with that test. See bug 1560454 (which is marked fixed, but was "fixed" by disabling the test)...

Comment 8

[James Graham [:jgraham]]

OK, I'll force merge this once it reaches central.

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4968ed697e0a
https://hg.mozilla.org/mozilla-central/rev/be26864d5a00
https://hg.mozilla.org/mozilla-central/rev/395ae88909c6

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham