[webkit-reviews] review granted: [Bug 22721] Implement WorkerUtils.importScripts() : [Attachment 28425] Implement importScripts, matching spec behaviour (which appears to be the consensus)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 10 00:42:21 PDT 2009

Alexey Proskuryakov <ap at webkit.org> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 22721: Implement WorkerUtils.importScripts()

Attachment 28425: Implement importScripts, matching spec behaviour (which
appears to be the consensus)

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
You can test cross-origin loading better by loading one of the scripts from
http://localhost:8000 (as http tests are loaded from

Please add WorkerImportScriptsClient to non-Mac projects.

Still no JSLock in evaluate()? Per IRC discussion, it's just an unsaved file.

	 virtual void resourceRetrievedByXMLHttpRequest(unsigned long
identifier, const ScriptString& sourceString) = 0;
+	 virtual void scriptImported(unsigned long, const String&) {

Why is scriptImported() not a pure virtual function, like the above

+	 virtual void didFinishLoading(unsigned long /* identifier */);

No need to comment out the argument name in declaration.

> In response to comments from dave_levin, i have removed the m_response field
> from WorkerImportScriptsClient as it was unnecessary

A good point, it's unnecessary - but I still see it in the patch.

r=me, please consider the comments, especially fixing non-Mac builds.

More information about the webkit-reviews mailing list