Bug 110866 – Web Inspector: Refactor AuditsPanel with AuditController as newly introduced entity

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

[Vivek Galatage]

AuditController is being introduced in order to follow MVC pattern with the Audits. This is the first step about the refactoring. 

Moving the methods from the AuditsPanel into AuditController would ease the event driven approach(to be added later) to de-couple the AuditsPanel and the AuditLauncherView.

Patch follows

[Vivek Galatage]

Created attachment 190253 [details]
Patch

[Alexander Pavlov (apavlov)]

Comment on attachment 190253 [details]
Patch

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

Barring the file patching for the new JS addition, looks good for a start.

> Source/WebCore/ChangeLog:16
> +        * inspector/front-end/AuditController.js: Added.

You forgot to patch WebCore.vcproj and compile-front-end.py for the new file.

> Source/WebCore/inspector/front-end/AuditController.js:91
> +        this._auditsPanel._auditFinishedCallback(mainResourceURL, results);

For the time being, you should remove the leading '_' from "_auditFinishedCallback", since this denotes a file-private member. Hopefully, this will get removed altogether soon.

> Source/WebCore/inspector/front-end/AuditLauncherView.js:160
> +        this._auditController.initiateAudit(catIds, this._progressIndicator, this._auditPresentStateElement.checked, onAuditStarted.bind(this), this._setAuditRunning.bind(this, false));

These callbacks will go away once we migrate to the events.

[Vivek Galatage]

Created attachment 190420 [details]
Patch

[Vivek Galatage]

(In reply to comment #2)
> Barring the file patching for the new JS addition, looks good for a start.
> 

Thank you for the review. :)

> > Source/WebCore/ChangeLog:16
> > +        * inspector/front-end/AuditController.js: Added.
> 
> You forgot to patch WebCore.vcproj and compile-front-end.py for the new file.
> 

Oh sure, forgot these ones. Done.

> > Source/WebCore/inspector/front-end/AuditController.js:91
> > +        this._auditsPanel._auditFinishedCallback(mainResourceURL, results);
> 
> For the time being, you should remove the leading '_' from "_auditFinishedCallback", since this denotes a file-private member. Hopefully, this will get removed altogether soon.
>

Modified the method to remove the leading '_'. Also modified the respective extension test which sniffs for this method.
 
> > Source/WebCore/inspector/front-end/AuditLauncherView.js:160
> > +        this._auditController.initiateAudit(catIds, this._progressIndicator, this._auditPresentStateElement.checked, onAuditStarted.bind(this), this._setAuditRunning.bind(this, false));
> 
> These callbacks will go away once we migrate to the events.

Yes with the events, every callback in controller should disappear.

[Alexander Pavlov (apavlov)]

Comment on attachment 190420 [details]
Patch

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

Once the JSDoc is added, we are good to land.

> Source/WebCore/inspector/front-end/AuditController.js:128
> +    _reloadResources: function(callback)

Please add a JSDoc:

/**
 * @param {function()=} callback
 */

[Vivek Galatage]

Created attachment 190510 [details]
Patch

[WebKit Review Bot]

Comment on attachment 190510 [details]
Patch

Clearing flags on attachment: 190510

Committed r144182: <http://trac.webkit.org/changeset/144182>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.