[webkit-reviews] review denied: [Bug 25385] Upstream changes to V8 bindings for supporting nested workers. : [Attachment 29777] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 5 15:56:02 PDT 2009


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Jian Li
<jianli at chromium.org>'s request for review:
Bug 25385: Upstream changes to V8 bindings for supporting nested workers.
https://bugs.webkit.org/show_bug.cgi?id=25385

Attachment 29777: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=29777&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Style looks good! :)

> +	   if (result.IsEmpty()) {
> +	       v8::Local<v8::Object> object = toV8(type, type, impl);
> +	       if (!object.IsEmpty())
> +		   static_cast<Worker*>(impl)->ref();
> +	       result = v8::Persistent<v8::Object>::New(object);
> +	       V8Proxy::SetJSWrapperForDOMObject(impl, result);
> +	   }
> +	   return result;

Would it be a bit more clear to inverse this:

if (!result.IsEmpty())
   return result;
...

> +    return PassRefPtr<EventListener>();

Can just return 0;


More information about the webkit-reviews mailing list