[Webkit-unassigned] [Bug 79851] Return value from executed script in Chromium.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79851


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #133928|review?                     |review-
               Flag|                            |




--- Comment #24 from Adam Barth <abarth at webkit.org>  2012-04-06 16:38:14 PST ---
(From update of attachment 133928)
View in context: https://bugs.webkit.org/attachment.cgi?id=133928&action=review

Sorry that it's taken me so long to review this patch.  This looks fine.  A couple of small suggestions below.  I think the main thing is that we can avoid converting back and forth between ScriptValues.  If WebCore wants to use this function at some point, we might need to use ScriptValue, but it doesn't seem to be needed right now.

> Source/WebCore/ChangeLog:8
> +        Providing a varaiant of evaluateScriptInIsolatedWorld that

typo: varaiant

> Source/WebKit/chromium/public/WebFrame.h:279
> +    virtual void executeScriptInIsolatedWorld(
> +        int worldID, const WebScriptSource* sourcesIn, unsigned numSources,
> +        int extensionGroup, WebVector<v8::Handle<v8::Value> >* results) = 0;

Should we be more specific that these are local handles?

> Source/WebKit/chromium/src/WebFrameImpl.cpp:906
> +    for (unsigned i = 0; i < numSources; ++i) {
> +        TextPosition position(OrdinalNumber::fromOneBasedInt(sourcesIn[i].startLine), OrdinalNumber::first());
> +        sources.append(ScriptSourceCode(sourcesIn[i].code, sourcesIn[i].url, position));
> +    }

Is this code copy/pasted from somewhere?  Should we share code instead?

> Source/WebKit/chromium/src/WebFrameImpl.cpp:909
> +        Vector<ScriptValue> scriptResults;

Do we really need to round-trip through ScriptValue?  Both the caller and the callee of this function always use V8, so it seems like we could just pass the vector directly.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.