[webkit-reviews] review denied: [Bug 92762] Web Inspector: display progress bar while loading timeline data : [Attachment 155571] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 92762: Web Inspector: display progress bar while loading timeline data
https://bugs.webkit.org/show_bug.cgi?id=92762

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

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


> Source/WebCore/WebCore.gypi:6343
> +	       'inspector/front-end/ProgressBar.js',

Please mind alphabetic order

> Source/WebCore/inspector/front-end/Panel.js:265
> +	   this._glassPane = new WebInspector.GlassPane(this.element);

I don't think we need to block the UI.

> Source/WebCore/inspector/front-end/ProgressBar.js:61
> +    hide: function()

I don't think that the task should be able to stop hide the progress indicator.
It also prevents you from implementing composite progress properly. The idustry
standard is to have "Progress" and "ProgressIndicator" entities. First is an
interface, second is a UI component implementing this interface.

Progress {
    setTotalWork(number),
    worked(opt_number, opt_title),
    setTitle(title),
    done(),
    boolean isCanceled()
}

CompositeProgress (implements the Progress as well) (parent, 100)
var p1 = subProgress(20);
var p2 = subProgress(30);

> Source/WebCore/inspector/front-end/ProgressBar.js:74
> +    stopRequested: function()

isCanceled ?

> Source/WebCore/inspector/front-end/ProgressBar.js:79
> +    requestStop: function()

There should be no programmatic way to stop progress (make it private?)