Bug 51528

Summary: [v8] ScriptController::evaluate returns empty result for undefined. Can't distinguish errors from undefined results.
Product: WebKit Reporter: Tom R <tom.rathbone>
Component: WebCore JavaScriptAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: anthony.taranto, magreenblatt, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
fix, allow the "undefined" v8 object to be returned
eric: review-
Patch none

Description Tom R 2010-12-23 03:02:15 PST
ScriptController::evaluate contains:

    if (object.IsEmpty() || object->IsUndefined())
        return ScriptValue();

Should be:

    if (object.IsEmpty())
        return ScriptValue();

Returning an empty ScriptValue should indicate an error. As undefined is a valid return value for void calls returning an empty ScriptValue makes it impossible to distinguish undefined results from errors.

Related mailing list thread:

https://lists.webkit.org/pipermail/webkit-dev/2010-December/015417.html
Comment 1 Tom R 2010-12-23 06:39:17 PST
Created attachment 77331 [details]
fix, allow the "undefined" v8 object to be returned
Comment 2 Marshall Greenblatt 2011-01-07 10:29:08 PST
Hi Yury, please review this patch.
Comment 3 Eric Seidel (no email) 2011-01-11 03:08:49 PST
Comment on attachment 77331 [details]
fix, allow the "undefined" v8 object to be returned

This patch was not created correctly.  Please create from a WebKit checkout.  Also, all patches require a ChangeLog.

webkit-patch upload

will walk you through much of this.
Comment 4 Yury Semikhatsky 2011-01-11 03:30:55 PST
Hi Tom, please add proper ChangeLog entry to the patch as Eric suggested, otherwise the change looks good to me.

(In reply to comment #2)
> Hi Yury, please review this patch.
Comment 5 anthony taranto 2011-02-22 12:04:55 PST
Created attachment 83357 [details]
Patch
Comment 6 anthony taranto 2011-02-22 12:06:50 PST
Generated a patch with webkit-patch upload and included a ChangeLog. Let me know if anything else is needed.
Comment 7 Marshall Greenblatt 2011-02-25 12:16:56 PST
Can someone add this patch to the commit queue? Thanks.
Comment 8 Yury Semikhatsky 2011-03-01 01:00:11 PST
Comment on attachment 83357 [details]
Patch

Clearing flags on attachment: 83357

Committed r79981: <http://trac.webkit.org/changeset/79981>
Comment 9 Yury Semikhatsky 2011-03-01 01:00:18 PST
All reviewed patches have been landed.  Closing bug.