Summary: | WebKit crashes when trying to send a msg via 'today's birthdays' dialogue box on Facebook | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | vomitols | ||||||||||||
Component: | JavaScriptCore | Assignee: | Chris Curtis <chris_curtis> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ap, chris_curtis, commit-queue, fpizlo, ggaren | ||||||||||||
Priority: | P1 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Mac (Intel) | ||||||||||||||
OS: | OS X 10.8 | ||||||||||||||
URL: | http://facebook.com | ||||||||||||||
Attachments: |
|
Description
vomitols
2013-09-02 16:38:55 PDT
Inside throwException, if appendSourceToMessage flag is set, the codeBlock is used to get the developers source code for the error message. In this case though, the codeBlock is never found. By checking to make sure that the codeBlock is valid we can fix the error, But because appendSourceToMessage was set there should have been a valid codeBlock. I am looking into why the codeBlock is not being found. JSValueToObject was almost certainly passed a "globalExec", which doesn't contain any CodeBlock. So, the bug here is the assumption that there will be a CodeBlock. Created attachment 210409 [details]
patch
Initial review, This patch still needs a regression test.
*** Bug 120825 has been marked as a duplicate of this bug. *** What is the status of this bug, did making a regression prove challenging? *** Bug 120509 has been marked as a duplicate of this bug. *** (In reply to comment #5) > What is the status of this bug, did making a regression prove challenging? A regression test can not be made to be run through DumpRenderTree, so I am making one in the testapi.c file. I was slightly distracted by a different patch, which is why it is not done. I will do it right now Created attachment 211261 [details]
patch with test
Created attachment 211262 [details]
removed extra whitespace line
Comment on attachment 211262 [details] removed extra whitespace line View in context: https://bugs.webkit.org/attachment.cgi?id=211262&action=review Looks good, but please fix these details. > Source/JavaScriptCore/API/tests/testapi.c:1064 > + JSClassRef globalObjectClass = JSClassCreate(&globalObjectClassDefinition); This needs a corresponding JSClassRelease. > Source/JavaScriptCore/API/tests/testapi.c:1065 > + context = JSGlobalContextCreateInGroup(NULL, globalObjectClass); This needs a corresponding JSContextRelease. Is it OK that we're overwriting the global here? Maybe this should be a local. > Source/JavaScriptCore/API/tests/testapi.c:1066 > + JSContextGetGlobalObject(context); You can remove this line. Created attachment 211268 [details]
patch 4
Created attachment 211269 [details]
name change.
Comment on attachment 211269 [details]
name change.
r=me
Comment on attachment 211269 [details] name change. Clearing flags on attachment: 211269 Committed r155495: <http://trac.webkit.org/changeset/155495> All reviewed patches have been landed. Closing bug. |