Bug 87960 – Web Inspector: [WebGL] Add WebGL instrumentation support on the backend

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Adaikin]

Adding WebGL instrumentation support on the backend side.

[Andrey Adaikin]

Created attachment 145064 [details]
Patch

[Build Bot]

Comment on attachment 145064 [details]
Patch

Attachment 145064 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12859347

[Tor Arne Vestbø]

Comment on attachment 145064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145064&action=review

> Source/WebCore/DerivedSources.pri:779
> +InjectedWebGLScriptSource.commands = perl $$PWD/inspector/xxd.pl InjectedWebGLScriptSource_js $$PWD/inspector/InjectedWebGLScriptSource.js ${QMAKE_FILE_OUT}

$$PWD/inspector/InjectedWebGLScriptSource.js -- > ${QMAKE_FILE_IN}

[Andrey Kosyakov]

Comment on attachment 145064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145064&action=review

> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:81
> +    if (context->is3d() && InspectorInstrumentation::isWebGLInspectorEnabled(canvas->document())) {
> +        ScriptObject wrapped = InspectorInstrumentation::wrapWebGLRenderingContextForInstrumentation(static_cast<WebGLRenderingContext*>(context));

Can we merge isWebGLInspectorEnabled() and wrapWebGLRenderingContextForInstrumentation() into one call, e.g. wrapWebGLRenderingContextForInstrumentationIfNecessary()?

> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
> +    // FIXME: Implement this!

Please file a bug to have this implemented.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:62
> +template<typename T> static v8::Local<v8::Object> toV8InLocalContext(v8::Handle<v8::FunctionTemplate> functionTemplate, WrapperTypeInfo* type, T* cptr)

We avoid abbreviations in WebKit, please find a more descriptive name for cptr.

> Source/WebCore/inspector/InjectedScriptManager.cpp:42
> +#if ENABLE(WEBGL)
> +#include "InjectedWebGLScriptSource.h"
> +#endif

Is this really necessary?

> Source/WebCore/inspector/InjectedScriptManager.cpp:182
> +    ScriptObject injectedWebGLScript = injectWebGLContextWrapperScript(injectedWebGLScriptSource(), scriptState);
> +    return executeInjectedWebGLContextWrapperScript(&injectedWebGLScript, scriptState, glContext);

Do we ever call any of these two function independently?

> Source/WebCore/inspector/InjectedWebGLScriptSource.js:2
> + * Copyright (C) 2012 Apple Inc.  All rights reserved.

Google copyright, please :)

> Source/WebCore/inspector/InspectorWebGLAgent.cpp:60
> +    m_instrumentingAgents = 0;

This appears redundant. I know we occasionally do this in other places, but I don't see justification for this there either.

> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:47
> +    if (InstrumentingAgents * instrumentingAgents = instrumentingAgentsForDocument(document)) {

InstrumentingAgents * => InstrumentingAgents*

> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:48
> +        if (InspectorWebGLAgent * webGLAgent = instrumentingAgents->inspectorWebGLAgent())

ditto

> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:59
> +    if (InstrumentingAgents * instrumentingAgents = instrumentingAgentsForDocument(glContext->canvas()->document())) {

ditto

> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:60
> +        if (InspectorWebGLAgent * webGLAgent = instrumentingAgents->inspectorWebGLAgent())

ditto

[Yury Semikhatsky]

Comment on attachment 145064 [details]
Patch

r- per Andrey's comments

[Andrey Adaikin]

Comment on attachment 145064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145064&action=review

>> Source/WebCore/DerivedSources.pri:779
>> +InjectedWebGLScriptSource.commands = perl $$PWD/inspector/xxd.pl InjectedWebGLScriptSource_js $$PWD/inspector/InjectedWebGLScriptSource.js ${QMAKE_FILE_OUT}
> 
> $$PWD/inspector/InjectedWebGLScriptSource.js -- > ${QMAKE_FILE_IN}

Done.

>> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:81
>> +        ScriptObject wrapped = InspectorInstrumentation::wrapWebGLRenderingContextForInstrumentation(static_cast<WebGLRenderingContext*>(context));
> 
> Can we merge isWebGLInspectorEnabled() and wrapWebGLRenderingContextForInstrumentation() into one call, e.g. wrapWebGLRenderingContextForInstrumentationIfNecessary()?

Removed isWebGLInspectorEnabled.

>> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
>> +    // FIXME: Implement this!
> 
> Please file a bug to have this implemented.

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

>> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:62
>> +template<typename T> static v8::Local<v8::Object> toV8InLocalContext(v8::Handle<v8::FunctionTemplate> functionTemplate, WrapperTypeInfo* type, T* cptr)
> 
> We avoid abbreviations in WebKit, please find a more descriptive name for cptr.

Done. cptr -> nativeObject

>> Source/WebCore/inspector/InjectedScriptManager.cpp:42
>> +#endif
> 
> Is this really necessary?

Yes, for the InjectedWebGLScriptSource_js variable

>> Source/WebCore/inspector/InjectedScriptManager.cpp:182
>> +    return executeInjectedWebGLContextWrapperScript(&injectedWebGLScript, scriptState, glContext);
> 
> Do we ever call any of these two function independently?

No. Merged into one injectWebGLScript()

>> Source/WebCore/inspector/InjectedWebGLScriptSource.js:2
>> + * Copyright (C) 2012 Apple Inc.  All rights reserved.
> 
> Google copyright, please :)

Done.

>> Source/WebCore/inspector/InspectorWebGLAgent.cpp:60
>> +    m_instrumentingAgents = 0;
> 
> This appears redundant. I know we occasionally do this in other places, but I don't see justification for this there either.

What exactly is redundant? Only the last line, or both?
As far as I see, it is to avoid a circular dependency, and both lines are "symmetrical" in that sense. If this is the case, I'd suggest preserving the symmetry, although it may be redundant.

>> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:47
>> +    if (InstrumentingAgents * instrumentingAgents = instrumentingAgentsForDocument(document)) {
> 
> InstrumentingAgents * => InstrumentingAgents*

Done.

>> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:48
>> +        if (InspectorWebGLAgent * webGLAgent = instrumentingAgents->inspectorWebGLAgent())
> 
> ditto

Done.

>> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:59
>> +    if (InstrumentingAgents * instrumentingAgents = instrumentingAgentsForDocument(glContext->canvas()->document())) {
> 
> ditto

Done.

>> Source/WebCore/inspector/InspectorWebGLInstrumentation.h:60
>> +        if (InspectorWebGLAgent * webGLAgent = instrumentingAgents->inspectorWebGLAgent())
> 
> ditto

Done.

[Andrey Adaikin]

Created attachment 145219 [details]
Patch

[Build Bot]

Comment on attachment 145219 [details]
Patch

Attachment 145219 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12866431

[Andrey Adaikin]

Created attachment 145233 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 145219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145219&action=review

> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
> +    // FIXME: Implement this!

Please file a bug against JSC and put its number after the fixme.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:138
> +    windowGlobal->SetHiddenValue(key, v);

I don't think you need to store the function in a hidden property. As I understand it is called only once to wrap glContextObject and then you use the wrapper. If so the hidden property is not needed. Please explain what the expected behavior is here.

> Source/WebCore/inspector/InspectorWebGLAgent.cpp:100
> +    ScriptState* scriptState = mainWorldScriptState(glContext->canvas()->document()->frame());

You should pass ScriptState as an explicit parameter. It is known in the bindings, you will need to covert v8::Context into ScriptState though.

[Andrey Adaikin]

Comment on attachment 145219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145219&action=review

>> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
>> +    // FIXME: Implement this!
> 
> Please file a bug against JSC and put its number after the fixme.

Done.

>> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:138
>> +    windowGlobal->SetHiddenValue(key, v);
> 
> I don't think you need to store the function in a hidden property. As I understand it is called only once to wrap glContextObject and then you use the wrapper. If so the hidden property is not needed. Please explain what the expected behavior is here.

Removed caching in the hidden property.

>> Source/WebCore/inspector/InspectorWebGLAgent.cpp:100
>> +    ScriptState* scriptState = mainWorldScriptState(glContext->canvas()->document()->frame());
> 
> You should pass ScriptState as an explicit parameter. It is known in the bindings, you will need to covert v8::Context into ScriptState though.

Done.

[Andrey Adaikin]

Created attachment 145251 [details]
Patch

[Build Bot]

Comment on attachment 145251 [details]
Patch

Attachment 145251 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12873223

[Andrey Kosyakov]

Comment on attachment 145251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145251&action=review

> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:82
> +        if (wrapped.jsValue().isObject())

Drop the .jsValue(). Just wrapped.isObject(). It's cleaner.

> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:46
> +#include "JSWebGLRenderingContext.h"

This could be replaced with forward declaration for the time being, I guess?

> Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:94
> +        if (!wrapped.v8Value().IsEmpty())

ditto.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:144
> +    v8::Local<v8::Value> injectedScriptValue = v8::Function::Cast(*v)->Call(windowGlobal, 1, args);

IIRC, this will crash if !v->IsFunction(). This might be ok given the assert above, but considering this may result from any JS error in a frequently modified injected script, we may want to fail more gracefully.

[Yury Semikhatsky]

Comment on attachment 145251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145251&action=review

> Source/WebCore/CMakeLists.txt:2602
> +ADD_CUSTOM_COMMAND(

Should we check if WebGL is enabled here?

> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
> +    // FIXME(87975): Implement this!

It shouldn't be hard to implement this for JSC, we already do similar things with the inspector injected script.

> Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:94
> +        if (!wrapped.v8Value().IsEmpty())

This code will create ScriptState instance for all contexts even if WebGL instrumentation is disabled. This should be fixed. We should probably check if WebGL inspection is enabled before getting ScriptState.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:-60
> -    v8::Local<v8::Function> function = V8InjectedScriptHost::GetTemplate()->GetFunction();

These changes are not needed anymore, please revert them if so.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:129
> +    v8::Local<v8::Object> glContextObject = toV8InLocalContext(V8WebGLRenderingContext::GetTemplate(), &V8WebGLRenderingContext::info, glContext);

You should be able to use standard toV8 mechanism here, it will create a binding for you which you will wrap with the instrumenting code. Existing code could probably changed as well I need to check.

[Andrey Adaikin]

Comment on attachment 145251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145251&action=review

>> Source/WebCore/CMakeLists.txt:2602
>> +ADD_CUSTOM_COMMAND(
> 
> Should we check if WebGL is enabled here?

Done.

>> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:82
>> +        if (wrapped.jsValue().isObject())
> 
> Drop the .jsValue(). Just wrapped.isObject(). It's cleaner.

Done. Using the hasNoValue() method now.

>> Source/WebCore/bindings/js/JSInjectedScriptManager.cpp:87
>> +    // FIXME(87975): Implement this!
> 
> It shouldn't be hard to implement this for JSC, we already do similar things with the inspector injected script.

Will do it in a separate patch. Let us finish with this first.

>>> Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:94
>>> +        if (!wrapped.v8Value().IsEmpty())
>> 
>> ditto.
> 
> This code will create ScriptState instance for all contexts even if WebGL instrumentation is disabled. This should be fixed. We should probably check if WebGL inspection is enabled before getting ScriptState.

Done.

>> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:129
>> +    v8::Local<v8::Object> glContextObject = toV8InLocalContext(V8WebGLRenderingContext::GetTemplate(), &V8WebGLRenderingContext::info, glContext);
> 
> You should be able to use standard toV8 mechanism here, it will create a binding for you which you will wrap with the instrumenting code. Existing code could probably changed as well I need to check.

Done.

[Andrey Adaikin]

Created attachment 145277 [details]
Patch

[Andrey Kosyakov]

Comment on attachment 145277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145277&action=review

> Source/WebCore/inspector/InspectorWebGLAgent.cpp:62
> +    m_instrumentingAgents = 0;

This one still appears redundant. Do we really need to clear this field in desturctor? Also, it belongs to the base class and is initialized by the base class, so it's unclear why it's cleared by derived class.

[Andrey Adaikin]

Comment on attachment 145277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145277&action=review

>> Source/WebCore/inspector/InspectorWebGLAgent.cpp:62
>> +    m_instrumentingAgents = 0;
> 
> This one still appears redundant. Do we really need to clear this field in desturctor? Also, it belongs to the base class and is initialized by the base class, so it's unclear why it's cleared by derived class.

Good point. Removed the line.

[Yury Semikhatsky]

Comment on attachment 145277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145277&action=review

> Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:94
> +        if (InspectorInstrumentation::isWebGLInspectorEnabled(imp->document())) {

Alternatively you could check InspectorInstrumentation::hasFrontends() here to avoid adding additional is* method on the inspector instrumentation.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:138
> +    v8::Local<v8::Object> injectedScriptObject(v8::Object::Cast(*injectedScriptValue));

You could do v8::Handle<v8::Object>::Cast(injectedScriptValue) instead to avoid creating additional handle.

[Andrey Adaikin]

Created attachment 145282 [details]
Patch

[Andrey Adaikin]

Created attachment 145287 [details]
Patch

[Andrey Adaikin]

Comment on attachment 145277 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145277&action=review

>> Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:94
>> +        if (InspectorInstrumentation::isWebGLInspectorEnabled(imp->document())) {
> 
> Alternatively you could check InspectorInstrumentation::hasFrontends() here to avoid adding additional is* method on the inspector instrumentation.

Done.

>> Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp:138
>> +    v8::Local<v8::Object> injectedScriptObject(v8::Object::Cast(*injectedScriptValue));
> 
> You could do v8::Handle<v8::Object>::Cast(injectedScriptValue) instead to avoid creating additional handle.

Done.

[Build Bot]

Comment on attachment 145287 [details]
Patch

Attachment 145287 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12864557

[Andrey Adaikin]

Created attachment 145310 [details]
Patch

[Andrey Adaikin]

Created attachment 145325 [details]
Patch

[Build Bot]

Comment on attachment 145325 [details]
Patch

Attachment 145325 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12864605

[Andrey Adaikin]

Created attachment 145331 [details]
Patch

[Andrey Adaikin]

Created attachment 145336 [details]
Patch

[Build Bot]

Comment on attachment 145336 [details]
Patch

Attachment 145336 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12866608

[Build Bot]

Comment on attachment 145336 [details]
Patch

Attachment 145336 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12869564

[Andrey Adaikin]

Created attachment 145526 [details]
Patch

Checking build bots

[Build Bot]

Comment on attachment 145526 [details]
Patch

Attachment 145526 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12893213

[Andrey Kosyakov]

Comment on attachment 145526 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145526&action=review

> Source/WebCore/WebCore.gypi:2824
> +            'inspector/InspectorWebGLAgent.cpp',
> +            'inspector/InspectorWebGLAgent.h',
> +            'inspector/InspectorWebGLInstrumentation.h',

Please also add this to WebCore.vcproj.

[Andrey Adaikin]

Created attachment 145551 [details]
Patch

Checking build bots: Updated project.pbxproj, reverted WebCode.exp.in

[Andrey Adaikin]

Created attachment 145553 [details]
Patch

Checking build bots

[Build Bot]

Comment on attachment 145551 [details]
Patch

Attachment 145551 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12898201

[Andrey Adaikin]

Created attachment 145555 [details]
Patch

Checking build bots: added WebCode.exp.in back

[Build Bot]

Comment on attachment 145553 [details]
Patch

Attachment 145553 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12903155

[Early Warning System Bot]

Comment on attachment 145555 [details]
Patch

Attachment 145555 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12902169

[Build Bot]

Comment on attachment 145555 [details]
Patch

Attachment 145555 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12894281

[Andrey Adaikin]

Created attachment 145705 [details]
Patch

Checking build bots

[Andrey Adaikin]

Created attachment 145714 [details]
Patch

Checking build bots

[Build Bot]

Comment on attachment 145705 [details]
Patch

Attachment 145705 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12900423

[Build Bot]

Comment on attachment 145705 [details]
Patch

Attachment 145705 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12901418

[Build Bot]

Comment on attachment 145714 [details]
Patch

Attachment 145714 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12898475

[Andrey Adaikin]

Created attachment 145734 [details]
Patch

Checking build bots: added WebCore.order

(In reply to comment #47)
> Created an attachment (id=145734) [details]
> Patch
> 
> Checking build bots: added WebCore.order

You need to add those symbols to WebCore.exp.in.

WebCore.order is only used for Safari builds or something, Apple engineers will take care of updating it

[Andrey Adaikin]

(In reply to comment #48)
> (In reply to comment #47)
> > Created an attachment (id=145734) [details] [details]
> > Patch
> > 
> > Checking build bots: added WebCore.order
> 
> You need to add those symbols to WebCore.exp.in.
> 
> WebCore.order is only used for Safari builds or something, Apple engineers will take care of updating it

I did add those symbols to WebCore.exp.in - no luck so far...

(In reply to comment #49)
> (In reply to comment #48)
> > (In reply to comment #47)
> > > Created an attachment (id=145734) [details] [details] [details]
> > > Patch
> > > 
> > > Checking build bots: added WebCore.order
> > 
> > You need to add those symbols to WebCore.exp.in.
> > 
> > WebCore.order is only used for Safari builds or something, Apple engineers will take care of updating it
> 
> I did add those symbols to WebCore.exp.in - no luck so far...

ah, indeed

on closer inspection, your patch to project.pbxproj looks incomplete. It doesn't list the new files in the PBXBuildFile section

[Andrey Adaikin]

(In reply to comment #50)
> (In reply to comment #49)
> > (In reply to comment #48)
> > > (In reply to comment #47)
> > > > Created an attachment (id=145734) [details] [details] [details] [details]
> > > > Patch
> > > > 
> > > > Checking build bots: added WebCore.order
> > > 
> > > You need to add those symbols to WebCore.exp.in.
> > > 
> > > WebCore.order is only used for Safari builds or something, Apple engineers will take care of updating it
> > 
> > I did add those symbols to WebCore.exp.in - no luck so far...
> 
> ah, indeed
> 
> on closer inspection, your patch to project.pbxproj looks incomplete. It doesn't list the new files in the PBXBuildFile section

yeah I played with it earlier at patch https://bugs.webkit.org/attachment.cgi?id=145551&action=review (my xcode would not add PBXBuildFile section for some reason) and got the following error from the bot:


xcodebuild: error: Unable to read project 'WebCore.xcodeproj'.
	Reason: Project /Volumes/Data/WebKit/Source/WebCore/WebCore.xcodeproj cannot be opened because because an exception occurred.: -[PBXFileReference _setBuildPhase:]: unrecognized selector sent to instance 0x4010c9140

[Early Warning System Bot]

Comment on attachment 145734 [details]
Patch

Attachment 145734 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12905415

[Build Bot]

Comment on attachment 145734 [details]
Patch

Attachment 145734 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12902459

[Build Bot]

Comment on attachment 145734 [details]
Patch

Attachment 145734 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12893550

[Andrey Adaikin]

Created attachment 145765 [details]
Patch

Checking build bots: tweaked project.pbxproj, removed WebCore.order

[Andrey Adaikin]

Created attachment 145770 [details]
Patch

Checking build bots: fixed project.pbxproj

[Build Bot]

Comment on attachment 145765 [details]
Patch

Attachment 145765 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12901478

[Build Bot]

Comment on attachment 145770 [details]
Patch

Attachment 145770 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12902506

[Andrey Adaikin]

Created attachment 145791 [details]
Patch

Checking build bots (hoping for the last time)

[Build Bot]

Comment on attachment 145770 [details]
Patch

Attachment 145770 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12893593

[Andrey Adaikin]

Created attachment 145802 [details]
Patch

Checking build bots: reverted WebCore.exp.in

[Andrey Adaikin]

Created attachment 145959 [details]
=Rebased patch

Rebased patch

[Andrey Kosyakov]

Committed r119572: <http://trac.webkit.org/changeset/119572>