Bug 146466 – Web Inspector: no need to allocate protocolErrors array for every dispatched backend command

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Brian Burg]

This can be reused per domain dispatcher, removing one InspectorArray allocation per dispatched command in the normal case.

While this is done, can also extract the code for getting the parameters object out of the payload into the dispatcher's main dispatch method. This removes more duplicated code from every generated command binding.

[Radar WebKit Bug Importer]

<rdar://problem/21616637>

[Brian Burg]

Created attachment 256229 [details]
Patch (no tests)

[Brian Burg]

This patch makes some subtle changes to how errors are reported. I'd like to write a test to cover the existing error cases before landing this, so we have a clear idea of the behavior change.

[Blaze Burg]

Created attachment 259866 [details]
Proposed Fix

[WebKit Commit Bot]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[Joseph Pecoraro]

Comment on attachment 259866 [details]
Proposed Fix

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

Nice! Much cleaner

> Source/JavaScriptCore/ChangeLog:67
> +        * inspector/scripts/codegen/generate_cpp_backend_dispatcher_header.py:

Duplicated in the changelog?

> Source/JavaScriptCore/inspector/InspectorBackendDispatcher.h:75
> +    unsigned protocolErrorCount() const { return m_protocolErrors.size(); }

This is only ever used alongside "> 0". Maybe it should just be `bool hasProtocolErrors()`?

[Blaze Burg]

Created attachment 259889 [details]
Proposed Fix (fix linker)

[Blaze Burg]

Created attachment 259903 [details]
Proposed Fix (EWS)

Now includes fixes for ObjC generated code.

[Joseph Pecoraro]

Comment on attachment 259903 [details]
Proposed Fix (EWS)

r=me

[Blaze Burg]

Committed r188965: <http://trac.webkit.org/changeset/188965>

[Csaba Osztrogonác]

(In reply to comment #10)
> Committed r188965: <http://trac.webkit.org/changeset/188965>

It broke the WinCairo build: https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/48696

[Brent Fulgham]

Interesting. Looks like a symbol isn't getting exported. Alex: I wonder if it's our old friend "the overly zealous symbol stripper".

[Alex Christensen]

I deleted the WebKitBuild directory on my bot.  I'm just going to close my eyes and hope this magically fixes it.