[Webkit-unassigned] [Bug 69295] Web Inspector: reimplement protocol backend/frontend source generator

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 7 09:38:35 PDT 2011


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





--- Comment #22 from Peter Rybin <peter.rybin at gmail.com>  2011-10-07 09:38:34 PST ---
Hi Ilya,

Thank you for review!

In this change I'm trying to provide a generator that is character-compatible with the old version. Old extra or missing lines are kept as legacy. I think we should leave all improvements to the next CL not to do 2 things at once.

> > Source/WebCore/ChangeLog:10655
> > +2011-10-03  Peter Rybin  <peter.rybin at gmail.com>
> > +
> > +        Web Inspector: reimplement protocol backend/frontend source generator
> > +        https://bugs.webkit.org/show_bug.cgi?id=69295
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        No new tests. (OOPS!)
> > +
> > +        * WebCore.gyp/WebCore.gyp:
> > +        * inspector/CodeGeneratorInspector.pm:
> > +        (finish):
> > +        * inspector/CodeGeneratorInspector.py: Added.
> > +
> 
> second change log entry.

Done

> > Source/WebCore/CodeGenerators.pri:679
> > +inspectorJSON.output = $${WC_GENERATED_SOURCES_DIR}/InspectorFrontend.cpp $${WC_GENERATED_SOURCES_DIR}/InspectorBackendDispatcher.cpp
> please add headers too

Are you sure it's the right thing to do? Look at the removed target below, it doesn't list header files.

> > Source/WebCore/WebCore.gyp/WebCore.gyp:369
> > +          'message': 'Generating Inspector protocol sources from Inspector.idl',
> from Inspector.json
Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:449
> > +${responseCook}sendResponse(callId, result, String::format("Some arguments of method '%s' can't be processed", "$domainName.$methodName"), protocolErrors, error);
> It'd be nice to extract String::format("Some arguments of method '%s' can't be processed", "some method name") call to an inline function.

There's no need for formatting it on runtime. We should format it right in generator instead.

> > Source/WebCore/inspector/CodeGeneratorInspector.py:540
> > +$enumConstants
> $methodNamesEnumContent ?

Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:601
> > +$messageHandlers
> wrong indent in the generated file.

This is a multi-line placeholder. Indents are inside argument.

> > Source/WebCore/inspector/CodeGeneratorInspector.py:840
> > +PassRefPtr<InspectorObject> InspectorBackendDispatcher::getObject(InspectorObject* object, const String& name, bool* valueFound, InspectorArray* protocolErrors)
> > +{
> The old version is generating the getters

I missed this. Let me handle it in the next change. I would consider leaving it as it is or programming using C++ template technology (to keep the minimum amount of generated code).

> > Source/WebCore/inspector/CodeGeneratorInspector.py:1398
> > +    enumConstants=join(Generator.method_name_enum_list, "\n"),
> bad name

Done

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list