Bug 61300 – Web Inspector: Reorganize InspectorBackendDispatch code to eliminate duplication

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

[Mikhail Naganov]

A small refactoring shaves off 10-20% (platform dependent) size of InspectorBackendDispatch.o file (release version).

[Mikhail Naganov]

Created attachment 94475 [details]
patch

[Ilya Tikhonovsky]

Comment on attachment 94475 [details]
patch

could you please provide a sample of generated code.

[Mikhail Naganov]

This is an example of generated code:

void InspectorBackendDispatcher::Page_getResourceContent(long callId, InspectorObject* requestMessageObject)
{
    RefPtr<InspectorArray> protocolErrors = InspectorArray::create();

    if (!m_pageAgent)
        protocolErrors->pushString("Page handler is not available.");

    String out_content = "";

    ErrorString error;

    if (RefPtr<InspectorObject> paramsContainer = requestMessageObject->getObject("params")) {
        String in_frameId = getString(paramsContainer.get(), "frameId", false, protocolErrors.get());
        String in_url = getString(paramsContainer.get(), "url", false, protocolErrors.get());
        bool in_base64Encode = getBoolean(paramsContainer.get(), "base64Encode", true, protocolErrors.get());

        if (!protocolErrors->length())
            m_pageAgent->getResourceContent(&error, in_frameId, in_url, &in_base64Encode, &out_content);
    } else
        protocolErrors->pushString("'params' property with type 'object' was not found.");
    RefPtr<InspectorObject> result = InspectorObject::create();
    if (!protocolErrors->length() && !error.length()) {
        result->setString("content", out_content);
    }
    sendResponse(callId, result, protocolErrors, error);
}

and InspectorBackendDispatcher::sendResponse looks like this:

void InspectorBackendDispatcher::sendResponse(long callId, PassRefPtr<InspectorObject> result, PassRefPtr<InspectorArray> protocolErrors, ErrorString invocationError)
{
    // use InspectorFrontend as a marker of WebInspector availability

    if (protocolErrors->length()) {
        reportProtocolError(&callId, InvalidParams, protocolErrors);
        return;
    }
    if (invocationError.length()) {
        reportProtocolError(&callId, ServerError, invocationError);
        return;
    }

    RefPtr<InspectorObject> responseMessage = InspectorObject::create();
    responseMessage->setObject("result", result);
    responseMessage->setNumber("id", callId);
    if (m_inspectorFrontendChannel)
        m_inspectorFrontendChannel->sendMessageToFrontend(responseMessage->toJSONString());
}    


I have moved result fields assignment higher, so the bottom part of the request handler has all the same code for all handlers. And I have moved this code out to "sendResponse".

[Ilya Tikhonovsky]

Comment on attachment 94475 [details]
patch

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

lgtm

> Source/WebCore/inspector/CodeGeneratorInspector.pm:510
> +    // use InspectorFrontend as a marker of WebInspector availability

please remove this comment

[Mikhail Naganov]

Created attachment 94603 [details]
patch

Comments addressed.

Also I have shrunk the size of the 'dispatch' method by organizing command names and pointers into arrays and replacing series of "dispatch.add" with a loop.

Please take another look.

[Mikhail Naganov]

Manually committed http://trac.webkit.org/changeset/87146


    2011-05-24  Mikhail Naganov  <[e-mail]>
    
            Reviewed by Yury Semikhatsky.
    
            Web Inspector: Reorganize InspectorBackendDispatch code to eliminate duplication
            https://bugs.webkit.org/show_bug.cgi?id=61300
    
            * inspector/CodeGeneratorInspector.pm:
    
    2011-05-24  Mikhail Naganov  <[e-mail]>
    
            Reviewed by Yury Semikhatsky.
    
            Web Inspector: Reorganize InspectorBackendDispatch code to eliminate duplication
            https://bugs.webkit.org/show_bug.cgi?id=61300
    
            * src/WebDevToolsAgentImpl.cpp:
            (WebKit::WebDevToolsAgent::shouldInterruptForMessage):