Skip to content

Commit

Permalink
Use more smart pointers in ProcessingInstruction & PseudoElement
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=263537

Reviewed by Ryosuke Niwa.

* Source/WebCore/css/CSSStyleSheet.cpp:
(WebCore::CSSStyleSheet::protectedContents):
* Source/WebCore/css/CSSStyleSheet.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::checkedStyleScope):
(WebCore::Document::checkedStyleScope const):
* Source/WebCore/dom/Document.h:
(WebCore::Document::styleScope):
(WebCore::Document::styleScope const):
* Source/WebCore/dom/ProcessingInstruction.cpp:
(WebCore::ProcessingInstruction::~ProcessingInstruction):

(WebCore::ProcessingInstruction::checkStyleSheet):
Drop some unnecessary code that was used to check if we got disconnected
after firing the beforeload event (as you can see in https://commits.webkit.org/187003@main).
We've stopped firing the beforeload event a while back but this code wasn't
simplified.

(WebCore::ProcessingInstruction::isLoading const):
(WebCore::ProcessingInstruction::sheetLoaded):
(WebCore::ProcessingInstruction::setCSSStyleSheet):
(WebCore::ProcessingInstruction::setXSLStyleSheet):
(WebCore::ProcessingInstruction::protectedSheet const):
(WebCore::ProcessingInstruction::parseStyleSheet):
(WebCore::ProcessingInstruction::insertedIntoAncestor):
(WebCore::ProcessingInstruction::removedFromAncestor):
* Source/WebCore/dom/ProcessingInstruction.h:
* Source/WebCore/dom/PseudoElement.cpp:
(WebCore::PseudoElement::create):
(WebCore::PseudoElement::clearHostElement):
* Source/WebCore/style/StyleScope.cpp:
(WebCore::Style::Scope::incrementPtrCount const):
(WebCore::Style::Scope::decrementPtrCount const):
(WebCore::Style::Scope::createDocumentResolver):
(WebCore::Style::Scope::releaseMemory):
(WebCore::Style::Scope::didRemovePendingStylesheet):
(WebCore::Style::Scope::addStyleSheetCandidateNode):
(WebCore::Style::Scope::collectActiveStyleSheets):
(WebCore::Style::Scope::updateActiveStyleSheets):
(WebCore::Style::Scope::invalidateStyleAfterStyleSheetChange):
(WebCore::Style::Scope::activeStyleSheetsForInspector):
(WebCore::Style::Scope::flushPendingDescendantUpdates):
(WebCore::Style::Scope::scheduleUpdate):
(WebCore::Style::Scope::collectResolverScopes):
(WebCore::Style::Scope::didChangeStyleSheetEnvironment):
(WebCore::Style::Scope::didChangeViewportSize):
(WebCore::Style::Scope::invalidateMatchedDeclarationsCache):
(WebCore::Style::Scope::pendingUpdateTimerFired):
(WebCore::Style::Scope::documentScope):
(WebCore::Style::Scope::updateQueryContainerState):
* Source/WebCore/style/StyleScope.h:

Canonical link: https://commits.webkit.org/269694@main
  • Loading branch information
@cdumez
cdumez committed Oct 24, 2023
1 parent 15b02a1 commit 728c252
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 101 deletions.
5 changes: 5 additions & 0 deletions Source/WebCore/css/CSSStyleSheet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,11 @@ void CSSStyleSheet::removeAdoptingTreeScope(ContainerNode& treeScope)
styleScopeFor(treeScope).didChangeStyleSheetContents();
}

Ref<StyleSheetContents> CSSStyleSheet::protectedContents()
{
return m_contents;
}

CSSStyleSheet::RuleMutationScope::RuleMutationScope(CSSStyleSheet* sheet, RuleMutationType mutationType, StyleRuleKeyframes* insertedKeyframesRule)
: m_styleSheet(sheet)
, m_mutationType(mutationType)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/css/CSSStyleSheet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class CSSStyleSheet final : public StyleSheet, public CanMakeCheckedPtr {
void reattachChildRuleCSSOMWrappers();

StyleSheetContents& contents() { return m_contents; }
Ref<StyleSheetContents> protectedContents();

bool isInline() const { return m_isInlineStylesheet; }
TextPosition startPosition() const { return m_startPosition; }
Expand Down
12 changes: 11 additions & 1 deletion Source/WebCore/dom/Document.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ Document::Document(LocalFrame* frame, const Settings& settings, const URL& url,
, m_cachedResourceLoader(createCachedResourceLoader(frame))
, m_creationURL(url)
, m_domTreeVersion(++s_globalTreeVersion)
, m_styleScope(makeUnique<Style::Scope>(*this))
, m_styleScope(makeUniqueRef<Style::Scope>(*this))
, m_extensionStyleSheets(makeUnique<ExtensionStyleSheets>(*this))
, m_visitedLinkState(makeUnique<VisitedLinkState>(*this))
, m_markers(makeUniqueRef<DocumentMarkerController>(*this))
Expand Down Expand Up @@ -9776,6 +9776,16 @@ Ref<CSSFontSelector> Document::protectedFontSelector() const
return m_fontSelector;
}

CheckedRef<Style::Scope> Document::checkedStyleScope()
{
return m_styleScope.get();
}

CheckedRef<const Style::Scope> Document::checkedStyleScope() const
{
return m_styleScope.get();
}

} // namespace WebCore

#undef DOCUMENT_RELEASE_LOG
Expand Down
9 changes: 6 additions & 3 deletions Source/WebCore/dom/Document.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,11 @@ class Document

WEBCORE_EXPORT StyleSheetList& styleSheets();

Style::Scope& styleScope() { return *m_styleScope; }
const Style::Scope& styleScope() const { return *m_styleScope; }
Style::Scope& styleScope() { return m_styleScope; }
const Style::Scope& styleScope() const { return m_styleScope; }
CheckedRef<Style::Scope> checkedStyleScope();
CheckedRef<const Style::Scope> checkedStyleScope() const;

ExtensionStyleSheets& extensionStyleSheets() { return *m_extensionStyleSheets; }
const ExtensionStyleSheets& extensionStyleSheets() const { return *m_extensionStyleSheets; }

Expand Down Expand Up @@ -2020,7 +2023,7 @@ class Document
WeakHashSet<NodeIterator> m_nodeIterators;
HashSet<CheckedRef<Range>> m_ranges;

std::unique_ptr<Style::Scope> m_styleScope;
UniqueRef<Style::Scope> m_styleScope;
std::unique_ptr<ExtensionStyleSheets> m_extensionStyleSheets;
RefPtr<StyleSheetList> m_styleSheetList;

Expand Down
112 changes: 52 additions & 60 deletions Source/WebCore/dom/ProcessingInstruction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ Ref<ProcessingInstruction> ProcessingInstruction::create(Document& document, Str

ProcessingInstruction::~ProcessingInstruction()
{
if (m_sheet)
m_sheet->clearOwnerNode();
if (RefPtr sheet = m_sheet)
sheet->clearOwnerNode();

if (m_cachedSheet)
m_cachedSheet->removeClient(*this);
if (CachedResourceHandle cachedSheet = m_cachedSheet)
cachedSheet->removeClient(*this);

if (isConnected())
document().styleScope().removeStyleSheetCandidateNode(*this);
document().checkedStyleScope()->removeStyleSheetCandidateNode(*this);
}

String ProcessingInstruction::nodeName() const
Expand Down Expand Up @@ -113,6 +113,7 @@ void ProcessingInstruction::checkStyleSheet()
if (m_alternate && m_title.isEmpty())
return;

Ref document = this->document();
if (href.length() > 1 && href[0] == '#') {
m_localHref = href.substring(1);
#if ENABLE(XSLT)
Expand All @@ -122,55 +123,42 @@ void ProcessingInstruction::checkStyleSheet()
URL finalURL({ }, m_localHref);
m_sheet = XSLStyleSheet::createEmbedded(*this, finalURL);
m_loading = false;
document().scheduleToApplyXSLTransforms();
document->scheduleToApplyXSLTransforms();
}
#endif
} else {
if (m_cachedSheet) {
m_cachedSheet->removeClient(*this);
m_cachedSheet = nullptr;
}

if (m_loading) {
m_loading = false;
document().styleScope().removePendingSheet(*this);
}

Ref<Document> originalDocument = document();

String url = document().completeURL(href).string();

bool didEventListenerDisconnectThisElement = !isConnected() || &document() != originalDocument.ptr();
if (didEventListenerDisconnectThisElement)
return;
if (CachedResourceHandle cachedSheet = std::exchange(m_cachedSheet, nullptr))
cachedSheet->removeClient(*this);

m_loading = true;
document().styleScope().addPendingSheet(*this);
if (!m_loading) {
m_loading = true;
document->checkedStyleScope()->addPendingSheet(*this);
}

ASSERT_WITH_SECURITY_IMPLICATION(!m_cachedSheet);

#if ENABLE(XSLT)
if (m_isXSL) {
auto options = CachedResourceLoader::defaultCachedResourceOptions();
options.mode = FetchOptions::Mode::SameOrigin;
m_cachedSheet = document().cachedResourceLoader().requestXSLStyleSheet({ResourceRequest(document().completeURL(href)), options}).value_or(nullptr);
m_cachedSheet = document->protectedCachedResourceLoader()->requestXSLStyleSheet({ ResourceRequest(document->completeURL(href)), options }).value_or(nullptr);
} else
#endif
{
String charset = attributes->get<HashTranslatorASCIILiteral>("charset"_s);
CachedResourceRequest request(document().completeURL(href), CachedResourceLoader::defaultCachedResourceOptions(), std::nullopt, charset.isEmpty() ? document().charset() : WTFMove(charset));
CachedResourceRequest request(document->completeURL(href), CachedResourceLoader::defaultCachedResourceOptions(), std::nullopt, charset.isEmpty() ? document->charset() : WTFMove(charset));

m_cachedSheet = document().cachedResourceLoader().requestCSSStyleSheet(WTFMove(request)).value_or(nullptr);
m_cachedSheet = document->protectedCachedResourceLoader()->requestCSSStyleSheet(WTFMove(request)).value_or(nullptr);
}
if (m_cachedSheet)
m_cachedSheet->addClient(*this);
if (CachedResourceHandle cachedSheet = m_cachedSheet)
cachedSheet->addClient(*this);
else {
// The request may have been denied if (for example) the stylesheet is local and the document is remote.
m_loading = false;
document().styleScope().removePendingSheet(*this);
document->checkedStyleScope()->removePendingSheet(*this);
#if ENABLE(XSLT)
if (m_isXSL)
document().scheduleToApplyXSLTransforms();
document->scheduleToApplyXSLTransforms();
#endif
}
}
Expand All @@ -181,19 +169,18 @@ bool ProcessingInstruction::isLoading() const
{
if (m_loading)
return true;
if (!m_sheet)
return false;
return m_sheet->isLoading();
return m_sheet && m_sheet->isLoading();
}

bool ProcessingInstruction::sheetLoaded()
{
if (!isLoading()) {
if (document().styleScope().hasPendingSheet(*this))
document().styleScope().removePendingSheet(*this);
Ref document = this->document();
if (CheckedRef styleScope = document->styleScope(); styleScope->hasPendingSheet(*this))
styleScope->removePendingSheet(*this);
#if ENABLE(XSLT)
if (m_isXSL)
document().scheduleToApplyXSLTransforms();
document->scheduleToApplyXSLTransforms();
#endif
return true;
}
Expand All @@ -207,20 +194,20 @@ void ProcessingInstruction::setCSSStyleSheet(const String& href, const URL& base
return;
}

Ref document = this->document();
ASSERT(m_isCSS);
CSSParserContext parserContext(document(), baseURL, charset);
CSSParserContext parserContext(document, baseURL, charset);

auto cssSheet = CSSStyleSheet::create(StyleSheetContents::create(href, parserContext), *this);
cssSheet.get().setDisabled(m_alternate);
cssSheet.get().setTitle(m_title);
cssSheet.get().setMediaQueries(MQ::MediaQueryParser::parse(m_media, MediaQueryParserContext(document())));
Ref cssSheet = CSSStyleSheet::create(StyleSheetContents::create(href, parserContext), *this);
cssSheet->setDisabled(m_alternate);
cssSheet->setTitle(m_title);
cssSheet->setMediaQueries(MQ::MediaQueryParser::parse(m_media, MediaQueryParserContext(document)));

m_sheet = WTFMove(cssSheet);

// We don't need the cross-origin security check here because we are
// getting the sheet text in "strict" mode. This enforces a valid CSS MIME
// type.
Ref<Document> protect(document());
parseStyleSheet(sheet->sheetText());
}

Expand All @@ -229,31 +216,36 @@ void ProcessingInstruction::setXSLStyleSheet(const String& href, const URL& base
{
ASSERT(m_isXSL);
m_sheet = XSLStyleSheet::create(*this, href, baseURL);
Ref<Document> protect(document());
Ref protectedDocument { document() };
parseStyleSheet(sheet);
}
#endif

RefPtr<StyleSheet> ProcessingInstruction::protectedSheet() const
{
return m_sheet;
}

void ProcessingInstruction::parseStyleSheet(const String& sheet)
{
Ref styleSheet = *m_sheet;
if (m_isCSS)
downcast<CSSStyleSheet>(*m_sheet).contents().parseString(sheet);
downcast<CSSStyleSheet>(styleSheet.get()).protectedContents()->parseString(sheet);
#if ENABLE(XSLT)
else if (m_isXSL)
downcast<XSLStyleSheet>(*m_sheet).parseString(sheet);
downcast<XSLStyleSheet>(styleSheet.get()).parseString(sheet);
#endif

if (m_cachedSheet)
m_cachedSheet->removeClient(*this);
m_cachedSheet = nullptr;
if (CachedResourceHandle cachedSheet = std::exchange(m_cachedSheet, nullptr))
cachedSheet->removeClient(*this);

m_loading = false;

if (m_isCSS)
downcast<CSSStyleSheet>(*m_sheet).contents().checkLoaded();
downcast<CSSStyleSheet>(styleSheet.get()).protectedContents()->checkLoaded();
#if ENABLE(XSLT)
else if (m_isXSL)
downcast<XSLStyleSheet>(*m_sheet).checkLoaded();
downcast<XSLStyleSheet>(styleSheet.get()).checkLoaded();
#endif
}

Expand All @@ -270,7 +262,7 @@ Node::InsertedIntoAncestorResult ProcessingInstruction::insertedIntoAncestor(Ins
CharacterData::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (!insertionType.connectedToDocument)
return InsertedIntoAncestorResult::Done;
document().styleScope().addStyleSheetCandidateNode(*this, m_createdByParser);
protectedDocument()->checkedStyleScope()->addStyleSheetCandidateNode(*this, m_createdByParser);
return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
}

Expand All @@ -285,20 +277,20 @@ void ProcessingInstruction::removedFromAncestor(RemovalType removalType, Contain
if (!removalType.disconnectedFromDocument)
return;

document().styleScope().removeStyleSheetCandidateNode(*this);
CheckedRef styleScope = document().styleScope();
styleScope->removeStyleSheetCandidateNode(*this);

if (m_sheet) {
ASSERT(m_sheet->ownerNode() == this);
m_sheet->clearOwnerNode();
m_sheet = nullptr;
if (RefPtr sheet = std::exchange(m_sheet, nullptr)) {
ASSERT(sheet->ownerNode() == this);
sheet->clearOwnerNode();
}

if (m_loading) {
m_loading = false;
document().styleScope().removePendingSheet(*this);
styleScope->removePendingSheet(*this);
}

document().styleScope().didChangeActiveStyleSheetCandidates();
styleScope->didChangeActiveStyleSheetCandidates();
}

void ProcessingInstruction::finishParsingChildren()
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/dom/ProcessingInstruction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class ProcessingInstruction final : public CharacterData, private CachedStyleShe

const String& localHref() const { return m_localHref; }
StyleSheet* sheet() const { return m_sheet.get(); }
RefPtr<StyleSheet> protectedSheet() const;

bool isCSS() const { return m_isCSS; }
#if ENABLE(XSLT)
Expand Down Expand Up @@ -84,7 +85,7 @@ class ProcessingInstruction final : public CharacterData, private CachedStyleShe
String m_localHref;
String m_title;
String m_media;
CachedResourceHandle<CachedResource> m_cachedSheet { nullptr };
CachedResourceHandle<CachedResource> m_cachedSheet;
RefPtr<StyleSheet> m_sheet;
bool m_loading { false };
bool m_alternate { false };
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/dom/PseudoElement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ PseudoElement::~PseudoElement()

Ref<PseudoElement> PseudoElement::create(Element& host, PseudoId pseudoId)
{
auto pseudoElement = adoptRef(*new PseudoElement(host, pseudoId));
Ref pseudoElement = adoptRef(*new PseudoElement(host, pseudoId));

InspectorInstrumentation::pseudoElementCreated(host.document().page(), pseudoElement.get());
InspectorInstrumentation::pseudoElementCreated(host.document().checkedPage().get(), pseudoElement.get());

return pseudoElement;
}

void PseudoElement::clearHostElement()
{
InspectorInstrumentation::pseudoElementDestroyed(document().page(), *this);
InspectorInstrumentation::pseudoElementDestroyed(document().checkedPage().get(), *this);

Styleable::fromElement(*this).elementWasRemoved();

Expand Down
Loading

0 comments on commit 728c252

Please sign in to comment.