Bug 61300 - Web Inspector: Reorganize InspectorBackendDispatch code to eliminate duplication
Summary: Web Inspector: Reorganize InspectorBackendDispatch code to eliminate duplication
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mikhail Naganov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-23 13:37 PDT by Mikhail Naganov
Modified: 2011-05-24 07:37 PDT (History)
10 users (show)

See Also:


Attachments
patch (6.68 KB, patch)
2011-05-23 13:40 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
patch (12.68 KB, patch)
2011-05-24 06:35 PDT, Mikhail Naganov
yurys: review+
mnaganov: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Naganov 2011-05-23 13:37:40 PDT
A small refactoring shaves off 10-20% (platform dependent) size of InspectorBackendDispatch.o file (release version).
Comment 1 Mikhail Naganov 2011-05-23 13:40:34 PDT
Created attachment 94475 [details]
patch
Comment 2 Ilya Tikhonovsky 2011-05-23 22:52:20 PDT
Comment on attachment 94475 [details]
patch

could you please provide a sample of generated code.
Comment 3 Mikhail Naganov 2011-05-24 00:32:54 PDT
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".
Comment 4 Ilya Tikhonovsky 2011-05-24 05:37:33 PDT
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
Comment 5 Mikhail Naganov 2011-05-24 06:35:56 PDT
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.
Comment 6 Mikhail Naganov 2011-05-24 07:37:38 PDT
Manually committed http://trac.webkit.org/changeset/87146


    2011-05-24  Mikhail Naganov  <mnaganov@chromium.org>
    
            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  <mnaganov@chromium.org>
    
            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):