Summary: | Web Inspector: break on DOM node insertion only once per operation, not once per inserted node | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Klein <adamk> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Adam Klein <adamk> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | loislo, ojan, pfeldman, yurys | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 82631 | ||||||||
Attachments: |
|
Description
Adam Klein
2012-04-02 16:22:12 PDT
Created attachment 135221 [details]
Patch
Looks good to me. Leaving r? so the inspector folks have a chance to r-/r+. (In reply to comment #2) > Looks good to me. Leaving r? so the inspector folks have a chance to r-/r+. Thanks. To inspector folks: I'd especially appreciate suggestions on a test. Can someone from the inspector side take a look at this? It's blocking me for moving forward with the DOM refactoring in bug 82631. Comment on attachment 135221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135221&action=review > Source/WebCore/ChangeLog:23 > + I tried to write a test for this in inspector/debugger/dom-breakpoints.html, Your change looks good. The way I would test it is I would use a snippet like: testNode.innerHTML = "<br><br>"; debugger; And would test that I stop on the innerHTML = line first, then I would click continue and check that I stopped on the next line. (In reply to comment #5) > (From update of attachment 135221 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135221&action=review > > > Source/WebCore/ChangeLog:23 > > + I tried to write a test for this in inspector/debugger/dom-breakpoints.html, > > Your change looks good. The way I would test it is I would use a snippet like: > > testNode.innerHTML = "<br><br>"; > debugger; > > And would test that I stop on the innerHTML = line first, then I would click continue and check that I stopped on the next line. That test manually works fine, but coming up with the right set of incantations around evaluateInPageWithoutTimeout, resumeExecution, and waitUntilPaused is leaving me mystified with test timeouts. Will try in a standalone test... Created attachment 135639 [details]
Patch with test
Comment on attachment 135639 [details]
Patch with test
Yay!
Comment on attachment 135639 [details]
Patch with test
Clearing cq+ while I am figuring out whether the test is actually right.
Comment on attachment 135639 [details] Patch with test View in context: https://bugs.webkit.org/attachment.cgi?id=135639&action=review False alarm, I think it is Ok. Thanks for fixing it, sorry for moving it back to the end of the cq! > LayoutTests/inspector/debugger/dom-breakpoints.html:55 > + debugger; Oh, ok, you need it because of the runAfterPendingDispatches used in waitUntilPausedAndDumpStack. (In reply to comment #10) > (From update of attachment 135639 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135639&action=review > > False alarm, I think it is Ok. Thanks for fixing it, sorry for moving it back to the end of the cq! > > > LayoutTests/inspector/debugger/dom-breakpoints.html:55 > > + debugger; > > Oh, ok, you need it because of the runAfterPendingDispatches used in waitUntilPausedAndDumpStack. Thanks for double-checking, I wasn't sure which thing in that helper was messing me up, but I could tell something was. Committed r113230: <http://trac.webkit.org/changeset/113230> |