Changeset 188897 in webkit


Ignore:
Timestamp:
Aug 24, 2015 4:58:31 PM (9 years ago)
Author:
BJ Burg
Message:

Web Inspector: add protocol test for existing error handling performed by the backend
​https://bugs.webkit.org/show_bug.cgi?id=147097

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

A new test revealed that the protocol "method" parameter was being parsed in a naive way.
Rewrite it to use String::split and improve error checking to avoid failing later.

  • inspector/InspectorBackendDispatcher.cpp:

(Inspector::BackendDispatcher::dispatch):

Source/WebInspectorUI:

Add a way to send raw messages to the backend while still awaiting on responses.
This is necessary to test protocol error handling in the inspector backend.

  • UserInterface/Test/InspectorProtocol.js:

(InspectorProtocol.sendCommand):
(InspectorProtocol.awaitCommand): Use awaitMessage internally.
(InspectorProtocol.awaitMessage): Added. Use a dummy requestId if none is supplied.
(InspectorProtocol._sendMessage): Added.
(InspectorProtocol.dispatchMessageFromBackend):
Reject with the error object instead of the error message, so error code/data can be checked.
(InspectorProtocol.sendMessage): Deleted, it is now a private method.

LayoutTests:

Add a bunch of test cases to cover existing error handling by the backend dispatcher.

  • inspector/protocol/backend-dispatcher-argument-errors-expected.txt: Added.
  • inspector/protocol/backend-dispatcher-argument-errors.html: Added.
  • inspector/protocol/backend-dispatcher-malformed-message-errors-expected.txt: Added.
  • inspector/protocol/backend-dispatcher-malformed-message-errors.html: Added.
Location:
trunk
Files:
4 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r188896 r188897  
     12015-08-24  Brian Burg  <bburg@apple.com>
     2
     3        Web Inspector: add protocol test for existing error handling performed by the backend
     4        https://bugs.webkit.org/show_bug.cgi?id=147097
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        Add a bunch of test cases to cover existing error handling by the backend dispatcher.
     9
     10        * inspector/protocol/backend-dispatcher-argument-errors-expected.txt: Added.
     11        * inspector/protocol/backend-dispatcher-argument-errors.html: Added.
     12        * inspector/protocol/backend-dispatcher-malformed-message-errors-expected.txt: Added.
     13        * inspector/protocol/backend-dispatcher-malformed-message-errors.html: Added.
     14
    1152015-08-24  Alexey Proskuryakov  <ap@apple.com>
    216

  • trunk/Source/JavaScriptCore/ChangeLog

    r188894 r188897  
     12015-08-24  Brian Burg  <bburg@apple.com>
     2
     3        Web Inspector: add protocol test for existing error handling performed by the backend
     4        https://bugs.webkit.org/show_bug.cgi?id=147097
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        A new test revealed that the protocol "method" parameter was being parsed in a naive way.
     9        Rewrite it to use String::split and improve error checking to avoid failing later.
     10
     11        * inspector/InspectorBackendDispatcher.cpp:
     12        (Inspector::BackendDispatcher::dispatch):
     13
    1142015-08-24  Yusuke Suzuki  <utatane.tea@gmail.com>
    215

  • trunk/Source/JavaScriptCore/inspector/InspectorBackendDispatcher.cpp

    r180116 r188897  
    107107    }
    108108
    109     String method;
    110     if (!methodValue->asString(method)) {
     109    String methodString;
     110    if (!methodValue->asString(methodString)) {
    111111        reportProtocolError(&callId, InvalidRequest, ASCIILiteral("The type of 'method' property must be string"));
    112112        return;
    113113    }
    114114
    115     size_t position = method.find('.');
    116     if (position == WTF::notFound) {
     115    Vector<String> domainAndMethod;
     116    methodString.split('.', true, domainAndMethod);
     117    if (domainAndMethod.size() != 2 || !domainAndMethod[0].length() || !domainAndMethod[1].length()) {
    117118        reportProtocolError(&callId, InvalidRequest, ASCIILiteral("The 'method' property was formatted incorrectly. It should be 'Domain.method'"));
    118119        return;
    119120    }
    120121
    121     String domain = method.substring(0, position);
     122    String domain = domainAndMethod[0];
    122123    SupplementalBackendDispatcher* domainDispatcher = m_dispatchers.get(domain);
    123124    if (!domainDispatcher) {
    … …  
    126127    }
    127128
    128     String domainMethod = method.substring(position + 1);
    129     domainDispatcher->dispatch(callId, domainMethod, messageObject.releaseNonNull());
     129    String method = domainAndMethod[1];
     130    domainDispatcher->dispatch(callId, method, messageObject.releaseNonNull());
    130131}
    131132

  • trunk/Source/WebInspectorUI/ChangeLog

    r188876 r188897  
     12015-08-24  Brian Burg  <bburg@apple.com>
     2
     3        Web Inspector: add protocol test for existing error handling performed by the backend
     4        https://bugs.webkit.org/show_bug.cgi?id=147097
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        Add a way to send raw messages to the backend while still awaiting on responses.
     9        This is necessary to test protocol error handling in the inspector backend.
     10
     11        * UserInterface/Test/InspectorProtocol.js:
     12        (InspectorProtocol.sendCommand):
     13        (InspectorProtocol.awaitCommand): Use awaitMessage internally.
     14        (InspectorProtocol.awaitMessage): Added. Use a dummy requestId if none is supplied.
     15        (InspectorProtocol._sendMessage): Added.
     16        (InspectorProtocol.dispatchMessageFromBackend):
     17        Reject with the error object instead of the error message, so error code/data can be checked.
     18        (InspectorProtocol.sendMessage): Deleted, it is now a private method.
     19
    1202015-08-24  Timothy Hatcher  <timothy@apple.com>
    221

  • trunk/Source/WebInspectorUI/UserInterface/Test/InspectorProtocol.js

    r188639 r188897  
    2727InspectorProtocol = {};
    2828InspectorProtocol._dispatchTable = [];
     29InspectorProtocol._placeholderRequestIds = [];
    2930InspectorProtocol._requestId = -1;
    3031InspectorProtocol.eventHandler = {};
    … …  
    3940    this._dispatchTable[++this._requestId] = handler;
    4041    let messageObject = {method, params, id: this._requestId};
    41     this.sendMessage(messageObject);
     42    this._sendMessage(messageObject);
    4243
    4344    return this._requestId;
    … …  
    4748{
    4849    let {method, params} = args;
     50    let messageObject = {method, params, id: ++this._requestId};
     51
     52    return this.awaitMessage(messageObject);
     53}
     54
     55InspectorProtocol.awaitMessage = function(messageObject)
     56{
     57    // Send a raw message to the backend. Mostly used to test the backend's error handling.
    4958    return new Promise((resolve, reject) => {
    50         this._dispatchTable[++this._requestId] = {resolve, reject};
    51         let messageObject = {method, params, id: this._requestId};
    52         this.sendMessage(messageObject);
     59        let requestId = messageObject.id;
     60
     61        // If the caller did not provide an id, then make one up so that the response
     62        // can be used to settle a promise.
     63        if (typeof requestId !== "number") {
     64            requestId = ++this._requestId;
     65            this._placeholderRequestIds.push(requestId);
     66        }
     67
     68        this._dispatchTable[requestId] = {resolve, reject};
     69        this._sendMessage(messageObject);
    5370    });
    5471}
    … …  
    6683        }
    6784    });
     85}
     86
     87InspectorProtocol._sendMessage = function(messageObject)
     88{
     89    let messageString = typeof messageObject !== "string" ? JSON.stringify(messageObject) : messageObject;
     90
     91    if (ProtocolTest.dumpInspectorProtocolMessages)
     92        console.log(`frontend: ${messageString}`);
     93
     94    InspectorFrontendHost.sendMessageToBackend(messageString);
    6895}
    6996
    … …  
    94121}
    95122
    96 InspectorProtocol.sendMessage = function(messageObject)
    97 {
    98     // This matches the debug dumping in InspectorBackend, which is bypassed
    99     // by InspectorProtocol. Return messages should be dumped by InspectorBackend.
    100     if (ProtocolTest.dumpInspectorProtocolMessages)
    101         console.log("frontend: " + JSON.stringify(messageObject));
    102 
    103     InspectorFrontendHost.sendMessageToBackend(JSON.stringify(messageObject));
    104 }
    105 
    106123InspectorProtocol.checkForError = function(responseObject)
    107124{
    … …  
    122139    // If the message has an id, then it is a reply to a command.
    123140    let messageId = messageObject.id;
     141
     142    // If the id is 'null', then it may be an error response.
     143    if (messageId === null)
     144        messageId = InspectorProtocol._placeholderRequestIds.shift();
     145
     146    // If we could figure out a requestId, then dispatch the message.
    124147    if (typeof messageId === "number") {
    125148        let handler = InspectorProtocol._dispatchTable[messageId];
    … …  
    132155            let {resolve, reject} = handler;
    133156            if ("error" in messageObject)
    134                 reject(messageObject.error.message);
     157                reject(messageObject.error);
    135158            else
    136159                resolve(messageObject.result);
    … …  
    150173            let {resolve, reject} = handler;
    151174            if ("error" in messageObject)
    152                 reject(messageObject.error.message);
     175                reject(messageObject.error);
    153176            else
    154177                resolve(messageObject.result);
Note: See TracChangeset for help on using the changeset viewer.