Bug 193158

Summary: Web Inspector: Audit: disable breakpoints when running Audit
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 190754    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

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.