Currently we update extension audit progress implicitly, based on number of results added and an expected number of results declared when creating the category. It's suggested to make progress indication explicit to the extension code.
Created attachment 164179 [details] Patch
Comment on attachment 164179 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164179&action=review Should this new API have a test? > Source/WebCore/inspector/front-end/AuditsPanel.js:284 > + progress.done(); Thisbshould not be needed
Comment on attachment 164179 [details] Patch r- per pfeldman's comments.
Created attachment 164410 [details] Patch
Comment on attachment 164410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164410&action=review > Source/WebCore/ChangeLog:3 > + Web Inspector: [Extensions API] exlpicitly manage extension audit progress Please fix this line, too > Source/WebCore/ChangeLog:12 > + - retain old magic for computing audit progress if extension specifies extension results count; Remove trailing ';' > Source/WebCore/ChangeLog:34 > + (WebInspector.ProgressIndicator.prototype.done): Assure only first call to done() has effect; Ditto > Source/WebCore/inspector/front-end/AuditsPanel.js:132 > + category.run(requests, ruleResultReadyCallback.bind(this, result), categoryDoneCallback.bind(this), subprogresses[i]); Can you instantiate subprogresses here instead? Or do you need to always know the full amount of work up front? > Source/WebCore/inspector/front-end/ExtensionAPI.js:514 > + console.warn("Passing resultCouint to audits.addCategory() is deprecated. Use AuditResult.updateProgress() instead."); resultCouint -> resultCount
Comment on attachment 164410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164410&action=review >> Source/WebCore/ChangeLog:12 >> + - retain old magic for computing audit progress if extension specifies extension results count; > > Remove trailing ';' replaced with '.' >> Source/WebCore/inspector/front-end/AuditsPanel.js:132 >> + category.run(requests, ruleResultReadyCallback.bind(this, result), categoryDoneCallback.bind(this), subprogresses[i]); > > Can you instantiate subprogresses here instead? Or do you need to always know the full amount of work up front? The reason I'm doing this upfront is that when all children are marked as done, the parent progress indicator is marked as done and hence is removed from view. So this is a guard against an audit category potentially marking its progress as done very early and hence causing the entire progress bar to disappear. I'm moving this down closer to usage, though. >> Source/WebCore/inspector/front-end/ExtensionAPI.js:514 >> + console.warn("Passing resultCouint to audits.addCategory() is deprecated. Use AuditResult.updateProgress() instead."); > > resultCouint -> resultCount fixed
Created attachment 164507 [details] Patch
Comment on attachment 164507 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164507&action=review > Source/WebCore/inspector/front-end/AuditsPanel.js:101 > + var compositeProgress = new WebInspector.CompositeProgress(this._progress); Please move this below, somewhere around var subprogresses = [];
Committed r128875: <http://trac.webkit.org/changeset/128875>
I get an additional message here when running inspector/extensions/extensions-audit-api.html on EFL: CONSOLE MESSAGE: line 480: Passing resultCount to audits.addCategory() is deprecated. Use AuditResult.updateProgress() instead.
(In reply to comment #10) > I get an additional message here when running inspector/extensions/extensions-audit-api.html on EFL: > > CONSOLE MESSAGE: line 480: Passing resultCount to audits.addCategory() is deprecated. Use AuditResult.updateProgress() instead. The console message is there by design, however, the test expectations differ in EFL because other platforms just don't dump console output coming from inspector page to test output (I remember fixing this for chromium in bug 69632). So the long term fix would be to get efl's DRT changed to skip inspector console output as well, and short term would be to add custom expectations or skip the test. It looks like this test is already skipped in LayoutTests/platform/efl/Skipped, I wonder if EFL is using both NRWT and older run-webkit-test that uses Skipped?
(In reply to comment #11) > (In reply to comment #10) > > I get an additional message here when running inspector/extensions/extensions-audit-api.html on EFL: > > > > CONSOLE MESSAGE: line 480: Passing resultCount to audits.addCategory() is deprecated. Use AuditResult.updateProgress() instead. > > The console message is there by design, however, the test expectations differ in EFL because other platforms just don't dump console output coming from inspector page to test output (I remember fixing this for chromium in bug 69632). So the long term fix would be to get efl's DRT changed to skip inspector console output as well Thanks for the information, I will work on that. > It looks like this test is already skipped in LayoutTests/platform/efl/Skipped, I wonder if EFL is using both NRWT and older run-webkit-test that uses Skipped? EFL uses NRWT; Skipped has extensions-api.html and extensions-audits.html, but not extensions-audit-apis.html :-)
(In reply to comment #12) > EFL uses NRWT; Skipped has extensions-api.html and extensions-audits.html, but not extensions-audit-apis.html :-) Ouch, sorry -- I missed it's extensions-audit-apis.html. Then I guess we could as well avoid using that deprecated parameter there -- it's not really required for the test and I guess it would be the must practical fix, as it would save us another Skipped test. The problem of inspector console output, though, remains relevant for some other tests (extensions-audits.html would be one example).