Summary: | [Shadow DOM][Refactoring] HTMLContentSelector family should have better name | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||||||
Component: | DOM | Assignee: | Hajime Morrita <morrita> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | dglazkov, gustavo, philn, rakuco, webkit.review.bot, xan.lopez | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 85263, 86350 | ||||||||||||||||
Attachments: |
|
Description
Hajime Morrita
2012-05-10 00:43:28 PDT
Created attachment 141329 [details]
WIP
Created attachment 141338 [details]
WIP
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12666417 Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12663603 Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12675125 Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12659011 Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12670225 Created attachment 141348 [details]
WIP
Comment on attachment 141348 [details] WIP Attachment 141348 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12671318 Comment on attachment 141348 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=141348&action=review Some name bikeshedding. > Source/WebCore/dom/ElementShadow.cpp:43 > + : m_needsRecalcDistribution(false) needsRedistributing > Source/WebCore/html/shadow/ShadowDistributor.h:46 > +class ShadowBoundaryDistribution { ContentDistribution > Source/WebCore/html/shadow/ShadowDistributor.h:68 > + RefPtr<Item> m_next; > + RefPtr<Item> m_previous; Shouldn't be RefPtrs, right? > Source/WebCore/html/shadow/ShadowDistributor.h:88 > + RefPtr<Item> m_first; > + RefPtr<Item> m_last; Ditto about RefPtrs. > Source/WebCore/html/shadow/ShadowDistributor.h:91 > +class ShadowDistributor { ContentDistributor Created attachment 141622 [details]
WIP
Created attachment 141623 [details]
Patch
Hi Dimitri, thanks for taking a look! I updated the patch. (In reply to comment #10) > (From update of attachment 141348 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=141348&action=review > > Some name bikeshedding. > > > Source/WebCore/dom/ElementShadow.cpp:43 > > + : m_needsRecalcDistribution(false) > > needsRedistributing Done. > > > Source/WebCore/html/shadow/ShadowDistributor.h:46 > > +class ShadowBoundaryDistribution { > > ContentDistribution Done. > > > Source/WebCore/html/shadow/ShadowDistributor.h:68 > > + RefPtr<Item> m_next; > > + RefPtr<Item> m_previous; > > Shouldn't be RefPtrs, right? Should be. But I'd like to do it in a separate patch to isolate possible revealing of hidden bugs... Actually, I'm thinking to kill this Item class in following patch(-es). > > > Source/WebCore/html/shadow/ShadowDistributor.h:88 > > + RefPtr<Item> m_first; > > + RefPtr<Item> m_last; > > Ditto about RefPtrs. > > > Source/WebCore/html/shadow/ShadowDistributor.h:91 > > +class ShadowDistributor { > > ContentDistributor Done. (In reply to comment #13) > > Shouldn't be RefPtrs, right? > Should be. But I'd like to do it in a separate patch to > isolate possible revealing of hidden bugs... > Actually, I'm thinking to kill this Item class in following patch(-es). Sounds good. We usually add FIXMEs referencing bugs for future work. Comment on attachment 141623 [details]
Patch
not sure why gtk is angry at you.
Thanks for r+ing! (In reply to comment #14) > (In reply to comment #13) > > > > Shouldn't be RefPtrs, right? > > Should be. But I'd like to do it in a separate patch to > > isolate possible revealing of hidden bugs... > > Actually, I'm thinking to kill this Item class in following patch(-es). > > Sounds good. We usually add FIXMEs referencing bugs for future work. True. Will add it before landing. Created attachment 141683 [details]
Patch for landing
Comment on attachment 141683 [details] Patch for landing Rejecting attachment 141683 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/12679475 Comment on attachment 141683 [details] Patch for landing Clearing flags on attachment: 141683 Committed r116974: <http://trac.webkit.org/changeset/116974> All reviewed patches have been landed. Closing bug. |