Diff
Modified: trunk/Source/WebCore/ChangeLog (274817 => 274818)
--- trunk/Source/WebCore/ChangeLog 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/ChangeLog 2021-03-22 23:36:35 UTC (rev 274818)
@@ -1,3 +1,46 @@
+2021-03-22 Patrick Angle <[email protected]>
+
+ REGRESSION(r272433): Inspector should not instrument inside `WebCore::Node::setRenderer`
+ https://bugs.webkit.org/show_bug.cgi?id=223559
+
+ Reviewed by Ryosuke Niwa and Devin Rousso.
+
+ Existing test coverage:
+ - inspector/css/nodeLayoutContextTypeChanged.html
+ - inspector/css/setLayoutContextTypeChangedMode.html
+
+ The previous approach to observing render changes was most likely a performance regression in a very hot code
+ path (`Node::setRenderer`). This patch resolves this by not instrumenting in this the hot path. Instead we call
+ inspector instrumentation inside the constructors/destructors of only the RenderObject subclasses we are
+ interested in observing layout changes for.
+
+ Additionally, layout change events are now added to a `Vector` of pending changes, which will be sent to the
+ front-end later in order to avoid evaluating _javascript_ inside a destructor in WK1 with the new instrumentation
+ points.
+
+ * dom/Element.cpp:
+ (WebCore::Element::didChangeRenderer): Deleted.
+ * dom/Element.h:
+ * dom/Node.h:
+ (WebCore::Node::didChangeRenderer): Deleted.
+ * inspector/InspectorInstrumentation.cpp:
+ (WebCore::InspectorInstrumentation::nodeLayoutContextChangedImpl):
+ * inspector/InspectorInstrumentation.h:
+ (WebCore::InspectorInstrumentation::nodeLayoutContextChanged):
+ * inspector/agents/InspectorCSSAgent.cpp:
+ (WebCore::InspectorCSSAgent::InspectorCSSAgent):
+ (WebCore::InspectorCSSAgent::reset):
+ (WebCore::InspectorCSSAgent::nodeLayoutContextTypeChanged):
+ (WebCore::InspectorCSSAgent::layoutContextTypeChangedTimerFired):
+ - Moved layout change events behind a timer firing.
+ * inspector/agents/InspectorCSSAgent.h:
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::RenderGrid):
+ (WebCore::RenderGrid::~RenderGrid):
+ - Move instrumentation from `Node::setRenderer` to `RenderGrid`.
+ * rendering/RenderObject.h:
+ (WebCore::Node::setRenderer):
+
2021-03-22 Peng Liu <[email protected]>
[GPUP] Add a "wallTime" field to struct RemoteMediaPlayerState
Modified: trunk/Source/WebCore/dom/Element.cpp (274817 => 274818)
--- trunk/Source/WebCore/dom/Element.cpp 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/dom/Element.cpp 2021-03-22 23:36:35 UTC (rev 274818)
@@ -4594,11 +4594,6 @@
return ElementIdentifier::generate();
}
-void Element::didChangeRenderer(RenderObject* oldRenderer)
-{
- InspectorInstrumentation::nodeLayoutContextChanged(*this, oldRenderer);
-}
-
#if ENABLE(CSS_TYPED_OM)
StylePropertyMap* Element::attributeStyleMap()
Modified: trunk/Source/WebCore/dom/Element.h (274817 => 274818)
--- trunk/Source/WebCore/dom/Element.h 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/dom/Element.h 2021-03-22 23:36:35 UTC (rev 274818)
@@ -733,8 +733,6 @@
void attachAttributeNodeIfNeeded(Attr&);
- void didChangeRenderer(RenderObject*) final;
-
#if ASSERT_ENABLED
WEBCORE_EXPORT bool fastAttributeLookupAllowed(const QualifiedName&) const;
#endif
Modified: trunk/Source/WebCore/dom/Node.h (274817 => 274818)
--- trunk/Source/WebCore/dom/Node.h 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/dom/Node.h 2021-03-22 23:36:35 UTC (rev 274818)
@@ -710,8 +710,6 @@
static void moveTreeToNewScope(Node&, TreeScope& oldScope, TreeScope& newScope);
void moveNodeToNewDocument(Document& oldDocument, Document& newDocument);
- virtual void didChangeRenderer(RenderObject*) { };
-
struct NodeRareDataDeleter {
void operator()(NodeRareData*) const;
};
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (274817 => 274818)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2021-03-22 23:36:35 UTC (rev 274818)
@@ -176,10 +176,10 @@
domAgent->didRemoveDOMNode(node);
}
-void InspectorInstrumentation::nodeLayoutContextChangedImpl(InstrumentingAgents& instrumentingAgents, Node& node, RenderObject* oldRenderer)
+void InspectorInstrumentation::nodeLayoutContextChangedImpl(InstrumentingAgents& instrumentingAgents, Node& node, RenderObject* newRenderer)
{
if (auto* cssAgent = instrumentingAgents.enabledCSSAgent())
- cssAgent->nodeLayoutContextTypeChanged(node, oldRenderer);
+ cssAgent->nodeLayoutContextTypeChanged(node, newRenderer);
}
void InspectorInstrumentation::willModifyDOMAttrImpl(InstrumentingAgents& instrumentingAgents, Element& element, const AtomString& oldValue, const AtomString& newValue)
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (274817 => 274818)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2021-03-22 23:36:35 UTC (rev 274818)
@@ -601,11 +601,11 @@
didRemoveDOMNodeImpl(*agents, node);
}
-inline void InspectorInstrumentation::nodeLayoutContextChanged(Node& node, RenderObject* oldRenderer)
+inline void InspectorInstrumentation::nodeLayoutContextChanged(Node& node, RenderObject* newRenderer)
{
FAST_RETURN_IF_NO_FRONTENDS(void());
if (auto* agents = instrumentingAgents(node.document()))
- nodeLayoutContextChangedImpl(*agents, node, oldRenderer);
+ nodeLayoutContextChangedImpl(*agents, node, newRenderer);
}
inline void InspectorInstrumentation::willModifyDOMAttr(Document& document, Element& element, const AtomString& oldValue, const AtomString& newValue)
Modified: trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp (274817 => 274818)
--- trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.cpp 2021-03-22 23:36:35 UTC (rev 274818)
@@ -304,6 +304,7 @@
: InspectorAgentBase("CSS"_s, context)
, m_frontendDispatcher(makeUnique<CSSFrontendDispatcher>(context.frontendRouter))
, m_backendDispatcher(CSSBackendDispatcher::create(context.backendDispatcher, this))
+ , m_layoutContextTypeChangedTimer(*this, &InspectorCSSAgent::layoutContextTypeChangedTimerFired)
{
}
@@ -326,6 +327,9 @@
m_nodeToInspectorStyleSheet.clear();
m_documentToInspectorStyleSheet.clear();
m_documentToKnownCSSStyleSheets.clear();
+ m_nodesWithPendingLayoutContextTypeChanges.clear();
+ if (m_layoutContextTypeChangedTimer.isActive())
+ m_layoutContextTypeChangedTimer.stop();
m_layoutContextTypeChangedMode = Protocol::CSS::LayoutContextTypeChangedMode::Observed;
resetPseudoStates();
}
@@ -966,16 +970,12 @@
return { };
}
-void InspectorCSSAgent::nodeLayoutContextTypeChanged(Node& node, RenderObject* oldRenderer)
+void InspectorCSSAgent::nodeLayoutContextTypeChanged(Node& node, RenderObject* newRenderer)
{
auto* domAgent = m_instrumentingAgents.persistentDOMAgent();
if (!domAgent)
return;
-
- auto newLayoutContextType = layoutContextTypeForRenderer(node.renderer());
- if (newLayoutContextType == layoutContextTypeForRenderer(oldRenderer))
- return;
-
+
auto nodeId = domAgent->boundNodeId(&node);
if (!nodeId && m_layoutContextTypeChangedMode == Protocol::CSS::LayoutContextTypeChangedMode::All) {
// FIXME: <https://webkit.org/b/189687> Preserve DOM.NodeId if a node is removed and re-added
@@ -983,10 +983,18 @@
}
if (!nodeId)
return;
-
- m_frontendDispatcher->nodeLayoutContextTypeChanged(nodeId, WTFMove(newLayoutContextType));
+
+ m_nodesWithPendingLayoutContextTypeChanges.set(nodeId, layoutContextTypeForRenderer(newRenderer));
+ if (!m_layoutContextTypeChangedTimer.isActive())
+ m_layoutContextTypeChangedTimer.startOneShot(0_s);
}
+void InspectorCSSAgent::layoutContextTypeChangedTimerFired()
+{
+ for (auto&& [nodeId, layoutContextType] : std::exchange(m_nodesWithPendingLayoutContextTypeChanges, { }))
+ m_frontendDispatcher->nodeLayoutContextTypeChanged(nodeId, WTFMove(layoutContextType));
+}
+
InspectorStyleSheetForInlineStyle& InspectorCSSAgent::asInspectorStyleSheet(StyledElement& element)
{
return m_nodeToInspectorStyleSheet.ensure(&element, [this, &element] {
Modified: trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h (274817 => 274818)
--- trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/inspector/agents/InspectorCSSAgent.h 2021-03-22 23:36:35 UTC (rev 274818)
@@ -30,6 +30,7 @@
#include "InspectorStyleSheet.h"
#include "InspectorWebAgentBase.h"
#include "SecurityContext.h"
+#include "Timer.h"
#include <_javascript_Core/InspectorBackendDispatchers.h>
#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
@@ -156,6 +157,7 @@
Ref<JSON::ArrayOf<Inspector::Protocol::CSS::RuleMatch>> buildArrayForMatchedRuleList(const Vector<RefPtr<const StyleRule>>&, Style::Resolver&, Element&, PseudoId);
RefPtr<Inspector::Protocol::CSS::CSSStyle> buildObjectForAttributesStyle(StyledElement&);
+ void layoutContextTypeChangedTimerFired();
void resetPseudoStates();
@@ -172,6 +174,9 @@
int m_lastStyleSheetId { 1 };
bool m_creatingViaInspectorStyleSheet { false };
+
+ HashMap<Inspector::Protocol::DOM::NodeId, Optional<Inspector::Protocol::CSS::LayoutContextType>> m_nodesWithPendingLayoutContextTypeChanges;
+ Timer m_layoutContextTypeChangedTimer;
Inspector::Protocol::CSS::LayoutContextTypeChangedMode m_layoutContextTypeChangedMode { Inspector::Protocol::CSS::LayoutContextTypeChangedMode::Observed };
};
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (274817 => 274818)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2021-03-22 23:36:35 UTC (rev 274818)
@@ -31,6 +31,7 @@
#include "GridLayoutFunctions.h"
#include "GridPositionsResolver.h"
#include "GridTrackSizingAlgorithm.h"
+#include "InspectorInstrumentation.h"
#include "LayoutRepainter.h"
#include "RenderChildIterator.h"
#include "RenderLayer.h"
@@ -56,9 +57,14 @@
{
// All of our children must be block level.
setChildrenInline(false);
+
+ InspectorInstrumentation::nodeLayoutContextChanged(element, this);
}
-RenderGrid::~RenderGrid() = default;
+RenderGrid::~RenderGrid()
+{
+ InspectorInstrumentation::nodeLayoutContextChanged(element(), nullptr);
+}
StyleSelfAlignmentData RenderGrid::selfAlignmentForChild(GridAxis axis, const RenderBox& child, const RenderStyle* gridStyle) const
{
Modified: trunk/Source/WebCore/rendering/RenderObject.h (274817 => 274818)
--- trunk/Source/WebCore/rendering/RenderObject.h 2021-03-22 23:30:10 UTC (rev 274817)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2021-03-22 23:36:35 UTC (rev 274818)
@@ -1150,9 +1150,7 @@
inline void Node::setRenderer(RenderObject* renderer)
{
- auto oldRenderer = this->renderer();
m_rendererWithStyleFlags.setPointer(renderer);
- didChangeRenderer(oldRenderer);
}
WTF::TextStream& operator<<(WTF::TextStream&, const RenderObject&);
