Bug 149241

Summary:

Add HTMLSlotElement, Element.slot, and NonDocumentTypeChildNode.assignedSlot

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

DOM

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, koivisto, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar, WebExposed

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

149244

Bug Blocks:

149242, 149243

Attachments:

Description	Flags
EWS test	none
EFL fix attempt	none
EFL fix attempt 2	none
Archive of layout-test-results from ews104 for mac-mavericks-wk2	none
Archive of layout-test-results from ews101 for mac-mavericks	none
Ready for review	none
Reverted changes to WebKit.xcworkspace	koivisto: review+

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	Details Formatted Diff Diff
EFL fix attempt (167.46 KB, patch) 2015-09-16 20:55 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
EFL fix attempt 2 (167.51 KB, patch) 2015-09-16 21:22 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
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	Details
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	Details
Ready for review (173.53 KB, patch) 2015-09-17 01:43 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Reverted changes to WebKit.xcworkspace (167.53 KB, patch) 2015-09-17 01:45 PDT, Ryosuke Niwa	koivisto: review+	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2015-09-16 18:04:40 PDT

<rdar://problem/22732247>

Radar WebKit Bug Importer

Comment 2 2015-09-16 18:04:52 PDT

<rdar://problem/22732252>

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

Committed r189950: <http://trac.webkit.org/changeset/189950>

Note You need to log in before you can comment on or make changes to this bug.