Bug 193158 - Web Inspector: Audit: disable breakpoints when running Audit
Summary: Web Inspector: Audit: disable breakpoints when running Audit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: WebInspectorAuditTab
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-04 14:12 PST by Devin Rousso
Modified: 2019-01-04 16:29 PST (History)
5 users (show)

See Also:


Attachments
Patch (10.53 KB, patch)
2019-01-04 14:19 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (18.46 KB, patch)
2019-01-04 16:12 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-01-04 14:12:52 PST
.
Comment 1 Radar WebKit Bug Importer 2019-01-04 14:13:27 PST
<rdar://problem/47057083>
Comment 2 Devin Rousso 2019-01-04 14:19:03 PST
Created attachment 358367 [details]
Patch
Comment 3 Joseph Pecoraro 2019-01-04 14:38:54 PST
Comment on attachment 358367 [details]
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:3
> +        Web Inspector: Audit: disable breakpoints when running Audit

Should we include a test that a `debugger` statement doesn't cause a pause when the audit runs?

> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:1162
> +    _handleAuditManagerTestScheduled(event)
> +    {
> +        this._startDisablingBreakpointsTemporarily();
> +
> +        if (this.paused)
> +            this.resume();
> +    }
> +
> +    _handleAuditManagerTestCompleted(event)
> +    {
> +        this._stopDisablingBreakpointsTemporarily();
> +    }
> +

We may need start/stop to be counters / enums. Someone could technically attempt to start/stop a Timeline profile during audits and then the debugger would be enabled during any remaining audits.
Comment 4 Devin Rousso 2019-01-04 15:16:22 PST
Comment on attachment 358367 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:3
>> +        Web Inspector: Audit: disable breakpoints when running Audit
> 
> Should we include a test that a `debugger` statement doesn't cause a pause when the audit runs?

Ooooooo, I like it!

>> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:1162
>> +
> 
> We may need start/stop to be counters / enums. Someone could technically attempt to start/stop a Timeline profile during audits and then the debugger would be enabled during any remaining audits.

Another thing I noticed is that we dispatch `WI.AuditManager.Event.TestCompleted` whenever a test result is imported as well, meaning that if we started a Timeline recording and imported an audit result, we'd re-enable breakpoints.  That would need to change as well.
Comment 5 Devin Rousso 2019-01-04 16:12:25 PST
Created attachment 358391 [details]
Patch
Comment 6 WebKit Commit Bot 2019-01-04 16:29:06 PST
Comment on attachment 358391 [details]
Patch

Clearing flags on attachment: 358391

Committed r239646: <https://trac.webkit.org/changeset/239646>
Comment 7 WebKit Commit Bot 2019-01-04 16:29:08 PST
All reviewed patches have been landed.  Closing bug.