Bug 88973 – Web Inspector: [WebGL] Add minimum transport protocol from backend to frontend

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

[Andrey Adaikin]

Minimum transport protocol implementation between backend and frontend to allow capturing WebGL function calls for a given frame.
This will add:
- WebGL.captureFrame command from FE to BE
- WebGL.contextCreated & WebGL.reportFunctionCall events from BE to FE
- simple experimental implementation of the InjectedWebGLScriptSource.js

[Andrey Adaikin]

Created attachment 147259 [details]
Patch

[WebKit Review Bot]

Attachment 147259 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/inspector/InjectedScriptBase.cpp:90:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScriptBase.cpp:109:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScriptBase.h:63:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScriptBase.h:64:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrey Adaikin]

Extracted part of this change to https://bugs.webkit.org/show_bug.cgi?id=89087

[Andrey Adaikin]

Implementation of the InjectedWebGLScriptSource.js extracted to https://bugs.webkit.org/show_bug.cgi?id=89088

[Pavel Feldman]

Comment on attachment 147259 [details]
Patch

Clearing r? from the obsolete patch.

[Andrey Adaikin]

Created attachment 159385 [details]
Patch

[WebKit Review Bot]

Attachment 159385 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:95:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScriptWebGLModule.h:54:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 159385 [details]
Patch

Attachment 159385 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13529906

[Pavel Feldman]

Comment on attachment 159385 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:75
> +void InjectedScriptWebGLModule::captureFrame(ErrorString* errorString, String* traceLogId)

captureTraceLog ?

>> Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:95
>> +void InjectedScriptWebGLModule::getTraceLog(ErrorString* errorString, const String& traceLogId, RefPtr<TypeBuilder::WebGL::TraceLog>* traceLog)
> 
> The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]

::traceLog (according to the WebKit guidelines). We only use get prefixes in the protocol.

> Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:97
> +    ScriptFunctionCall function(injectedScriptObject(), "getTraceLog");

traceLog

> Source/WebCore/inspector/Inspector.json:3160
> +                    { "name": "traceLogId", "$ref": "TraceLogId" }

Is there a "logCaptured" event? I wonder if you should return traceLogId there instead.

> Source/WebCore/inspector/Inspector.json:3164
> +                "name": "getTraceLog",

traceLog

> Source/WebCore/inspector/InspectorWebGLAgent.cpp:103
> +    InjectedScript injectedScript = m_injectedScriptManager->injectedScriptForObjectId(traceLogId);

It sounds like N-1 lines from here could be extracted and reused below.

[Andrey Adaikin]

Comment on attachment 159385 [details]
Patch

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

>> Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:75
>> +void InjectedScriptWebGLModule::captureFrame(ErrorString* errorString, String* traceLogId)
> 
> captureTraceLog ?

This is to capture only a single frame, i.e. we say webgl module to start capturing now *and* end capturing after the frame ends. Opposite to that, there may be startCapturing & endCapturing separate commands in the future, should we want to support capturing arbitrary number of frames.

>> Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:97
>> +    ScriptFunctionCall function(injectedScriptObject(), "getTraceLog");
> 
> traceLog

done.

>> Source/WebCore/inspector/Inspector.json:3160
>> +                    { "name": "traceLogId", "$ref": "TraceLogId" }
> 
> Is there a "logCaptured" event? I wonder if you should return traceLogId there instead.

No events for now (need a InjectedModuleHost class implementation). For now we will be poking the trace logs with getTraceLog after a time out.

>> Source/WebCore/inspector/Inspector.json:3164
>> +                "name": "getTraceLog",
> 
> traceLog

did you really mean it? this is a protocol command, and as you said, we do use "get" prefixes in the protocol.

>> Source/WebCore/inspector/InspectorWebGLAgent.cpp:103
>> +    InjectedScript injectedScript = m_injectedScriptManager->injectedScriptForObjectId(traceLogId);
> 
> It sounds like N-1 lines from here could be extracted and reused below.

done.

[Andrey Adaikin]

Created attachment 159420 [details]
Patch

[WebKit Review Bot]

Attachment 159420 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/inspector/InjectedScriptWebGLModule.cpp:95:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScriptWebGLModule.h:56:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 159420 [details]
Patch

Rejecting attachment 159420 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Review Bot]

Comment on attachment 159420 [details]
Patch

Clearing flags on attachment: 159420

Committed r126028: <http://trac.webkit.org/changeset/126028>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 94576

[Andrey Adaikin]

Created attachment 159879 [details]
Rebased patch to land

[Andrey Kosyakov]

Committed r126280: <http://trac.webkit.org/changeset/126280>