Bug 44837 – Web Inspector: persist DOM breakpoints between page reloads

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Pavel Podivilov]

Web Inspector: persist DOM breakpoints between page reloads

[Pavel Podivilov]

Created attachment 65902 [details]
Proposed patch.

[Yury Semikhatsky]

Comment on attachment 65902 [details]
Proposed patch.

> +++ b/LayoutTests/inspector/dom-breakpoints.html
> @@ -35,7 +35,7 @@ var test = function()
>      function step2(node)
>      {
>          InspectorTest.addResult("Found dom node d0.");
> -        WebInspector.domBreakpointManager.setBreakpoint(node, WebInspector.DOMBreakpoint.Types.SubtreeModified);
> +        node.setBreakpoint("SubtreeModified");
Use constants for such arguments.


> +++ b/WebCore/inspector/front-end/DOMAgent.js
> @@ -143,6 +143,40 @@ WebInspector.DOMNode.prototype = {
>          this.ownerDocument._domAgent.removeAttributeAsync(this, name, callback);
>      },
>  
> +    path: function()
> +    {
> +        var path = [];
> +        var node = this;
> +        while (node && "index" in node && node.nodeName && node.nodeName.length) {
Is it possible that there is node.index but but no node.nodeName?

[Yury Semikhatsky]

If we restore breakpoints from the front-end we may miss some early events, is it intentional or should we move the logic to the backend?

[Joseph Pecoraro]

Pavel, please use the http://webkit.org/new-inspector-bug link, or the "Inspector"
link from within Bugzilla <https://bugs.webkit.org/enter_bug.cgi>. I would like to
get CC'd on your bugs. That automatically applies the proper template and a CC
list of interested people (including reviewers to get extra eyes on patches). Thanks!

[Pavel Podivilov]

Created attachment 66047 [details]
All comments addressed.

[Pavel Podivilov]

(In reply to comment #4)
> Pavel, please use the http://webkit.org/new-inspector-bug link, or the "Inspector"
> link from within Bugzilla <https://bugs.webkit.org/enter_bug.cgi>. I would like to
> get CC'd on your bugs. That automatically applies the proper template and a CC
> list of interested people (including reviewers to get extra eyes on patches). Thanks!

ok

[Yury Semikhatsky]

Comment on attachment 66047 [details]
All comments addressed.

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

> WebCore/inspector/front-end/DOMAgent.js:804
> +        WebInspector.DOMBreakpoint._ids = {};
Why do we need both integer and string type value? We should either use strings or intergers everywhere, r- for this. Given that the backend uses integers and we have named constants in the frontend it seems like we may well get rid of string values.

> WebCore/inspector/front-end/DOMAgent.js:842
> +        WebInspector.DOMBreakpoint._contextMenuLabels[WebInspector.DOMBreakpoint.Types.NodeRemoved] = WebInspector.UIString("Break on Node Removal");        
Revert this please.

[Pavel Podivilov]

Created attachment 66353 [details]
Proposed patch.

Comments addressed. Added test for restoring DOM breakpoints on page reload.

[Pavel Podivilov]

Created attachment 66359 [details]
Proposed patch.

Comments addressed. Added test for restoring DOM breakpoints on page reload.

[WebKit Commit Bot]

Comment on attachment 66359 [details]
Proposed patch.

Clearing flags on attachment: 66359

Committed r66712: <http://trac.webkit.org/changeset/66712>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.