785060 - Check for JS_NewUCStringCopyZ failure

Status: RESOLVED FIXED

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

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Of the places we call JS_NewUCStringCopyZ outside js/, only one out of four deals with the exception correctly.

Comment 1

[Nagarjuna Varma [:Infinity]]

Attachment: Patchv1

Does this look fine?

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 682703 [details] [diff] [review]
Patchv1

Review of attachment 682703 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

::: content/html/content/src/nsHTMLMediaElement.cpp
@@ +1468,5 @@
> +    NS_WARNING("Failed to perform string copy");
> +    args->error = true;
> +    return PL_DHASH_STOP;
> +  }
> +  

Just remove this trailing whitespace.

Comment 3

[Nagarjuna Varma [:Infinity]]

Ok...done! Do you want me to run any specific tests before I submit the patch for review?

Comment 4

[Nagarjuna Varma [:Infinity]]

Attachment: Patchv2

It compiled faster than expected...wow!

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 682727 [details] [diff] [review]
Patchv2

Review of attachment 682727 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 6

[Josh Matthews [:jdm]]

Could you submit a patch with a proper author and commit message? That would make it easier to commit it for you. See https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F for more details.

Comment 7

[Nagarjuna Varma [:Infinity]]

Attachment: Patchv3

I had it configured exactly like that. Check it out!
http://pastebin.mozilla.org/1949277
So I shouldn't be doing hg diff -p -U 8 I guess.....anyways edited patch here :)

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Could you also make sure there is a commit message in the patch? Maybe something like

Bug 785060 - Check for allocation failure in JS_NewUCStringCopyZ calls; r=Ms2ger

Comment 9

[Nagarjuna Varma [:Infinity]]

Attachment: patchv4

Hope its ok now. I got MQ fixed just now. So any extra metadata you'd like, lemme know.

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 685258 [details] [diff] [review]
patchv4

Thanks!

Comment 11

[Mark Capella [:capella]]

push to try first ...
https://tbpl.mozilla.org/?tree=Try&rev=5a14d27d6d64

Comment 12

[Mark Capella [:capella]]

on to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bab7ae06e579

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bab7ae06e579