Skip to content

Commit

Permalink
[StyleBuilder] Pass a DisplayStyle to LayoutObjectIsNeeded
Browse files Browse the repository at this point in the history
We need to call Element::LayoutObjectIsNeeded in several places
before the ComputedStyle is actually finished. To avoid calling
InternalStyle, we can instead transport the relevant data in a new
DisplayStyle class, and let various LayoutObjectIsNeeded overrides
act on this instead of acting on ComputedStyle directly.

This is a noisy CL, but there should not be any behavior change.

Bug: 1377295
Change-Id: I565ece32a6d127c549dee5f9bc1f5786c9927e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188888
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096856}
  • Loading branch information
andruud authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 1b732c5 commit 4479f5f
Show file tree
Hide file tree
Showing 57 changed files with 150 additions and 80 deletions.
Expand Up @@ -685,7 +685,7 @@ void StyleAdjuster::AdjustEffectiveTouchAction(
IsA<HTMLImageElement>(element) || is_replaced_canvas);
bool is_table_row_or_column = builder.IsDisplayTableRowOrColumnType();
bool is_layout_object_needed =
element && element->LayoutObjectIsNeeded(*builder.InternalStyle());
element && element->LayoutObjectIsNeeded(builder.GetDisplayStyle());

TouchAction element_touch_action = TouchAction::kAuto;
// Touch actions are only supported by elements that support both the CSS
Expand Down Expand Up @@ -846,7 +846,7 @@ void StyleAdjuster::AdjustComputedStyle(StyleResolverState& state,
auto* html_element = DynamicTo<HTMLElement>(element);
if (html_element &&
(builder.Display() != EDisplay::kNone ||
element->LayoutObjectIsNeeded(*builder.InternalStyle()))) {
element->LayoutObjectIsNeeded(builder.GetDisplayStyle()))) {
AdjustStyleForHTMLElement(builder, *html_element);
}

Expand Down
Expand Up @@ -169,7 +169,7 @@ void StyleResolverState::LoadPendingResources() {
if (pseudo_request_type_ == StyleRequest::kForComputedStyle ||
(ParentStyle() && ParentStyle()->IsEnsuredInDisplayNone()) ||
(StyleBuilder().Display() == EDisplay::kNone &&
!GetElement().LayoutObjectIsNeeded(*style_builder_.InternalStyle())) ||
!GetElement().LayoutObjectIsNeeded(style_builder_.GetDisplayStyle())) ||
StyleBuilder().IsEnsuredOutsideFlatTree()) {
return;
}
Expand Down
6 changes: 5 additions & 1 deletion third_party/blink/renderer/core/dom/element.cc
Expand Up @@ -2640,11 +2640,15 @@ const AtomicString Element::ImageSourceURL() const {
return FastGetAttribute(html_names::kSrcAttr);
}

bool Element::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool Element::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return style.Display() != EDisplay::kNone &&
style.Display() != EDisplay::kContents;
}

bool Element::LayoutObjectIsNeeded(const ComputedStyle& style) const {
return LayoutObjectIsNeeded(style.GetDisplayStyle());
}

LayoutObject* Element::CreateLayoutObject(const ComputedStyle& style,
LegacyLayout legacy) {
return LayoutObject::CreateObject(this, style, legacy);
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/dom/element.h
Expand Up @@ -83,6 +83,7 @@ class DOMRectList;
class DOMStringMap;
class DOMTokenList;
class DisplayLockContext;
class DisplayStyle;
class Document;
class EditContext;
class ElementAnimations;
Expand Down Expand Up @@ -593,7 +594,8 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
void DetachLayoutTree(bool performing_reattach = false) override;

virtual LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout);
virtual bool LayoutObjectIsNeeded(const ComputedStyle&) const;
virtual bool LayoutObjectIsNeeded(const DisplayStyle&) const;
bool LayoutObjectIsNeeded(const ComputedStyle&) const;

const ComputedStyle* ParentComputedStyle() const;

Expand Down
25 changes: 18 additions & 7 deletions third_party/blink/renderer/core/dom/pseudo_element.cc
Expand Up @@ -48,6 +48,9 @@

namespace blink {

bool PseudoElementLayoutObjectIsNeeded(const DisplayStyle& pseudo_style,
const Element* originating_element);

PseudoElement* PseudoElement::Create(Element* parent,
PseudoId pseudo_id,
const AtomicString& view_transition_name) {
Expand Down Expand Up @@ -313,8 +316,8 @@ void PseudoElement::AttachLayoutTree(AttachContext& context) {
}
}

bool PseudoElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
return PseudoElementLayoutObjectIsNeeded(&style, parentElement());
bool PseudoElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return PseudoElementLayoutObjectIsNeeded(style, parentElement());
}

bool PseudoElement::CanGeneratePseudoElement(PseudoId pseudo_id) const {
Expand Down Expand Up @@ -350,9 +353,16 @@ bool PseudoElementLayoutObjectIsNeeded(const ComputedStyle* pseudo_style,
const Element* originating_element) {
if (!pseudo_style)
return false;
if (pseudo_style->Display() == EDisplay::kNone)
return PseudoElementLayoutObjectIsNeeded(pseudo_style->GetDisplayStyle(),
originating_element);
}

bool PseudoElementLayoutObjectIsNeeded(const DisplayStyle& pseudo_style,
const Element* originating_element) {
if (pseudo_style.Display() == EDisplay::kNone) {
return false;
switch (pseudo_style->StyleType()) {
}
switch (pseudo_style.StyleType()) {
case kPseudoIdFirstLetter:
case kPseudoIdBackdrop:
case kPseudoIdViewTransition:
Expand All @@ -363,10 +373,11 @@ bool PseudoElementLayoutObjectIsNeeded(const ComputedStyle* pseudo_style,
return true;
case kPseudoIdBefore:
case kPseudoIdAfter:
return !pseudo_style->ContentPreventsBoxGeneration();
return !pseudo_style.ContentPreventsBoxGeneration();
case kPseudoIdMarker: {
if (!pseudo_style->ContentBehavesAsNormal())
return !pseudo_style->ContentPreventsBoxGeneration();
if (!pseudo_style.ContentBehavesAsNormal()) {
return !pseudo_style.ContentPreventsBoxGeneration();
}
const ComputedStyle* parent_style =
originating_element->GetComputedStyle();
return parent_style && (parent_style->ListStyleType() ||
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/dom/pseudo_element.h
Expand Up @@ -57,7 +57,7 @@ class CORE_EXPORT PseudoElement : public Element {
scoped_refptr<ComputedStyle> CustomStyleForLayoutObject(
const StyleRecalcContext&) override;
void AttachLayoutTree(AttachContext&) override;
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
bool CanGeneratePseudoElement(PseudoId) const override;

bool CanStartSelection() const override { return false; }
Expand Down
Expand Up @@ -610,7 +610,7 @@ void HTMLFencedFrameElement::AttachLayoutTree(AttachContext& context) {
}

bool HTMLFencedFrameElement::LayoutObjectIsNeeded(
const ComputedStyle& style) const {
const DisplayStyle& style) const {
return !collapsed_by_client_ &&
HTMLFrameOwnerElement::LayoutObjectIsNeeded(style);
}
Expand Down
Expand Up @@ -145,7 +145,7 @@ class CORE_EXPORT HTMLFencedFrameElement : public HTMLFrameOwnerElement {
const QualifiedName&,
const AtomicString&,
MutableCSSPropertyValueSet*) override;
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;
void AttachLayoutTree(AttachContext& context) override;
bool SupportsFocus() const override;
Expand Down
Expand Up @@ -939,7 +939,7 @@ void HTMLInputElement::FinishParsingChildren() {
}
}

bool HTMLInputElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool HTMLInputElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return input_type_->LayoutObjectIsNeeded() &&
TextControlElement::LayoutObjectIsNeeded(style);
}
Expand Down
Expand Up @@ -217,7 +217,7 @@ class CORE_EXPORT HTMLInputElement
unsigned end,
ExceptionState&);

bool LayoutObjectIsNeeded(const ComputedStyle&) const final;
bool LayoutObjectIsNeeded(const DisplayStyle&) const final;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;
void DetachLayoutTree(bool performing_reattach) final;
void UpdateSelectionOnFocus(SelectionBehaviorOnFocus,
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/html_embed_element.cc
Expand Up @@ -173,7 +173,7 @@ void HTMLEmbedElement::UpdatePluginInternal() {
RequestObject(plugin_params);
}

bool HTMLEmbedElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool HTMLEmbedElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
if (IsImageType())
return HTMLPlugInElement::LayoutObjectIsNeeded(style);

Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/html_embed_element.h
Expand Up @@ -51,7 +51,7 @@ class CORE_EXPORT HTMLEmbedElement final : public HTMLPlugInElement {
const AtomicString&,
MutableCSSPropertyValueSet*) override;

bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;

bool IsURLAttribute(const Attribute&) const override;
const QualifiedName& SubResourceAttributeName() const override;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/html_frame_element.cc
Expand Up @@ -37,7 +37,7 @@ HTMLFrameElement::HTMLFrameElement(Document& document)
frame_border_(true),
frame_border_set_(false) {}

bool HTMLFrameElement::LayoutObjectIsNeeded(const ComputedStyle&) const {
bool HTMLFrameElement::LayoutObjectIsNeeded(const DisplayStyle&) const {
// For compatibility, frames render even when display: none is set.
return ContentFrame();
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/html_frame_element.h
Expand Up @@ -49,7 +49,7 @@ class CORE_EXPORT HTMLFrameElement final : public HTMLFrameElementBase {
}

private:
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;

void ParseAttribute(const AttributeModificationParams&) override;
Expand Down
Expand Up @@ -408,7 +408,7 @@ const Vector<bool>& HTMLFrameSetElement::AllowBorderColumns() const {
}

bool HTMLFrameSetElement::LayoutObjectIsNeeded(
const ComputedStyle& style) const {
const DisplayStyle& style) const {
// For compatibility, frames layoutObject even when display: none is set.
return true;
}
Expand Down
Expand Up @@ -89,7 +89,7 @@ class HTMLFrameSetElement final : public HTMLElement {
MutableCSSPropertyValueSet*) override;

void AttachLayoutTree(AttachContext&) override;
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;

void DefaultEventHandler(Event&) override;
Expand Down
Expand Up @@ -443,7 +443,7 @@ ParsedPermissionsPolicy HTMLIFrameElement::ConstructContainerPolicy() const {
return container_policy;
}

bool HTMLIFrameElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool HTMLIFrameElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return ContentFrame() && !collapsed_by_client_ &&
HTMLElement::LayoutObjectIsNeeded(style);
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/html_iframe_element.h
Expand Up @@ -76,7 +76,7 @@ class CORE_EXPORT HTMLIFrameElement : public HTMLFrameElementBase,
InsertionNotificationRequest InsertedInto(ContainerNode&) override;
void RemovedFrom(ContainerNode&) override;

bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;

bool IsInteractiveContent() const override;
Expand Down
Expand Up @@ -40,8 +40,7 @@ namespace blink {
HTMLNoEmbedElement::HTMLNoEmbedElement(Document& document)
: HTMLElement(html_names::kNoembedTag, document) {}

bool HTMLNoEmbedElement::LayoutObjectIsNeeded(
const ComputedStyle& style) const {
bool HTMLNoEmbedElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
if (GetDocument().GetFrame()->Loader().AllowPlugins())
return false;
return Element::LayoutObjectIsNeeded(style);
Expand Down
Expand Up @@ -42,7 +42,7 @@ class HTMLNoEmbedElement final : public HTMLElement {
explicit HTMLNoEmbedElement(Document&);

private:
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
};

} // namespace blink
Expand Down
Expand Up @@ -41,7 +41,7 @@ HTMLNoScriptElement::HTMLNoScriptElement(Document& document)
: HTMLElement(html_names::kNoscriptTag, document) {}

bool HTMLNoScriptElement::LayoutObjectIsNeeded(
const ComputedStyle& style) const {
const DisplayStyle& style) const {
if (GetExecutionContext()->CanExecuteScripts(kNotAboutToExecuteScript))
return false;
return Element::LayoutObjectIsNeeded(style);
Expand Down
Expand Up @@ -42,7 +42,7 @@ class HTMLNoScriptElement final : public HTMLElement {
explicit HTMLNoScriptElement(Document&);

private:
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
};

} // namespace blink
Expand Down
Expand Up @@ -800,7 +800,7 @@ void HTMLMediaElement::FinishParsingChildren() {
ScheduleTextTrackResourceLoad();
}

bool HTMLMediaElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool HTMLMediaElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return ShouldShowControls() && HTMLElement::LayoutObjectIsNeeded(style);
}

Expand Down
Expand Up @@ -494,7 +494,7 @@ class CORE_EXPORT HTMLMediaElement

bool SupportsFocus() const final;
bool IsMouseFocusable() const final;
bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;
void DidNotifySubtreeInsertionsToDocument() override;
void DidRecalcStyle(const StyleRecalcChange) final;
Expand Down
Expand Up @@ -144,7 +144,7 @@ void HTMLVideoElement::ContextDestroyed() {
HTMLMediaElement::ContextDestroyed();
}

bool HTMLVideoElement::LayoutObjectIsNeeded(const ComputedStyle& style) const {
bool HTMLVideoElement::LayoutObjectIsNeeded(const DisplayStyle& style) const {
return HTMLElement::LayoutObjectIsNeeded(style);
}

Expand Down
Expand Up @@ -178,7 +178,7 @@ class CORE_EXPORT HTMLVideoElement final
// ExecutionContextLifecycleStateObserver functions.
void ContextDestroyed() final;

bool LayoutObjectIsNeeded(const ComputedStyle&) const override;
bool LayoutObjectIsNeeded(const DisplayStyle&) const override;
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;
void AttachLayoutTree(AttachContext&) override;
void UpdatePosterImage();
Expand Down
Expand Up @@ -43,9 +43,7 @@ class HTMLFreezableIFrameElement : public HTMLIFrameElement {
}

private:
bool LayoutObjectIsNeeded(const ComputedStyle&) const override {
return true;
}
bool LayoutObjectIsNeeded(const DisplayStyle&) const override { return true; }
LayoutObject* CreateLayoutObject(const ComputedStyle&,
LegacyLayout) override {
return MakeGarbageCollected<LayoutFreezableIFrame>(this);
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/style/build.gni
Expand Up @@ -28,6 +28,7 @@ blink_core_sources_style = [
"cursor_data.h",
"cursor_list.h",
"data_ref.h",
"display_style.h",
"fill_layer.cc",
"fill_layer.h",
"filter_operation.cc",
Expand Down
25 changes: 10 additions & 15 deletions third_party/blink/renderer/core/style/computed_style.h
Expand Up @@ -52,6 +52,7 @@
#include "third_party/blink/renderer/core/style/computed_style_initial_values.h"
#include "third_party/blink/renderer/core/style/cursor_list.h"
#include "third_party/blink/renderer/core/style/data_ref.h"
#include "third_party/blink/renderer/core/style/display_style.h"
#include "third_party/blink/renderer/core/style/style_cached_data.h"
#include "third_party/blink/renderer/core/style/style_highlight_data.h"
#include "third_party/blink/renderer/core/style/transform_origin.h"
Expand Down Expand Up @@ -1730,25 +1731,16 @@ class ComputedStyle : public ComputedStyleBase,
// Isolation utility functions.
bool HasIsolation() const { return Isolation() != EIsolation::kAuto; }

DisplayStyle GetDisplayStyle() const {
return DisplayStyle(Display(), StyleType(), GetContentData());
}

// Content utility functions.
bool ContentBehavesAsNormal() const {
switch (StyleType()) {
case kPseudoIdMarker:
return !GetContentData();
default:
return !GetContentData() || GetContentData()->IsNone();
}
return GetDisplayStyle().ContentBehavesAsNormal();
}
bool ContentPreventsBoxGeneration() const {
switch (StyleType()) {
case kPseudoIdBefore:
case kPseudoIdAfter:
return ContentBehavesAsNormal();
case kPseudoIdMarker:
return GetContentData() && GetContentData()->IsNone();
default:
return false;
}
return GetDisplayStyle().ContentPreventsBoxGeneration();
}

// Cursor utility functions.
Expand Down Expand Up @@ -2958,6 +2950,9 @@ class ComputedStyleBuilder final : public ComputedStyleBuilderBase {
Display() == EDisplay::kTableColumn ||
Display() == EDisplay::kTableColumnGroup;
}
DisplayStyle GetDisplayStyle() const {
return DisplayStyle(Display(), StyleType(), GetContentData());
}

// filter
FilterOperations& MutableFilter() {
Expand Down

0 comments on commit 4479f5f

Please sign in to comment.