[Webkit-unassigned] [Bug 61016] [WebWorkers][Chromium] Use v8 Isolates for in-process implementation of WebWorkers

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from David Levin <levin at chromium.org>  2011-05-23 11:05:45 PST ---
(From update of attachment 94291)
View in context: https://bugs.webkit.org/attachment.cgi?id=94291&action=review

Loks good overall.
Mostly minor style nits - a few more substantive comments.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2165
> +    if (!data->rawTemplateMap().contains(&info)) {

Use "find" instead of contains to only do the lookup once. (Right now it does contains and get).

Sketch of code:

result = data->rawTemplateMap().find(&info);
if (result != end)
    return result...;

v8::Persistent<v8::FunctionTemplate> rawTemplate = createRawTemplate();
data->rawTemplateMap().add(&info, rawTemplate);
return rawTemplate;

> Source/WebCore/bindings/v8/V8Binding.cpp:51
> +V8BindingPerIsolateData::V8BindingPerIsolateData(v8::Isolate* isolate) {

{ on next line (in many places).

> Source/WebCore/bindings/v8/V8Binding.cpp:57
> +// static

Chromium does these "static" comments but WebKit doesn't.

> Source/WebCore/bindings/v8/V8Binding.cpp:60
> +    if (embedderData != 0) {

Do use {} for single line clauses and avoid comparisons to 0.

> Source/WebCore/bindings/v8/V8Binding.cpp:71
> +    if (data != 0) {

Ditto.

> Source/WebCore/bindings/v8/V8Binding.h:59
> +        static void dispose(v8::Isolate* isolate);

isolate -- param name adds no information so don't include it.

> Source/WebCore/bindings/v8/V8Binding.h:67
> +        V8BindingPerIsolateData(v8::Isolate* isolate);

Ditto.

> Source/WebKit/chromium/src/PlatformBridge.cpp:1048
> +    return new WorkerMessagingProxy(worker);

Is there a different code path for shared workers?

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