RESOLVED FIXED 149241
Add HTMLSlotElement, Element.slot, and NonDocumentTypeChildNode.assignedSlot
https://bugs.webkit.org/show_bug.cgi?id=149241
Summary Add HTMLSlotElement, Element.slot, and NonDocumentTypeChildNode.assignedSlot
Ryosuke Niwa
Reported 2015-09-16 18:03:29 PDT
Add the support for slotting.
Attachments
EWS test (167.38 KB, patch)
2015-09-16 20:38 PDT, Ryosuke Niwa
no flags
EFL fix attempt (167.46 KB, patch)
2015-09-16 20:55 PDT, Ryosuke Niwa
no flags
EFL fix attempt 2 (167.51 KB, patch)
2015-09-16 21:22 PDT, Ryosuke Niwa
no flags
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (807.36 KB, application/zip)
2015-09-16 22:14 PDT, Build Bot
no flags
Archive of layout-test-results from ews101 for mac-mavericks (737.69 KB, application/zip)
2015-09-16 23:34 PDT, Build Bot
no flags
Ready for review (173.53 KB, patch)
2015-09-17 01:43 PDT, Ryosuke Niwa
no flags
Reverted changes to WebKit.xcworkspace (167.53 KB, patch)
2015-09-17 01:45 PDT, Ryosuke Niwa
koivisto: review+
Radar WebKit Bug Importer
Comment 1 2015-09-16 18:04:40 PDT
Radar WebKit Bug Importer
Comment 2 2015-09-16 18:04:52 PDT
Ryosuke Niwa
Comment 3 2015-09-16 20:38:37 PDT
Created attachment 261361 [details] EWS test
Ryosuke Niwa
Comment 4 2015-09-16 20:55:13 PDT
Created attachment 261362 [details] EFL fix attempt
Antti Koivisto
Comment 5 2015-09-16 21:17:52 PDT
Comment on attachment 261362 [details] EFL fix attempt View in context: https://bugs.webkit.org/attachment.cgi?id=261362&action=review > Source/WebCore/dom/ShadowRoot.cpp:155 > + RELEASE_ASSERT(m_slotAssignments); > + return m_slotAssignments->removeSlotElementByName(name, slot); No need for RELEASE_ASSERT, this is going to crash with clean(ish) null if it fails. > Source/WebCore/dom/ShadowRoot.cpp:158 > +void ShadowRoot::setNeedsReslotting() "Reslotting" is bit strange. Maybe invalidateSlotAssignments()? > Source/WebCore/dom/ShadowRoot.cpp:161 > + m_slotAssignments->setNeedsReslotting(); ...and just invalidate() > Source/WebCore/dom/ShadowRoot.h:87 > + const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&); > + > +#endif Unnecessary empty line. > Source/WebCore/dom/SlotAssignment.h:78 > + bool m_needsToResolveSlotElemenets { false }; Spelling "Elemenets"
Ryosuke Niwa
Comment 6 2015-09-16 21:22:08 PDT
Created attachment 261363 [details] EFL fix attempt 2
Build Bot
Comment 7 2015-09-16 22:14:07 PDT
Comment on attachment 261363 [details] EFL fix attempt 2 Attachment 261363 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/178356 New failing tests: fast/shadow-dom/HTMLSlotElement-interface.html
Build Bot
Comment 8 2015-09-16 22:14:10 PDT
Created attachment 261367 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Build Bot
Comment 9 2015-09-16 23:34:46 PDT
Comment on attachment 261363 [details] EFL fix attempt 2 Attachment 261363 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/178476 New failing tests: fast/shadow-dom/HTMLSlotElement-interface.html
Build Bot
Comment 10 2015-09-16 23:34:49 PDT
Created attachment 261374 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Ryosuke Niwa
Comment 11 2015-09-17 01:41:10 PDT
Comment on attachment 261363 [details] EFL fix attempt 2 View in context: https://bugs.webkit.org/attachment.cgi?id=261363&action=review > Source/WebCore/html/HTMLSlotElement.cpp:49 > + ASSERT_UNUSED(insertionPoint, HTMLElement::insertedInto(insertionPoint) == InsertionDone); Fuck, another case of not calling in release builds :(
Ryosuke Niwa
Comment 12 2015-09-17 01:43:32 PDT
Created attachment 261378 [details] Ready for review
Ryosuke Niwa
Comment 13 2015-09-17 01:45:32 PDT
Created attachment 261379 [details] Reverted changes to WebKit.xcworkspace
Antti Koivisto
Comment 14 2015-09-17 11:22:10 PDT
Comment on attachment 261379 [details] Reverted changes to WebKit.xcworkspace View in context: https://bugs.webkit.org/attachment.cgi?id=261379&action=review > Source/WebCore/dom/SlotAssignment.h:66 > + unsigned count { 0 }; This could use a more descriptive name. This is the count of equivalent (same name) slot elements in the shadow tree if I understand correctly. Maybe duplicateSlotElementCount or something? > Source/WebCore/dom/SlotAssignment.h:74 > + HashMap<AtomicString, SlotInfo> m_slots; You might consider <AtomicString, std::unique_ptr<SlotInfo>> instead. SlotInfo can be large and rehashing involves copying. With that the struct definition can be in cpp too.
Ryosuke Niwa
Comment 15 2015-09-17 17:47:23 PDT
Note You need to log in before you can comment on or make changes to this bug.