387478 - NS_ScriptErrorReporter() doesn't need to null-check the report.

Status: RESOLVED FIXED

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

Description

[Johnny Stenback (:jst)]

Brendan says we don't need to null-check the JSErrorReport in NS_ScriptErrorReporter() any more, we're guaranteed to get a non-null report.

Comment 1

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1.0

Patch v1.0

Like so?

Comment 2

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1 (-w)

Patch v1 (-w)

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 271642 [details] [diff] [review]
Patch v1.0

Yes, only this happens to be against a tree w/o my crash fix that was needed after I landed my latest change to NS_ScriptErrorReporter(). You'll need to find that change (i.e. sameOrigin = report->filename == nsnull) and stick that in here and this'll look great! Update that and I'll r+sr. Thanks for the patch!

Comment 4

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1.0

Patch v1.1

Updated to include the latest tree changes (my build was a few days out-of-date due to internet issues on this end).

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 271693 [details] [diff] [review]
Patch v1.0

-          if (report) {
+          if (message) {
+            rv = errorObject->Init(msg.get(), nsnull, nsnull, 0, 0, 0,
+                                   category);
+          }
+          else {
             PRUint32 column = report->uctokenptr - report->uclinebuf;
 
             rv = errorObject->Init(msg.get(), fileName.get(),
                                    reinterpret_cast<const PRUnichar*>
                                                    (report->uclinebuf),
                                    report->lineno, column, report->flags,
                                    category);

This isn't correct, we'll often (always?) get a non-null message as well as a report, so I think you simply want to use the report here, and never create an error object based on the message alone.

r+sr=jst with that.

Comment 6

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1.2 (for checkin)

Patch v1.2

Patch updated and ready for checkin.

Comment 7

[Johnny Stenback (:jst)]

Fix landed on the trunk. Thanks for the patch!