[webkit-reviews] review granted: [Bug 89088] Web Inspector: [WebGL] Simple implementation of the InjectedWebGLScriptSource to support capturing WebGL calls for a frame : [Attachment 147577] Patch

[bugzilla-daemon at webkit.org]

Vsevolod Vlasov <vsevik at chromium.org> has granted Andrey Adaikin
<aandrey at chromium.org>'s request for review:
Bug 89088: Web Inspector: [WebGL] Simple implementation of the
InjectedWebGLScriptSource to support capturing WebGL calls for a frame
https://bugs.webkit.org/show_bug.cgi?id=89088

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

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147577&action=review


>>>> Source/WebCore/inspector/InjectedWebGLScriptSource.js:57
>>>> +	      nameProcessed.__proto__ = null;
>>> 
>>> Does this have any effect?
>> 
>> Ah, I see the intent. I think it would be better to use
Object.hasOwnProperty when checking for property to be in nameProcessed
> 
> We will execute nameProcessed["hasOwnProperty"]=true at some point :)
> If we get rid of nameProcessed and use proxy.hasOwnProperty() instead, the
same problem will arize after executing proxy.hasOwnProperty =
_wrappedFunction(...)

Consider using prefix for storing in nameProcessed or
Object.getOwnPropertyDescriptor(object, propertyName)

> Source/WebCore/inspector/InjectedWebGLScriptSource.js:102
> +    {

Shouldn't we check that we are not capturing already here?