[webkit-reviews] review denied: [Bug 91528] Web Inspector: Embeddable Web Inspector : [Attachment 155833] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Gabriel Peal
<gpeal at google.com>'s request for review:
Bug 91528: Web Inspector: Embeddable Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=91528

Attachment 155833: Patch
https://bugs.webkit.org/attachment.cgi?id=155833&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=155833&action=review


This change is missing the ChangeLog.

> Source/WebCore/inspector/front-end/inspector.js:418
> +	      
InspectorFrontendAPI.dispatch([WebInspector.queryParamsObject["callAPI"],WebIns
pector.queryParamsObject["params"]])

space after ",". I would also name it dispatchQueryParameters and move
queryParamsObject along with the constants into the InspectorFrontendAPI

> Source/WebCore/inspector/front-end/utilities.js:703
> +function loadXHR(url, sync, callback) {

XHR is typically using "async" parameter.

> Source/WebCore/inspector/front-end/utilities.js:720
> +    xhr.open("GET", url, false);

false -> async ?

> Source/WebCore/inspector/front-end/utilities.js:721
> +    if (sync === false)

if (async)

> Source/WebCore/inspector/front-end/utilities.js:724
> +    if (sync === true) {

else or if (!async)