| Summary: | Move ElementShadow creation to ElementRareData | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Elliott Sprehn <esprehn> | ||||||
| Component: | New Bugs | Assignee: | Elliott Sprehn <esprehn> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarcelo, darin, dglazkov, morrita, ojan.autocc, ojan, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Elliott Sprehn
2013-01-29 07:57:17 PST
Created attachment 185248 [details]
Patch
Comment on attachment 185248 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185248&action=review > Source/WebCore/ChangeLog:13 > + * dom/Element.cpp: It's good WebKit practice to describe changes inline here. Darin Adler once told me: "If you're not putting anything here, might as well delete this text". Comment on attachment 185248 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185248&action=review >> Source/WebCore/ChangeLog:13 >> + * dom/Element.cpp: > > It's good WebKit practice to describe changes inline here. Darin Adler once told me: "If you're not putting anything here, might as well delete this text". Okay. Created attachment 185257 [details]
Patch for landing
Comment on attachment 185257 [details] Patch for landing Clearing flags on attachment: 185257 Committed r141132: <http://trac.webkit.org/changeset/141132> All reviewed patches have been landed. Closing bug. This kind of change is OK, but the name “data” in the names of classes like NodeRareData and ElementRareData is supposed to indicate that those classes hold data but not policy, and generally speaking we want most of the code to be in the Node and Element class, not the NodeRareData and ElementRareData classes. As soon as the code was anything more than ElementShadow::create() I think we’d not want to make this sort of change. (In reply to comment #7) > This kind of change is OK, but the name “data” in the names of classes like NodeRareData and ElementRareData is supposed to indicate that those classes hold data but not policy, and generally speaking we want most of the code to be in the Node and Element class, not the NodeRareData and ElementRareData classes. As soon as the code was anything more than ElementShadow::create() I think we’d not want to make this sort of change. Okay. My change is inline with all the code in NodeRareData though: see ensureNodeLists, ensureMutationObserverData, ensureMicroDataTokenLists etc. (In reply to comment #8) > My change is inline with all the code in NodeRareData though: see ensureNodeLists, ensureMutationObserverData, ensureMicroDataTokenLists etc. Sure. Yours was just the one I happened to spot. I haven’t been watching all the check-ins as much as I used to. |