Bug 86064

Summary: [Shadow DOM][Refactoring] HTMLContentSelector family should have better name
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: 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 Flags
WIP
none
WIP
none
WIP
none
WIP
none
Patch
none
Patch for landing none

Hajime Morrita
Reported 2012-05-10 00:43:28 PDT
The name was invented before Shadow DOM spec was formally written. There is now a concept named "distribution". So that implementation should follow such terminology.
Attachments
WIP (52.67 KB, patch)
2012-05-10 22:27 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
WIP (52.74 KB, patch)
2012-05-11 00:03 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
WIP (52.71 KB, patch)
2012-05-11 01:13 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
WIP (52.50 KB, patch)
2012-05-13 17:55 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (57.66 KB, patch)
2012-05-13 18:16 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch for landing (57.33 KB, patch)
2012-05-14 03:56 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-05-10 22:27:58 PDT
Created attachment 141329 [details] WIP
Hajime Morrita
Comment 2 2012-05-11 00:03:50 PDT
Created attachment 141338 [details] WIP
Build Bot
Comment 3 2012-05-11 00:10:01 PDT
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12666417
Early Warning System Bot
Comment 4 2012-05-11 00:13:04 PDT
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12663603
Gyuyoung Kim
Comment 5 2012-05-11 00:13:19 PDT
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12675125
Early Warning System Bot
Comment 6 2012-05-11 00:14:47 PDT
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12659011
Gustavo Noronha (kov)
Comment 7 2012-05-11 01:08:10 PDT
Comment on attachment 141338 [details] WIP Attachment 141338 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12670225
Hajime Morrita
Comment 8 2012-05-11 01:13:24 PDT
Created attachment 141348 [details] WIP
Gustavo Noronha (kov)
Comment 9 2012-05-11 04:51:48 PDT
Comment on attachment 141348 [details] WIP Attachment 141348 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12671318
Dimitri Glazkov (Google)
Comment 10 2012-05-11 10:51:53 PDT
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
Hajime Morrita
Comment 11 2012-05-13 17:55:03 PDT
Created attachment 141622 [details] WIP
Hajime Morrita
Comment 12 2012-05-13 18:16:04 PDT
Created attachment 141623 [details] Patch
Hajime Morrita
Comment 13 2012-05-13 18:18:53 PDT
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.
Dimitri Glazkov (Google)
Comment 14 2012-05-13 19:40:03 PDT
(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.
Dimitri Glazkov (Google)
Comment 15 2012-05-13 19:41:29 PDT
Comment on attachment 141623 [details] Patch not sure why gtk is angry at you.
Hajime Morrita
Comment 16 2012-05-13 23:54:59 PDT
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.
Hajime Morrita
Comment 17 2012-05-14 03:56:41 PDT
Created attachment 141683 [details] Patch for landing
WebKit Review Bot
Comment 18 2012-05-14 03:58:26 PDT
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
WebKit Review Bot
Comment 19 2012-05-14 11:53:42 PDT
Comment on attachment 141683 [details] Patch for landing Clearing flags on attachment: 141683 Committed r116974: <http://trac.webkit.org/changeset/116974>
WebKit Review Bot
Comment 20 2012-05-14 11:53:49 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.