[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 #3 from Dmitry Lomov <dslomov at google.com>  2011-05-23 21:22:04 PST ---
(In reply to comment #2)
> (From update of attachment 94291 [details])
> 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.

Thanks for taking a look - fixed all of the above
> 
> > Source/WebKit/chromium/src/PlatformBridge.cpp:1048
> > +    return new WorkerMessagingProxy(worker);
> 
> Is there a different code path for shared workers?

Yes there is.

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