Bug 170033 – Web Inspector: XHR breakpoints should be global

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Created attachment 305238 [details]
[Image] new tree element titles

Summary:
XHR breakpoints should be global. They are currently associated with the main frame URL.

Note:
Tree element titles for XHR breakpoints should "URL — <url text>", instead of "URL contains: <url text>". See screenshot.

[Matt Baker]

Created attachment 305297 [details]
Patch

[Build Bot]

Comment on attachment 305297 [details]
Patch

Attachment 305297 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3404861

New failing tests:
inspector/debugger/tail-deleted-frames.html
inspector/debugger/probe-manager-add-remove-actions.html
inspector/debugger/tail-recursion.html
inspector/debugger/breakpoint-columns.html
inspector/debugger/tail-deleted-frames-this-value.html
http/tests/inspector/network/resource-response-source-memory-cache.html
inspector/debugger/tail-deleted-frames-from-vm-entry.html
inspector/dom-debugger/xhr-breakpoints.html
inspector/debugger/search-scripts.html
inspector/debugger/breakpoint-scope.html
http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html
inspector/console/messagesCleared.html
inspector/debugger/debugger-stack-overflow.html

[Build Bot]

Created attachment 305307 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Comment on attachment 305297 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
> +        if (this._xhrBreakpoints.some((entry) => entry.url === breakpoint.url))
> +            return;

How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:363
> +            breakpoint.dispatchEventToListeners(WebInspector.XHRBreakpoint.Event.XHRBreakpointResolved);

My checkout doesn't have this event name. What is this? r- for this

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css:112
>  .sidebar > .panel.navigation.debugger .details-section.xhr-breakpoints .item.breakpoint .subtitle {
> -    padding-left: 5px;
>      font-family: Menlo, monospace;

Food for thought: If we supported regular expressions (which I think we should because it would be a very small patch) we could use a FormattedValue here. We could show:

    URL – "foo"
    URL – /foo\d/

or:

    URL contains: "foo"
    URL matches: /foo\d/

And they would be syntax highlighted like they are in the console. What you have now looks good to me, far more compact.

[Matt Baker]

Comment on attachment 305297 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
>> +            return;
> 
> How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.

The ".some(() => ...)" test prevents this from happening. No message is shown to the user when attempting to add an XHR breakpoint with a duplicate.

>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:363
>> +            breakpoint.dispatchEventToListeners(WebInspector.XHRBreakpoint.Event.XHRBreakpointResolved);
> 
> My checkout doesn't have this event name. What is this? r- for this

This is a typo. Fixed.

>> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css:112
>>      font-family: Menlo, monospace;
> 
> Food for thought: If we supported regular expressions (which I think we should because it would be a very small patch) we could use a FormattedValue here. We could show:
> 
>     URL – "foo"
>     URL – /foo\d/
> 
> or:
> 
>     URL contains: "foo"
>     URL matches: /foo\d/
> 
> And they would be syntax highlighted like they are in the console. What you have now looks good to me, far more compact.

This is exactly what I'd like to do! We should also use a CodeMirror instance in InputPopover to take advantage of CodeMirror's regex parsing/highlighting. Filed a follow-up:

Web Inspector: Add regular expression support to XHR breakpoints
https://bugs.webkit.org/show_bug.cgi?id=170099

[Matt Baker]

Created attachment 305378 [details]
Patch

[Build Bot]

Comment on attachment 305378 [details]
Patch

Attachment 305378 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3409890

New failing tests:
inspector/debugger/tail-deleted-frames.html
inspector/debugger/probe-manager-add-remove-actions.html
inspector/debugger/tail-recursion.html
inspector/debugger/breakpoint-columns.html
inspector/debugger/tail-deleted-frames-this-value.html
http/tests/inspector/network/resource-response-source-memory-cache.html
inspector/debugger/tail-deleted-frames-from-vm-entry.html
inspector/dom-debugger/xhr-breakpoints.html
inspector/debugger/search-scripts.html
inspector/debugger/breakpoint-scope.html
http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html
inspector/console/messagesCleared.html
inspector/debugger/debugger-stack-overflow.html

[Build Bot]

Created attachment 305383 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305378 [details]
Patch

Attachment 305378 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3409900

New failing tests:
inspector/debugger/tail-deleted-frames.html
inspector/debugger/probe-manager-add-remove-actions.html
inspector/debugger/tail-recursion.html
inspector/debugger/breakpoint-columns.html
inspector/debugger/tail-deleted-frames-this-value.html
http/tests/inspector/network/resource-response-source-memory-cache.html
inspector/debugger/tail-deleted-frames-from-vm-entry.html
inspector/dom-debugger/xhr-breakpoints.html
inspector/debugger/search-scripts.html
inspector/debugger/breakpoint-scope.html
http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html
inspector/console/messagesCleared.html
inspector/debugger/debugger-stack-overflow.html

[Build Bot]

Created attachment 305384 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305378 [details]
Patch

Attachment 305378 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3409894

New failing tests:
inspector/debugger/tail-deleted-frames-from-vm-entry.html
inspector/debugger/probe-manager-add-remove-actions.html
inspector/debugger/tail-recursion.html
inspector/debugger/tail-deleted-frames-this-value.html
http/tests/inspector/network/resource-response-source-memory-cache.html
inspector/debugger/tail-deleted-frames.html
inspector/dom-debugger/xhr-breakpoints.html
http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html

[Build Bot]

Created attachment 305385 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Matt Baker]

Created attachment 305386 [details]
Patch

[Build Bot]

Comment on attachment 305386 [details]
Patch

Attachment 305386 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3410131

New failing tests:
inspector/dom-debugger/xhr-breakpoints.html

[Build Bot]

Created attachment 305390 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305386 [details]
Patch

Attachment 305386 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3410135

New failing tests:
inspector/dom-debugger/xhr-breakpoints.html

[Build Bot]

Created attachment 305391 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305386 [details]
Patch

Attachment 305386 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3410132

New failing tests:
inspector/dom-debugger/xhr-breakpoints.html

[Build Bot]

Created attachment 305392 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Matt Baker]

Created attachment 305396 [details]
Patch

[Build Bot]

Comment on attachment 305396 [details]
Patch

Attachment 305396 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3411157

New failing tests:
compositing/absolute-inside-out-of-view-fixed.html

[Build Bot]

Created attachment 305413 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Matt Baker]

Created attachment 305420 [details]
Patch

[Build Bot]

Comment on attachment 305420 [details]
Patch

Attachment 305420 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3417159

New failing tests:
compositing/absolute-inside-out-of-view-fixed.html

[Build Bot]

Created attachment 305464 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Comment on attachment 305297 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
>>> +            return;
>> 
>> How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.
> 
> The ".some(() => ...)" test prevents this from happening. No message is shown to the user when attempting to add an XHR breakpoint with a duplicate.

Oh, so the user attempts to add a breakpoint but no new breakpoint shows up because one already exists with the same URL. Okay, that seems fine.

[Joseph Pecoraro]

Comment on attachment 305420 [details]
Patch

r=me

[Matt Baker]

Created attachment 305883 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 305883 [details]
Patch for landing

Clearing flags on attachment: 305883

Committed r214956: <http://trac.webkit.org/changeset/214956>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.