mozBackgroundRequest ignore errors only in the main thread

RESOLVED FIXED

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Because it is annotated as [SetterThrows=Workers] while it can actually fail even in the main thread.

Is this intentional? If so, why is it different between the main thread and workers?
This totally looks like a bug.  Should just be SetterThrows.  Want to fix?
Flags: needinfo?(VYV03354)
(Assignee)

Comment 2

4 years ago
Will do.
Assignee: nobody → VYV03354
Flags: needinfo?(VYV03354)
(Assignee)

Comment 3

4 years ago
Created attachment 8564126 [details] [diff] [review]
patch

|void SetMozBackgroundRequest(bool, nsresult&)| was not even called, it was not even updated when ErrorResult was introduced. The compiler didn't catch the error because the signature of the XPCOM version (that is, |NS_IMETHODIMP nsXMLHttpRequest::SetMozBackgroundRequest(bool)|) happened to match the signature of the infallible WebIDL setter function (the difference of the return type did not matter).

Is it possible to prevent similar mistakes in the future?
Attachment #8564126 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

4 years ago
> |void SetMozBackgroundRequest(bool, nsresult&)| was not even called
...from the generated code. It is called from the XPCOM version, of course.
Comment on attachment 8564126 [details] [diff] [review]
patch

r=me

As far as preventing this in the future, I filed bug 1132934.
Attachment #8564126 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 7

4 years ago
Hm, some tests set mozBackgroundRequest after open() that should have no effect...
/dom/system/NetworkGeolocationProvider.js
/toolkit/components/search/SearchSuggestionController.jsm
/toolkit/identity/Identity.jsm
(Assignee)

Updated

4 years ago
(Assignee)

Comment 8

2 years ago
Fixed by bug 1269162.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.