Bug 149241 - Add HTMLSlotElement, Element.slot, and NonDocumentTypeChildNode.assignedSlot
Summary: Add HTMLSlotElement, Element.slot, and NonDocumentTypeChildNode.assignedSlot
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar, WebExposed
Depends on: 149244
Blocks: 149242 149243
  Show dependency treegraph
 
Reported: 2015-09-16 18:03 PDT by Ryosuke Niwa
Modified: 2015-09-17 17:47 PDT (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2015-09-16 18:03:29 PDT
Add the support for slotting.
Comment 1 Radar WebKit Bug Importer 2015-09-16 18:04:40 PDT
<rdar://problem/22732247>
Comment 2 Radar WebKit Bug Importer 2015-09-16 18:04:52 PDT
<rdar://problem/22732252>
Comment 3 Ryosuke Niwa 2015-09-16 20:38:37 PDT
Created attachment 261361 [details]
EWS test
Comment 4 Ryosuke Niwa 2015-09-16 20:55:13 PDT
Created attachment 261362 [details]
EFL fix attempt
Comment 5 Antti Koivisto 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"
Comment 6 Ryosuke Niwa 2015-09-16 21:22:08 PDT
Created attachment 261363 [details]
EFL fix attempt 2
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Ryosuke Niwa 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 :(
Comment 12 Ryosuke Niwa 2015-09-17 01:43:32 PDT
Created attachment 261378 [details]
Ready for review
Comment 13 Ryosuke Niwa 2015-09-17 01:45:32 PDT
Created attachment 261379 [details]
Reverted changes to WebKit.xcworkspace
Comment 14 Antti Koivisto 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.
Comment 15 Ryosuke Niwa 2015-09-17 17:47:23 PDT
Committed r189950: <http://trac.webkit.org/changeset/189950>