Bug 44837 - Web Inspector: persist DOM breakpoints between page reloads
Summary: Web Inspector: persist DOM breakpoints between page reloads
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 45209
  Show dependency treegraph
 
Reported: 2010-08-29 06:32 PDT by Pavel Podivilov
Modified: 2010-09-03 15:57 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch. (19.11 KB, patch)
2010-08-30 04:03 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
All comments addressed. (19.69 KB, patch)
2010-08-31 07:19 PDT, Pavel Podivilov
yurys: review-
yurys: commit-queue-
Details | Formatted Diff | Diff
Proposed patch. (20.46 KB, patch)
2010-09-02 04:40 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (20.38 KB, patch)
2010-09-02 04:55 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.