Bug 170916

Summary: Web Inspector: Enabled state of "All Requests" XHR breakpoint not restored correctly
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Description Matt Baker 2017-04-17 14:26:58 PDT
Summary:
Enabled state of "All Requests" XHR breakpoint not restored correctly. After creating the global breakpoint from the setting value, it should be set on the backend if enabled.

Right now the breakpoint has to be unchecked/checked for the setting to stick.

Steps to Reproduce:
1. Open a page using XHRs
2. Enable the All Requests breakpoint
3. Initiate an XHR
  => Pause
4. Reload Inspector
5. Initiate an XHR
  => No pause
Comment 1 Matt Baker 2017-04-17 14:28:11 PDT
Created attachment 307299 [details]
Patch
Comment 2 Joseph Pecoraro 2017-04-17 14:30:17 PDT
Comment on attachment 307299 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:71
> +            if (!this._allRequestsBreakpoint.disabled)
> +                this._updateXHRBreakpoint(this._allRequestsBreakpoint);

r- I do not think this makes sense in DOMDebuggerManager.

Workers can have "XHR"Breakpoints but they cannot have "DOM"Breakpoints. So I'd expect this in a location that would be compatible with workers.
Comment 3 Joseph Pecoraro 2017-04-17 14:32:14 PDT
Comment on attachment 307299 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:71
>> +                this._updateXHRBreakpoint(this._allRequestsBreakpoint);
> 
> r- I do not think this makes sense in DOMDebuggerManager.
> 
> Workers can have "XHR"Breakpoints but they cannot have "DOM"Breakpoints. So I'd expect this in a location that would be compatible with workers.

Ahh, okay "DOMDebugger" is what holds all of the Web specific breakpoints (DOM, XHR, eventually Event, Timeouts, etc). I was mistaken.

r=me
Comment 4 Radar WebKit Bug Importer 2017-04-17 14:35:24 PDT
<rdar://problem/31666196>
Comment 5 WebKit Commit Bot 2017-04-17 15:15:47 PDT
Comment on attachment 307299 [details]
Patch

Clearing flags on attachment: 307299

Committed r215435: <http://trac.webkit.org/changeset/215435>
Comment 6 WebKit Commit Bot 2017-04-17 15:15:49 PDT
All reviewed patches have been landed.  Closing bug.