New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 681134 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

assignedSlot() is allocating a SlotAssignment on every ShadowRoot, even ones without slots

Project Member Reported by esprehn@chromium.org, Jan 13 2017

Issue description

Inside recalcStyle we do .assignedSlot():

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp?sq=package:chromium&rcl=1484310312&l=269

And assignedSlot() does ensureSlotAssignment().find(...) which means we end up allocating a SlotAssignment every ShadowRoot in the entire document.

assignedSlot() and assignedSlotForBindings() should both null check the SlotAssignment and not force one to exist. If one doesn't exist then you can early return nullptr since there's no slot you're assigned to. Similarly touching element.assignedSlot from script shouldn't force an assignment to exist.
 

Comment 1 by hayato@chromium.org, Jan 16 2017

Owner: hayato@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b

commit db77e9ace4f1948a8d1c87c2bd2de1380526bb5b
Author: hayato <hayato@chromium.org>
Date: Thu Jan 19 05:00:45 2017

Do not allocate SlotAssignment unless a ShadowRoot has a slot

Make ShadowRoot::ensureSlotAssignment() private so that it can not be used from
outside. Now only ShadowRoot has a responsibility in allocating SlotAssignment.

Instead of ShadowRoot::ensureSlotAssignment(), ShadowRoot now provides
public ShadowRoot::slotAssignment(), but it uses DCHECK(m_slotAssignment) so that
it can be used only where we can assume that SlotAssignment is already allocated.

BUG= 681134 

Review-Url: https://codereview.chromium.org/2630293003
Cr-Commit-Position: refs/heads/master@{#444647}

[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h
[modify] https://crrev.com/db77e9ace4f1948a8d1c87c2bd2de1380526bb5b/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp

Comment 3 by hayato@chromium.org, Jan 19 2017

Status: Fixed (was: Assigned)

Sign in to add a comment