If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 514585 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp

The return value is never used so we can use JSOPTION_NO_SCRIPT_RVAL.
Attachment #514585 - Flags: review?(bent.mozilla)
Comment on attachment 514585 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp

Hm... I think you need to update nsDOMWorkerTimeout::ExpressionCallback::Run() as well.
(Assignee)

Comment 2

7 years ago
Created attachment 514606 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerTimeout.cpp

Good catch. nsDOMWorkerTimeout.cpp compiles and executes the code in one step so it's easier here. These patches remain independent of each other, but we might as well fix all the dom worker stuff in one bug.
Attachment #514606 - Flags: review?(bent.mozilla)
Comment on attachment 514606 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerTimeout.cpp

Looks like this is the wrong patch.
Attachment #514606 - Flags: review?(bent.mozilla)
Comment on attachment 514585 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp

After talking with one of the JS devs I think we should add JSOPTION_NO_SCRIPT_RVAL around the call to JS_Execute here too.
(Assignee)

Comment 5

7 years ago
Created attachment 514625 [details] [diff] [review]
Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerTimeout.cpp (right patch)

Opps.
Attachment #514606 - Attachment is obsolete: true
(Assignee)

Comment 6

7 years ago
(In reply to comment #4)
> Comment on attachment 514585 [details] [diff] [review]
> Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp
> 
> After talking with one of the JS devs I think we should add
> JSOPTION_NO_SCRIPT_RVAL around the call to JS_Execute here too.

Counter example:
http://hg.mozilla.org/mozilla-central/file/8f8f80356db4/js/src/shell/js.cpp#l448
(Assignee)

Updated

7 years ago
Attachment #514625 - Flags: review?(bent.mozilla)
(Assignee)

Updated

7 years ago
Attachment #514585 - Flags: review?(bent.mozilla) → review?(jorendorff)
(Assignee)

Updated

7 years ago
Attachment #514625 - Flags: review?(bent.mozilla) → review?(jorendorff)
Attachment #514585 - Flags: review?(jorendorff) → review+
Attachment #514625 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 7

7 years ago
http://hg.mozilla.org/mozilla-central/rev/dfcd2378bd75
http://hg.mozilla.org/mozilla-central/rev/e9b3541f85d1
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.