Skip to content

Commit

Permalink
Deploy more smart pointers in ExtensionStyleSheets.cpp, FragmentDirec…
Browse files Browse the repository at this point in the history 
…tiveRangeFinder.cpp,

FullscreenManager.cpp, IdleDeadline.cpp, and ImageOverlay.cpp
https://bugs.webkit.org/show_bug.cgi?id=264797

Reviewed by Chris Dumez.

Deployed more smart pointers as warned by the clang static analyzer.

* Source/WebCore/dom/ExtensionStyleSheets.cpp:
(WebCore::ExtensionStyleSheets::pageUserSheet):
(WebCore::ExtensionStyleSheets::clearPageUserSheet):
(WebCore::ExtensionStyleSheets::updatePageUserSheet):
(WebCore::ExtensionStyleSheets::updateInjectedStyleSheetCache const):
(WebCore::ExtensionStyleSheets::invalidateInjectedStyleSheetCache):
(WebCore::ExtensionStyleSheets::addUserStyleSheet):
(WebCore::ExtensionStyleSheets::addAuthorStyleSheetForTesting):
(WebCore::ExtensionStyleSheets::addDisplayNoneSelector):
(WebCore::ExtensionStyleSheets::maybeAddContentExtensionSheet):
* Source/WebCore/dom/ExtensionStyleSheets.h:
* Source/WebCore/dom/FragmentDirectiveRangeFinder.cpp:
(WebCore::FragmentDirectiveRangeFinder::rangeOfStringInRange):
(WebCore::FragmentDirectiveRangeFinder::advanceRangeStartToNextNonWhitespace):
* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::protectedTopDocument):
(WebCore::FullscreenManager::finishExitFullscreen):
(WebCore::FullscreenManager::didExitFullscreen):
* Source/WebCore/dom/FullscreenManager.h:
* Source/WebCore/dom/IdleDeadline.cpp:
(WebCore::IdleDeadline::timeRemaining const):
* Source/WebCore/dom/ImageOverlay.cpp:
(WebCore::ImageOverlay::installImageOverlayStyleSheet):
(WebCore::ImageOverlay::updateSubtree):

Canonical link: https://commits.webkit.org/270750@main
  • Loading branch information
@rniwa
rniwa committed Nov 15, 2023
1 parent 48c502b commit e2b769d
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 35 deletions.
39 changes: 22 additions & 17 deletions Source/WebCore/dom/ExtensionStyleSheets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ ExtensionStyleSheets::ExtensionStyleSheets(Document& document)
{
}

Ref<Document> ExtensionStyleSheets::protectedDocument() const
{
return const_cast<Document&>(m_document.get());
}

static Ref<CSSStyleSheet> createExtensionsStyleSheet(Document& document, URL url, const String& text, UserStyleLevel level)
{
auto contents = StyleSheetContents::create(url.string(), CSSParserContext(document, url));
Expand All @@ -73,15 +78,15 @@ CSSStyleSheet* ExtensionStyleSheets::pageUserSheet()
if (m_pageUserSheet)
return m_pageUserSheet.get();

Page* owningPage = m_document.page();
Page* owningPage = m_document->page();
if (!owningPage)
return 0;

String userSheetText = owningPage->userStyleSheet();
if (userSheetText.isEmpty())
return 0;

m_pageUserSheet = createExtensionsStyleSheet(m_document, m_document.settings().userStyleSheetLocation(), userSheetText, UserStyleLevel::User);
m_pageUserSheet = createExtensionsStyleSheet(protectedDocument().get(), m_document->settings().userStyleSheetLocation(), userSheetText, UserStyleLevel::User);

return m_pageUserSheet.get();
}
Expand All @@ -90,15 +95,15 @@ void ExtensionStyleSheets::clearPageUserSheet()
{
if (m_pageUserSheet) {
m_pageUserSheet = nullptr;
m_document.styleScope().didChangeStyleSheetEnvironment();
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}
}

void ExtensionStyleSheets::updatePageUserSheet()
{
clearPageUserSheet();
if (pageUserSheet())
m_document.styleScope().didChangeStyleSheetEnvironment();
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}

const Vector<RefPtr<CSSStyleSheet>>& ExtensionStyleSheets::injectedUserStyleSheets() const
Expand All @@ -123,12 +128,12 @@ void ExtensionStyleSheets::updateInjectedStyleSheetCache() const
m_injectedAuthorStyleSheets.clear();
m_injectedStyleSheetToSource.clear();

Page* owningPage = m_document.page();
CheckedPtr owningPage = m_document->page();
if (!owningPage)
return;

auto addStyleSheet = [&](const UserStyleSheet& userStyleSheet) {
auto sheet = createExtensionsStyleSheet(const_cast<Document&>(m_document), userStyleSheet.url(), userStyleSheet.source(), userStyleSheet.level());
Ref sheet = createExtensionsStyleSheet(protectedDocument().get(), userStyleSheet.url(), userStyleSheet.source(), userStyleSheet.level());

m_injectedStyleSheetToSource.set(sheet.copyRef(), userStyleSheet.source());

Expand All @@ -145,10 +150,10 @@ void ExtensionStyleSheets::updateInjectedStyleSheetCache() const
if (userStyleSheet.pageID())
return;

if (userStyleSheet.injectedFrames() == UserContentInjectedFrames::InjectInTopFrameOnly && m_document.ownerElement())
if (userStyleSheet.injectedFrames() == UserContentInjectedFrames::InjectInTopFrameOnly && m_document->ownerElement())
return;

if (!UserContentURLPattern::matchesPatterns(m_document.url(), userStyleSheet.allowlist(), userStyleSheet.blocklist()))
if (!UserContentURLPattern::matchesPatterns(m_document->url(), userStyleSheet.allowlist(), userStyleSheet.blocklist()))
return;

addStyleSheet(userStyleSheet);
Expand All @@ -174,34 +179,34 @@ void ExtensionStyleSheets::removePageSpecificUserStyleSheet(const UserStyleSheet
void ExtensionStyleSheets::invalidateInjectedStyleSheetCache()
{
m_injectedStyleSheetCacheValid = false;
m_document.styleScope().didChangeStyleSheetEnvironment();
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}

void ExtensionStyleSheets::addUserStyleSheet(Ref<StyleSheetContents>&& userSheet)
{
ASSERT(userSheet.get().isUserStyleSheet());
m_userStyleSheets.append(CSSStyleSheet::create(WTFMove(userSheet), m_document));
m_document.styleScope().didChangeStyleSheetEnvironment();
m_userStyleSheets.append(CSSStyleSheet::create(WTFMove(userSheet), protectedDocument().get()));
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}

void ExtensionStyleSheets::addAuthorStyleSheetForTesting(Ref<StyleSheetContents>&& authorSheet)
{
ASSERT(!authorSheet.get().isUserStyleSheet());
m_authorStyleSheetsForTesting.append(CSSStyleSheet::create(WTFMove(authorSheet), m_document));
m_document.styleScope().didChangeStyleSheetEnvironment();
m_authorStyleSheetsForTesting.append(CSSStyleSheet::create(WTFMove(authorSheet), protectedDocument().get()));
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}

#if ENABLE(CONTENT_EXTENSIONS)
void ExtensionStyleSheets::addDisplayNoneSelector(const String& identifier, const String& selector, uint32_t selectorID)
{
auto result = m_contentExtensionSelectorSheets.add(identifier, nullptr);
if (result.isNewEntry) {
result.iterator->value = ContentExtensionStyleSheet::create(m_document);
result.iterator->value = ContentExtensionStyleSheet::create(protectedDocument().get());
m_userStyleSheets.append(&result.iterator->value->styleSheet());
}

if (result.iterator->value->addDisplayNoneSelector(selector, selectorID))
m_document.styleScope().didChangeStyleSheetEnvironment();
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();
}

void ExtensionStyleSheets::maybeAddContentExtensionSheet(const String& identifier, StyleSheetContents& sheet)
Expand All @@ -211,10 +216,10 @@ void ExtensionStyleSheets::maybeAddContentExtensionSheet(const String& identifie
if (m_contentExtensionSheets.contains(identifier))
return;

Ref<CSSStyleSheet> cssSheet = CSSStyleSheet::create(sheet, m_document);
Ref<CSSStyleSheet> cssSheet = CSSStyleSheet::create(sheet, protectedDocument().get());
m_contentExtensionSheets.set(identifier, &cssSheet.get());
m_userStyleSheets.append(adoptRef(cssSheet.leakRef()));
m_document.styleScope().didChangeStyleSheetEnvironment();
protectedDocument()->checkedStyleScope()->didChangeStyleSheetEnvironment();

}
#endif // ENABLE(CONTENT_EXTENSIONS)
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/dom/ExtensionStyleSheets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ class ExtensionStyleSheets {
void detachFromDocument();

private:
Document& m_document;
Ref<Document> protectedDocument() const;

CheckedRef<Document> m_document;

RefPtr<CSSStyleSheet> m_pageUserSheet;

Expand Down
16 changes: 8 additions & 8 deletions Source/WebCore/dom/FragmentDirectiveRangeFinder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ static std::optional<SimpleRange> rangeOfStringInRange(const String& query, Simp
} while (is<DocumentType>(currentNode));
continue;
}
auto& blockAncestor = nearestBlockAncestor(*currentNode);

Ref blockAncestor = nearestBlockAncestor(*currentNode);
Vector<Ref<Text>> textNodeList;
// FIXME: this is O^2 since treeOrder will also do traversal, optimize.
while (currentNode && currentNode->isDescendantOf(blockAncestor) && is_lteq(treeOrder(BoundaryPoint(*currentNode, 0), searchRange.end))) {
Expand Down Expand Up @@ -267,13 +267,13 @@ static std::optional<SimpleRange> advanceRangeStartToNextNonWhitespace(SimpleRan
{
auto newRange = range;
while (!newRange.collapsed()) {
auto& node = newRange.startContainer();
Ref node = newRange.startContainer();
auto offset = newRange.startOffset();

// This check is not in the spec.
// I believe there is an error in the spec which I have filed an issue for
// https://github.com/WICG/scroll-to-text-fragment/issues/189
if (offset == node.length()) {
if (offset == node->length()) {
if (auto newStart = NodeTraversal::next(node)) {
newRange.start = { *newStart, 0 };
continue;
Expand All @@ -297,7 +297,7 @@ static std::optional<SimpleRange> advanceRangeStartToNextNonWhitespace(SimpleRan
continue;
}

auto string = node.textContent();
auto string = node->textContent();

if (string.substringSharingImpl(offset, 6) == "&nbsp;"_s)
offset += 6;
Expand All @@ -308,14 +308,14 @@ static std::optional<SimpleRange> advanceRangeStartToNextNonWhitespace(SimpleRan
if (!isUnicodeWhitespace(string[offset]))
return newRange;
offset++;
if (offset >= node.length()) {

if (offset >= node->length()) {
if (auto newStart = NodeTraversal::next(node))
newRange.start = { *newStart, 0 };
else
return newRange;
} else
newRange.start = { node, offset };
newRange.start = { node.get(), offset };
}
return newRange;
}
Expand Down
11 changes: 8 additions & 3 deletions Source/WebCore/dom/FullscreenManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ Element* FullscreenManager::fullscreenElement() const
return nullptr;
}

Ref<Document> FullscreenManager::protectedTopDocument()
{
return topDocument();
}

// https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen
void FullscreenManager::requestFullscreenForElement(Ref<Element>&& element, RefPtr<DeferredPromise>&& promise, FullscreenCheckType checkType)
{
Expand Down Expand Up @@ -414,8 +419,8 @@ void FullscreenManager::finishExitFullscreen(Document& currentDocument, ExitMode

// Let descendantDocs be an ordered set consisting of doc’s descendant browsing contexts' active documents whose fullscreen element is non-null, if any, in tree order.
Deque<Ref<Document>> descendantDocuments;
for (auto* descendant = currentDocument.frame() ? currentDocument.frame()->tree().traverseNext() : nullptr; descendant; descendant = descendant->tree().traverseNext()) {
auto* localFrame = dynamicDowncast<LocalFrame>(descendant);
for (RefPtr descendant = currentDocument.frame() ? currentDocument.frame()->tree().traverseNext() : nullptr; descendant; descendant = descendant->tree().traverseNext()) {
RefPtr localFrame = dynamicDowncast<LocalFrame>(descendant);
if (!localFrame || !localFrame->document())
continue;
if (localFrame->document()->fullscreenManager().fullscreenElement())
Expand Down Expand Up @@ -607,7 +612,7 @@ bool FullscreenManager::didExitFullscreen()
}
INFO_LOG(LOGIDENTIFIER);

finishExitFullscreen(topDocument(), ExitMode::Resize);
finishExitFullscreen(protectedTopDocument(), ExitMode::Resize);

if (m_fullscreenElement)
m_fullscreenElement->didStopBeingFullscreenElement();
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/FullscreenManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class FullscreenManager final : public CanMakeWeakPtr<FullscreenManager>, public
#endif

Document& topDocument() { return m_topDocument ? *m_topDocument : document().topDocument(); }
Ref<Document> protectedTopDocument();

CheckedRef<Document> m_document;
WeakPtr<Document, WeakPtrImplWithEventTargetData> m_topDocument;
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/dom/IdleDeadline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ DOMHighResTimeStamp IdleDeadline::timeRemaining(Document& document) const
RefPtr window { document.domWindow() };
if (!window || m_didTimeout == DidTimeout::Yes)
return 0;
auto& performance = window->performance();
auto now = performance.now();
auto deadline = performance.relativeTimeFromTimeOriginInReducedResolution(document.windowEventLoop().computeIdleDeadline() - performance.timeResolution());
Ref performance = window->performance();
auto now = performance->now();
auto deadline = performance->relativeTimeFromTimeOriginInReducedResolution(document.windowEventLoop().computeIdleDeadline() - performance->timeResolution());
return deadline < now ? 0 : deadline - now;
}

Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/dom/ImageOverlay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ IntRect containerRect(HTMLElement& element)
static void installImageOverlayStyleSheet(ShadowRoot& shadowRoot)
{
static MainThreadNeverDestroyed<const String> shadowStyle(StringImpl::createWithoutCopying(imageOverlayUserAgentStyleSheet, sizeof(imageOverlayUserAgentStyleSheet)));
Ref style = HTMLStyleElement::create(HTMLNames::styleTag, shadowRoot.document(), false);
Ref style = HTMLStyleElement::create(HTMLNames::styleTag, shadowRoot.protectedDocument(), false);
style->setTextContent(String { shadowStyle });
shadowRoot.appendChild(WTFMove(style));
}
Expand Down Expand Up @@ -406,8 +406,9 @@ static Elements updateSubtree(HTMLElement& element, const TextRecognitionResult&
}

if (line.hasTrailingNewline) {
lineElements.lineBreak = HTMLBRElement::create(document.get());
lineContainer->appendChild(*lineElements.lineBreak);
Ref lineBreak = HTMLBRElement::create(document.get());
lineContainer->appendChild(lineBreak.get());
lineElements.lineBreak = WTFMove(lineBreak);
}

elements.lines.append(WTFMove(lineElements));
Expand Down

0 comments on commit e2b769d

Please sign in to comment.