31212 2009-11-06 12:52:13 -0800 showTree(CounterNode*) generates too little info and has too many spaces. 2009-11-09 11:43:30 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 carol webkit-unassigned commit-queue darin enrica eric justin.garcia laszlo.gombos rniwa oldest_to_newest 161230 0 carol 2009-11-06 12:52:13 -0800 I worked on fixing the issue of DOM update reactivity in webkit's counter implementation. I found the information provided by showTree(CounterNode*) insufficient for my needs and I did not like the usage of tab for indentation as it would create too much whitespace. Here I am proposing an alternative (hopefully better) solution. Patch coming soon. 161249 1 42671 carol 2009-11-06 14:40:57 -0800 Created attachment 42671 Proposed Patch Adds information about the address of the counters in the tree, the renderer that they are attached to. Also allows for the inspection of the integrity of the tree structure. 161266 2 eric 2009-11-06 16:36:11 -0800 It would be helpful if you could show an example of old vs. new behavior. 161289 3 carol 2009-11-06 18:33:23 -0800 New behavior: 0x09432630 reset: 0 0 P:0x00000000 PS:0x00000000 NS:0x00000000 R:0x0942D6EC 0x0942DC60 increment: 1 1 P:0x09432630 PS:0x00000000 NS:0x0942C660 R:0x0942ECDC 0x0942C660 increment: 0 1 P:0x09432630 PS:0x0942DC60 NS:0x09438EE0 R:0x0942EE64 * 0x09438EE0 reset: 0 1 P:0x09432630 PS:0x0942C660 NS:0x00000000 R:0x0942F8E4 0x09439178 increment: 1 1 P:0x09438EE0 PS:0x00000000 NS:0x00000000 R:0x09430164 Where P stands for parent, PS for previous sibling, NS for next sibling R for the renderer that the counter is attached to. The first hex number is the address of the counter dumped. OldBehavior: reset: 0 0 increment: 1 1 increment: 0 1 reset: 0 1 increment: 1 1 The decimal numbers represent the value and countInParent members of the counter. Note: On modern terminals that support more than 100 characters per line the text in the new format would not wrap till 14 levels deep nesting. 161308 4 eric 2009-11-07 00:06:28 -0800 Wow. The new format looks super-confusing! I think we should have other editing folks (those who use these methods regularly for debugging) weigh in here. 161362 5 42671 darin 2009-11-07 11:43:22 -0800 Comment on attachment 42671 Proposed Patch This is great! > + fprintf(stderr, "0x%08X %s: %d %d P:0x%08X PS:0x%08X NS:0x%08X R:0x%08X\n", > + current, current->isReset() ? "reset" : "increment", current->value(), > + current->countInParent(), current->parent(), current->previousSibling(), > + current->nextSibling(), current->renderer()); This won't compile without warnings under gcc. The specifier %08X takes an "unsigned" as its argument, but we are passing in a pointer. The most portable way to do this is probably to use %p instead of 0x%08X. Other solutions involve converting the type by adding a typecast. review- because this will break the build 161363 6 darin 2009-11-07 11:52:25 -0800 (In reply to comment #4) > Wow. The new format looks super-confusing! I think we should have other > editing folks (those who use these methods regularly for debugging) weigh in > here. Counter is not about editing, so I don’t see the relevance of "editing folks". But despite my positive comment in the review, I think that dumping all those pointers is not extremely useful. On 64-bit systems they will be even wider too. But on the other hand, the only person who will do this is someone debugging counters. It still seems OK to change it like this. 161373 7 eric 2009-11-07 13:53:49 -0800 Oh, I guess I took this to be the generic showTreeForNode(Node*). My apologies for my confusion. 161623 8 42762 carol 2009-11-09 09:58:34 -0800 Created attachment 42762 Proposed Patch This patch addresses Darin's comments about pointer versus int in printf. 161636 9 42762 darin 2009-11-09 10:28:10 -0800 Comment on attachment 42762 Proposed Patch Heh, I would have said "counter" instead of "current" -- nouns work better than adjectives for variable names. And the best function for writing one or two characters is probably fputc rather than fwrite. And on reflection I would probably use things like "parent", "prev" and "next" instead of "P", "PS" and "NS". r=me as is though, no need to go over and over this! 161679 10 42762 commit-queue 2009-11-09 11:43:25 -0800 Comment on attachment 42762 Proposed Patch Clearing flags on attachment: 42762 Committed r50673: <http://trac.webkit.org/changeset/50673> 161680 11 commit-queue 2009-11-09 11:43:30 -0800 All reviewed patches have been landed. Closing bug. 42671 2009-11-06 14:40:57 -0800 2009-11-09 09:58:34 -0800 Proposed Patch patchFor31212-1.patch text/plain 2183 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDYwNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMDktMTEtMDYgIENhcm9sIFN6YWJvICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IHNob3dUcmVlKENvdW50ZXJOb2RlKikgZ2VuZXJhdGVzIHRvbyBsaXR0bGUgaW5mbyBhbmQgaGFz IHRvbyBtYW55IHNwYWNlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTMxMjEyCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIGFzIHRoZSBjaGFuZ2UgaGFz IG5vIGZ1bmN0aW9uYWwgZWZmZWN0IGl0IGlzIGp1c3QgZm9yCisgICAgICAgIGltcHJvdmVkIGRl YnVnZ2luZy4KKworICAgICAgICAqIHJlbmRlcmluZy9Db3VudGVyTm9kZS5jcHA6CisgICAgICAg IChXZWJDb3JlOjpzaG93VHJlZUFuZE1hcmspOgorICAgICAgICBDaGFuZ2VkIHRvIGFsc28gc2hv dyBhZGRyZXNzZXMgb2YgcGFyZW50LCBuZXh0IGFuZCBwcmV2aW91cworICAgICAgICBzaWJsaW5n cy4KKwogMjAwOS0xMS0wNiAgRGlyayBTY2h1bHplICA8a3JpdEB3ZWJraXQub3JnPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IE5pa29sYXMgWmltbWVybWFubi4KSW5kZXg6IFdlYkNvcmUvcmVuZGVy aW5nL0NvdW50ZXJOb2RlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3JlbmRlcmluZy9Db3Vu dGVyTm9kZS5jcHAJKHJldmlzaW9uIDUwNTc2KQorKysgV2ViQ29yZS9yZW5kZXJpbmcvQ291bnRl ck5vZGUuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNzEsMTUgKzE3MSwxNCBAQCBzdGF0aWMgdm9p ZCBzaG93VHJlZUFuZE1hcmsoY29uc3QgQ291bnRlCiAgICAgd2hpbGUgKHJvb3QtPnBhcmVudCgp KQogICAgICAgICByb290ID0gcm9vdC0+cGFyZW50KCk7CiAKLSAgICBmb3IgKGNvbnN0IENvdW50 ZXJOb2RlKiBjID0gcm9vdDsgYzsgYyA9IG5leHRJblByZU9yZGVyKGMpKSB7Ci0gICAgICAgIGlm IChjID09IG5vZGUpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgIioiKTsKLSAgICAgICAg Zm9yIChjb25zdCBDb3VudGVyTm9kZSogZCA9IGM7IGQgJiYgZCAhPSByb290OyBkID0gZC0+cGFy ZW50KCkpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgIlx0Iik7Ci0gICAgICAgIGlmIChj LT5pc1Jlc2V0KCkpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgInJlc2V0OiAlZCAlZFxu IiwgYy0+dmFsdWUoKSwgYy0+Y291bnRJblBhcmVudCgpKTsKLSAgICAgICAgZWxzZQotICAgICAg ICAgICAgZnByaW50ZihzdGRlcnIsICJpbmNyZW1lbnQ6ICVkICVkXG4iLCBjLT52YWx1ZSgpLCBj LT5jb3VudEluUGFyZW50KCkpOworICAgIGZvciAoY29uc3QgQ291bnRlck5vZGUqIGN1cnJlbnQg PSByb290OyBjdXJyZW50OyBjdXJyZW50ID0gbmV4dEluUHJlT3JkZXIoY3VycmVudCkpIHsKKyAg ICAgICAgZndyaXRlKChjdXJyZW50ID09IG5vZGUpID8gIioiIDogIiAiLCAxLCAxLCBzdGRlcnIp OworICAgICAgICBmb3IgKGNvbnN0IENvdW50ZXJOb2RlKiBwYXJlbnQgPSBjdXJyZW50OyBwYXJl bnQgJiYgcGFyZW50ICE9IHJvb3Q7IHBhcmVudCA9IHBhcmVudC0+cGFyZW50KCkpCisgICAgICAg ICAgICBmd3JpdGUoIiAgIiwgMSwgMiwgc3RkZXJyKTsKKyAgICAgICAgZnByaW50ZihzdGRlcnIs ICIweCUwOFggJXM6ICVkICVkIFA6MHglMDhYIFBTOjB4JTA4WCBOUzoweCUwOFggUjoweCUwOFhc biIsCisgICAgICAgICAgICBjdXJyZW50LCBjdXJyZW50LT5pc1Jlc2V0KCkgPyAicmVzZXQiIDog ImluY3JlbWVudCIsIGN1cnJlbnQtPnZhbHVlKCksCisgICAgICAgICAgICBjdXJyZW50LT5jb3Vu dEluUGFyZW50KCksIGN1cnJlbnQtPnBhcmVudCgpLCBjdXJyZW50LT5wcmV2aW91c1NpYmxpbmco KSwKKyAgICAgICAgICAgIGN1cnJlbnQtPm5leHRTaWJsaW5nKCksIGN1cnJlbnQtPnJlbmRlcmVy KCkpOwogICAgIH0KIH0KIAo= 42762 2009-11-09 09:58:34 -0800 2009-11-09 11:43:24 -0800 Proposed Patch patchFor31212-2.patch text/plain 2167 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDYwNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMDktMTEtMDYgIENhcm9sIFN6YWJvICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IHNob3dUcmVlKENvdW50ZXJOb2RlKikgZ2VuZXJhdGVzIHRvbyBsaXR0bGUgaW5mbyBhbmQgaGFz IHRvbyBtYW55IHNwYWNlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTMxMjEyCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIGFzIHRoZSBjaGFuZ2UgaGFz IG5vIGZ1bmN0aW9uYWwgZWZmZWN0IGl0IGlzIGp1c3QgZm9yCisgICAgICAgIGltcHJvdmVkIGRl YnVnZ2luZy4KKworICAgICAgICAqIHJlbmRlcmluZy9Db3VudGVyTm9kZS5jcHA6CisgICAgICAg IChXZWJDb3JlOjpzaG93VHJlZUFuZE1hcmspOgorICAgICAgICBDaGFuZ2VkIHRvIGFsc28gc2hv dyBhZGRyZXNzZXMgb2YgcGFyZW50LCBuZXh0IGFuZCBwcmV2aW91cworICAgICAgICBzaWJsaW5n cy4KKwogMjAwOS0xMS0wNiAgRGlyayBTY2h1bHplICA8a3JpdEB3ZWJraXQub3JnPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IE5pa29sYXMgWmltbWVybWFubi4KSW5kZXg6IFdlYkNvcmUvcmVuZGVy aW5nL0NvdW50ZXJOb2RlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3JlbmRlcmluZy9Db3Vu dGVyTm9kZS5jcHAJKHJldmlzaW9uIDUwNTc2KQorKysgV2ViQ29yZS9yZW5kZXJpbmcvQ291bnRl ck5vZGUuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNzEsMTUgKzE3MSwxNCBAQCBzdGF0aWMgdm9p ZCBzaG93VHJlZUFuZE1hcmsoY29uc3QgQ291bnRlCiAgICAgd2hpbGUgKHJvb3QtPnBhcmVudCgp KQogICAgICAgICByb290ID0gcm9vdC0+cGFyZW50KCk7CiAKLSAgICBmb3IgKGNvbnN0IENvdW50 ZXJOb2RlKiBjID0gcm9vdDsgYzsgYyA9IG5leHRJblByZU9yZGVyKGMpKSB7Ci0gICAgICAgIGlm IChjID09IG5vZGUpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgIioiKTsKLSAgICAgICAg Zm9yIChjb25zdCBDb3VudGVyTm9kZSogZCA9IGM7IGQgJiYgZCAhPSByb290OyBkID0gZC0+cGFy ZW50KCkpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgIlx0Iik7Ci0gICAgICAgIGlmIChj LT5pc1Jlc2V0KCkpCi0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgInJlc2V0OiAlZCAlZFxu IiwgYy0+dmFsdWUoKSwgYy0+Y291bnRJblBhcmVudCgpKTsKLSAgICAgICAgZWxzZQotICAgICAg ICAgICAgZnByaW50ZihzdGRlcnIsICJpbmNyZW1lbnQ6ICVkICVkXG4iLCBjLT52YWx1ZSgpLCBj LT5jb3VudEluUGFyZW50KCkpOworICAgIGZvciAoY29uc3QgQ291bnRlck5vZGUqIGN1cnJlbnQg PSByb290OyBjdXJyZW50OyBjdXJyZW50ID0gbmV4dEluUHJlT3JkZXIoY3VycmVudCkpIHsKKyAg ICAgICAgZndyaXRlKChjdXJyZW50ID09IG5vZGUpID8gIioiIDogIiAiLCAxLCAxLCBzdGRlcnIp OworICAgICAgICBmb3IgKGNvbnN0IENvdW50ZXJOb2RlKiBwYXJlbnQgPSBjdXJyZW50OyBwYXJl bnQgJiYgcGFyZW50ICE9IHJvb3Q7IHBhcmVudCA9IHBhcmVudC0+cGFyZW50KCkpCisgICAgICAg ICAgICBmd3JpdGUoIiAgIiwgMSwgMiwgc3RkZXJyKTsKKyAgICAgICAgZnByaW50ZihzdGRlcnIs ICIlcCAlczogJWQgJWQgUDolcCBQUzolcCBOUzolcCBSOiVwXG4iLAorICAgICAgICAgICAgY3Vy cmVudCwgY3VycmVudC0+aXNSZXNldCgpID8gInJlc2V0X19fXyIgOiAiaW5jcmVtZW50IiwgY3Vy cmVudC0+dmFsdWUoKSwKKyAgICAgICAgICAgIGN1cnJlbnQtPmNvdW50SW5QYXJlbnQoKSwgY3Vy cmVudC0+cGFyZW50KCksIGN1cnJlbnQtPnByZXZpb3VzU2libGluZygpLAorICAgICAgICAgICAg Y3VycmVudC0+bmV4dFNpYmxpbmcoKSwgY3VycmVudC0+cmVuZGVyZXIoKSk7CiAgICAgfQogfQog Cg==