Closed Bug 636243 Opened 9 years ago Closed 9 years ago

Use JSOPTION_NO_SCRIPT_RVAL in nsDOMWorkerScriptLoader.cpp

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mwu, Assigned: mwu)

Details

Attachments

(2 files, 1 obsolete file)

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.
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.
(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
Attachment #514625 - Flags: review?(bent.mozilla)
Attachment #514585 - Flags: review?(bent.mozilla) → review?(jorendorff)
Attachment #514625 - Flags: review?(bent.mozilla) → review?(jorendorff)
Attachment #514585 - Flags: review?(jorendorff) → review+
Attachment #514625 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/mozilla-central/rev/dfcd2378bd75
http://hg.mozilla.org/mozilla-central/rev/e9b3541f85d1
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.