Bug 44837

Summary: Web Inspector: persist DOM breakpoints between page reloads
Product: WebKit Reporter: Pavel Podivilov <podivilov>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, joepeck, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 45209    
Attachments:
Description Flags
Proposed patch.
none
All comments addressed.
yurys: review-, yurys: commit-queue-
Proposed patch.
none
Proposed patch. none

Description Pavel Podivilov 2010-08-29 06:32:50 PDT
Web Inspector: persist DOM breakpoints between page reloads
Comment 1 Pavel Podivilov 2010-08-30 04:03:16 PDT
Created attachment 65902 [details]
Proposed patch.
Comment 2 Yury Semikhatsky 2010-08-30 06:56:26 PDT
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?
Comment 3 Yury Semikhatsky 2010-08-30 06:57:40 PDT
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?
Comment 4 Joseph Pecoraro 2010-08-30 16:47:34 PDT
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!
Comment 5 Pavel Podivilov 2010-08-31 07:19:51 PDT
Created attachment 66047 [details]
All comments addressed.
Comment 6 Pavel Podivilov 2010-08-31 07:22:56 PDT
(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
Comment 7 Yury Semikhatsky 2010-09-01 02:38:26 PDT
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.
Comment 8 Pavel Podivilov 2010-09-02 04:40:00 PDT
Created attachment 66353 [details]
Proposed patch.

Comments addressed. Added test for restoring DOM breakpoints on page reload.
Comment 9 Pavel Podivilov 2010-09-02 04:55:03 PDT
Created attachment 66359 [details]
Proposed patch.

Comments addressed. Added test for restoring DOM breakpoints on page reload.
Comment 10 WebKit Commit Bot 2010-09-02 22:06:09 PDT
Comment on attachment 66359 [details]
Proposed patch.

Clearing flags on attachment: 66359

Committed r66712: <http://trac.webkit.org/changeset/66712>
Comment 11 WebKit Commit Bot 2010-09-02 22:06:14 PDT
All reviewed patches have been landed.  Closing bug.