189643 2018-09-14 18:48:45 -0700 Re-order Node flags based on semantics 2018-09-17 14:08:20 -0700 1 1 1 Unclassified WebKit DOM WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 rniwa rniwa cdumez dbates esprehn+autocc ews-watchlist kangil.han koivisto simon.fraser webkit-bug-importer oldest_to_newest 1460008 0 rniwa 2018-09-14 18:48:45 -0700 Re-order Node flags so that flags used for similar purposes are grouped together. 1460009 1 349845 rniwa 2018-09-14 18:52:13 -0700 Created attachment 349845 Cleanup 1460010 2 349845 simon.fraser 2018-09-14 19:01:16 -0700 Comment on attachment 349845 Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=349845&action=review > Source/WebCore/dom/Node.h:566 > - IsConnectedFlag = 1 << 6, > - IsInShadowTreeFlag = 1 << 7, > - IsDocumentNodeFlag = 1 << 8, > - IsShadowRootFlag = 1 << 9, > + IsDocumentNodeFlag = 1 << 6, > + IsShadowRootFlag = 1 << 7, > + IsConnectedFlag = 1 << 8, > + IsInShadowTreeFlag = 1 << 9, > HasRareDataFlag = 1 << 10, I would prefer all the "= 1 << N" be aligned, so it's easy to see that they are increasing by one, but not everyone agrees. > Source/WebCore/dom/Node.h:584 > + StyleValidityShift = 22, Does this break our ability to use this enum with OptionSet<>? 1460310 3 rniwa 2018-09-17 13:47:15 -0700 (In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 349845 [details] > Cleanup > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349845&action=review > > > Source/WebCore/dom/Node.h:566 > > - IsConnectedFlag = 1 << 6, > > - IsInShadowTreeFlag = 1 << 7, > > - IsDocumentNodeFlag = 1 << 8, > > - IsShadowRootFlag = 1 << 9, > > + IsDocumentNodeFlag = 1 << 6, > > + IsShadowRootFlag = 1 << 7, > > + IsConnectedFlag = 1 << 8, > > + IsInShadowTreeFlag = 1 << 9, > > HasRareDataFlag = 1 << 10, > > I would prefer all the "= 1 << N" be aligned, so it's easy to see that they > are increasing by one, but not everyone agrees. Well, if we did that, then we'd have to re-align them whenever a flag with a longer name than the current longest one is added. > > Source/WebCore/dom/Node.h:584 > > + StyleValidityShift = 22, > > Does this break our ability to use this enum with OptionSet<>? Yes. But we can probably just add two vales of StyleValidity directly to NodeFlags and do the manual conversion instead. 1460320 4 rniwa 2018-09-17 14:07:10 -0700 Committed r236083: <https://trac.webkit.org/changeset/236083> 1460321 5 webkit-bug-importer 2018-09-17 14:08:20 -0700 <rdar://problem/44533292> 349845 2018-09-14 18:52:13 -0700 2018-09-14 19:01:16 -0700 Cleanup bug-189643-20180914185212.patch text/plain 3405 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIzNjAzMCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE4LTA5LTE0ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIFJlLW9yZGVyIE5vZGUgZmxhZ3Mg YmFzZWQgb24gc2VtYW50aWNzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xODk2NDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBSZS1vcmRlcmVkIE5vZGUgZmxhZ3MgYmFzZWQgb24gdGhlaXIgY2F0ZWdvcnkg YW5kIHN1YmNsYXNzZXMgb2YgTm9kZSB3aGljaCB1c2UgdGhlbS4KKworICAgICAgICAqIGRvbS9O b2RlLmg6CisKIDIwMTgtMDktMTQgIEJhc3VrZSBTdXp1a2kgIDxCYXN1a2UuU3V6dWtpQHNvbnku Y29tPgogCiAgICAgICAgIFtDdXJsXSBCdWcgZml4IG9uIHNvbWUgaW5hY2N1cmF0ZSB2YWx1ZXMg aW4gTmV0d29ya0xvYWRNZXRyaWNzLgpJbmRleDogU291cmNlL1dlYkNvcmUvZG9tL05vZGUuaAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9kb20vTm9kZS5oCShyZXZpc2lvbiAyMzYwMjcp CisrKyBTb3VyY2UvV2ViQ29yZS9kb20vTm9kZS5oCSh3b3JraW5nIGNvcHkpCkBAIC01NTksMzkg KzU1OSw0MSBAQCBwcm90ZWN0ZWQ6CiAgICAgICAgIElzSFRNTEZsYWcgPSAxIDw8IDMsCiAgICAg ICAgIElzU1ZHRmxhZyA9IDEgPDwgNCwKICAgICAgICAgSXNNYXRoTUxGbGFnID0gMSA8PCA1LAot ICAgICAgICBJc0Nvbm5lY3RlZEZsYWcgPSAxIDw8IDYsCi0gICAgICAgIElzSW5TaGFkb3dUcmVl RmxhZyA9IDEgPDwgNywKLSAgICAgICAgSXNEb2N1bWVudE5vZGVGbGFnID0gMSA8PCA4LAotICAg ICAgICBJc1NoYWRvd1Jvb3RGbGFnID0gMSA8PCA5LAorICAgICAgICBJc0RvY3VtZW50Tm9kZUZs YWcgPSAxIDw8IDYsCisgICAgICAgIElzU2hhZG93Um9vdEZsYWcgPSAxIDw8IDcsCisgICAgICAg IElzQ29ubmVjdGVkRmxhZyA9IDEgPDwgOCwKKyAgICAgICAgSXNJblNoYWRvd1RyZWVGbGFnID0g MSA8PCA5LAogICAgICAgICBIYXNSYXJlRGF0YUZsYWcgPSAxIDw8IDEwLAogICAgICAgICBIYXNF dmVudFRhcmdldERhdGFGbGFnID0gMSA8PCAxMSwKIAogICAgICAgICAvLyBUaGVzZSBiaXRzIGFy ZSB1c2VkIGJ5IGRlcml2ZWQgY2xhc3NlcywgcHVsbGVkIHVwIGhlcmUgc28gdGhleSBjYW4KICAg ICAgICAgLy8gYmUgc3RvcmVkIGluIHRoZSBzYW1lIG1lbW9yeSB3b3JkIGFzIHRoZSBOb2RlIGJp dHMgYWJvdmUuCiAgICAgICAgIENoaWxkTmVlZHNTdHlsZVJlY2FsY0ZsYWcgPSAxIDw8IDEyLCAv LyBDb250YWluZXJOb2RlCisgICAgICAgIERpcmVjdENoaWxkTmVlZHNTdHlsZVJlY2FsY0ZsYWcg PSAxIDw8IDEzLAogCi0gICAgICAgIElzUGFyc2luZ0NoaWxkcmVuRmluaXNoZWRGbGFnID0gMSA8 PCAxMywgLy8gRWxlbWVudAotICAgICAgICBTdHlsZVZhbGlkaXR5U2hpZnQgPSAxNCwKKyAgICAg ICAgSXNFZGl0aW5nVGV4dE9yVW5kZWZpbmVkQ3VzdG9tRWxlbWVudEZsYWcgPSAxIDw8IDE0LCAv LyBUZXh0IGFuZCBFbGVtZW50CisgICAgICAgIElzQ3VzdG9tRWxlbWVudCA9IDEgPDwgMTUsIC8v IEVsZW1lbnQKKyAgICAgICAgSGFzRm9jdXNXaXRoaW4gPSAxIDw8IDE2LAorICAgICAgICBJc0xp bmtGbGFnID0gMSA8PCAxNywKKyAgICAgICAgSXNVc2VyQWN0aW9uRWxlbWVudCA9IDEgPDwgMTgs CisgICAgICAgIElzUGFyc2luZ0NoaWxkcmVuRmluaXNoZWRGbGFnID0gMSA8PCAxOSwKKyAgICAg ICAgSGFzU3ludGhldGljQXR0ckNoaWxkTm9kZXNGbGFnID0gMSA8PCAyMCwKKyAgICAgICAgU2Vs Zk9yQW5jZXN0b3JIYXNEaXJBdXRvRmxhZyA9IDEgPDwgMjEsCisKKyAgICAgICAgLy8gVGhlIGZv bGxvd2luZyBmbGFncyBhcmUgdXNlZCBpbiBzdHlsZSBpbnZhbGlkYXRpb24uCisgICAgICAgIFN0 eWxlVmFsaWRpdHlTaGlmdCA9IDIyLAogICAgICAgICBTdHlsZVZhbGlkaXR5TWFzayA9IDMgPDwg U3R5bGVWYWxpZGl0eVNoaWZ0LAotICAgICAgICBTdHlsZVJlc29sdXRpb25TaG91bGRSZWNvbXBv c2l0ZUxheWVyRmxhZyA9IDEgPDwgMTYsCi0gICAgICAgIElzRWRpdGluZ1RleHRPclVuZGVmaW5l ZEN1c3RvbUVsZW1lbnRGbGFnID0gMSA8PCAxNywKLSAgICAgICAgSGFzRm9jdXNXaXRoaW4gPSAx IDw8IDE4LAotICAgICAgICBIYXNTeW50aGV0aWNBdHRyQ2hpbGROb2Rlc0ZsYWcgPSAxIDw8IDE5 LAotICAgICAgICBIYXNDdXN0b21TdHlsZVJlc29sdmVDYWxsYmFja3NGbGFnID0gMSA8PCAyMCwK LSAgICAgICAgRGVzY2VuZGFudHNBZmZlY3RlZEJ5UHJldmlvdXNTaWJsaW5nRmxhZyA9IDEgPDwg MjEsCi0gICAgICAgIElzQ3VzdG9tRWxlbWVudCA9IDEgPDwgMjIsCi0gICAgICAgIElzTGlua0Zs YWcgPSAxIDw8IDIzLAotICAgICAgICBJc1VzZXJBY3Rpb25FbGVtZW50ID0gMSA8PCAyNCwKKyAg ICAgICAgU3R5bGVSZXNvbHV0aW9uU2hvdWxkUmVjb21wb3NpdGVMYXllckZsYWcgPSAxIDw8IDI0 LAogCiAgICAgICAgIENoaWxkcmVuQWZmZWN0ZWRCeUZpcnN0Q2hpbGRSdWxlc0ZsYWcgPSAxIDw8 IDI1LAogICAgICAgICBDaGlsZHJlbkFmZmVjdGVkQnlMYXN0Q2hpbGRSdWxlc0ZsYWcgPSAxIDw8 IDI2LAogICAgICAgICBDaGlsZHJlbkFmZmVjdGVkQnlIb3ZlclJ1bGVzRmxhZyA9IDEgPDwgMjcs CiAKLSAgICAgICAgRGlyZWN0Q2hpbGROZWVkc1N0eWxlUmVjYWxjRmxhZyA9IDEgPDwgMjgsCi0g ICAgICAgIEFmZmVjdHNOZXh0U2libGluZ0VsZW1lbnRTdHlsZSA9IDEgPDwgMjksCi0gICAgICAg IFN0eWxlSXNBZmZlY3RlZEJ5UHJldmlvdXNTaWJsaW5nID0gMSA8PCAzMCwKKyAgICAgICAgQWZm ZWN0c05leHRTaWJsaW5nRWxlbWVudFN0eWxlID0gMSA8PCAyOCwKKyAgICAgICAgU3R5bGVJc0Fm ZmVjdGVkQnlQcmV2aW91c1NpYmxpbmcgPSAxIDw8IDI5LAorICAgICAgICBEZXNjZW5kYW50c0Fm ZmVjdGVkQnlQcmV2aW91c1NpYmxpbmdGbGFnID0gMSA8PCAzMCwKIAotICAgICAgICBTZWxmT3JB bmNlc3Rvckhhc0RpckF1dG9GbGFnID0gMSA8PCAzMSwKKyAgICAgICAgSGFzQ3VzdG9tU3R5bGVS ZXNvbHZlQ2FsbGJhY2tzRmxhZyA9IDEgPDwgMzEsCiAKICAgICAgICAgRGVmYXVsdE5vZGVGbGFn cyA9IElzUGFyc2luZ0NoaWxkcmVuRmluaXNoZWRGbGFnCiAgICAgfTsK