Bug 96755 – Web Inspector: reuse WebInspector.ProgressIndicator in Audits panel

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Kosyakov]

Audits panel currently has a custom implementation of progress indciator. Let's replaces it with generic one already used in other places of inspector.

[Andrey Kosyakov]

Created attachment 164108 [details]
Patch

[Alexander Pavlov (apavlov)]

Comment on attachment 164108 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=164108&action=review

> Source/WebCore/ChangeLog:8
> +        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).

I don't believe this line should be here.

> Source/WebCore/ChangeLog:10
> +        - remove AuditProgressMonitor, use WebInspector.ProgressIndicator instead

Please use item delimiters consistently (semicolons, commas, periods, whatever).

> Source/WebCore/ChangeLog:12
> +        - simplify control flow -- remove callbacks where possible;

and this one should end in a period, it seems

>              this._setAuditRunning(true);

tip: you could try extracting this along with its counterpart as this._setAuditRunning(!this._auditRunning);

> Source/WebCore/inspector/front-end/AuditRules.js:77
> +     * @param {WebInspector.NetworkRequest} requests

Can you explain this change? The new annotations should not compile. Same thing for the other rules' doRun().

[Andrey Kosyakov]

(In reply to comment #2)

> > +        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).
> 
> I don't believe this line should be here.

Oops, thanks, fixed!

> Please use item delimiters consistently (semicolons, commas, periods, whatever).

fixed.

> 
> > Source/WebCore/ChangeLog:12
> > +        - simplify control flow -- remove callbacks where possible;
> 
> and this one should end in a period, it seems

ditto.

> 
> >              this._setAuditRunning(true);
> 
> tip: you could try extracting this along with its counterpart as this._setAuditRunning(!this._auditRunning);

I've split it into _startAudit()/_stopAudit()

> > Source/WebCore/inspector/front-end/AuditRules.js:77
> > +     * @param {WebInspector.NetworkRequest} requests
> 
> Can you explain this change? The new annotations should not compile. Same thing for the other rules' doRun().

Fixed this one be an array. Unfortunately, this did compile :(

[Andrey Kosyakov]

Created attachment 164119 [details]
Patch

[Alexander Pavlov (apavlov)]

Comment on attachment 164119 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=164119&action=review

> Source/WebCore/inspector/front-end/AuditsPanel.js:180
> +            function onReload()

You could move it a bit above and call from the "if (runImmediately)" branch (changing the name appropriately) to reduce code duplication.

[Andrey Kosyakov]

Committed r128598: <http://trac.webkit.org/changeset/128598>