[Webkit-unassigned] [Bug 91528] Embeddable Developer Tools

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Andrey Kosyakov <caseq at chromium.org>  2012-07-18 07:53:55 PST ---
(From update of attachment 152793)
View in context: https://bugs.webkit.org/attachment.cgi?id=152793&action=review

> Source/WebCore/inspector/front-end/InspectorEmbedded.js:5
> +var domContentLoaded = function() {
> +    InspectorFrontendAPI.showTimeline();

This needs to be more generic -- e.g. allow various "commands" passed via query string.

> Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js:39
> +    this._hiddenPanels = [];

I don't think this belongs to InspectorFrontendHostStub -- this is supposed to be stub for InspectorFornendHost, after all.

> Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js:120
> +    addHiddenPanel: function(panel)
> +    {
> +        if (this._hiddenPanels.indexOf(panel) == -1)
> +            this._hiddenPanels.push(panel);
> +    },
> +
> +    removeHiddenPanel: function(panel)
> +    {
> +        var panelIndex = this._hiddenPanels.indexOf(panel);
> +        if (panelIndex != -1)
> +            this._hiddenPanels.splice(panelIndex,1);
>      },

Ditto.

> Source/WebCore/inspector/front-end/TimelineModel.js:169
> +    loadRecords: function(records)

Here and below, please add JSDoc annotations.

> Source/WebCore/inspector/front-end/TimelinePanel.js:778
> +        if(xhr.status == 200) {

style: if (xhr.status === 200)
also, is there a reason for this to be synchronous?

> Source/WebCore/inspector/front-end/TimelinePanel.js:779
> +            var data = JSON.parse(xhr.responseText);

I think we need some diagnostics in case parsing fails.

> Source/WebCore/inspector/front-end/inspector.html:224
> +    <script type="text/javascript" src="InspectorEmbedded.js"></script>

You will have to add this to several project files -- please look up changes that added other files recently.

> Source/WebCore/inspector/front-end/inspector.js:415
>      }
> +    else if (InspectorFrontendHost.isStub) {

style: "} else" need to be on the same line.

> Source/WebCore/inspector/front-end/inspector.js:419
> +            var hiddenPanels = WebInspector.queryParamsObject["hiddenPanels"].split(",");
> +            for (var i = 0; i < hiddenPanels.length; ++i)
> +                InspectorFrontendHost.addHiddenPanel(hiddenPanels[i]);

nit: WebInspector.queryParamsObject["hiddenPanels"].split(",").forEach(InspectorFrontendHost.addHiddenPanel, InspectorFrontendHost)

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