Diff
Modified: releases/WebKitGTK/webkit-2.28/LayoutTests/ChangeLog (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/LayoutTests/ChangeLog 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/LayoutTests/ChangeLog 2020-03-06 14:28:18 UTC (rev 257985)
@@ -1,3 +1,15 @@
+2020-02-28 Chris Dumez <cdu...@apple.com>
+
+ Garbage collection prevents FontFace.loaded promise from getting resolved
+ https://bugs.webkit.org/show_bug.cgi?id=208382
+
+ Reviewed by Ryosuke Niwa.
+
+ Add layout test coverage. Thanks to Alexey Proskuryakov for writing the test.
+
+ * fast/text/font-promises-gc-expected.txt: Added.
+ * fast/text/font-promises-gc.html: Added.
+
2020-02-27 Doug Kelly <do...@apple.com>
Hit test with clipPath referencing parent element causes infinite recursion
Added: releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc-expected.txt (0 => 257985)
--- releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc-expected.txt 2020-03-06 14:28:18 UTC (rev 257985)
@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: line 34: Font status: unloaded
+CONSOLE MESSAGE: line 36: Font loaded
+This test should not time out, and should say "SUCCESS".
+Hello
+SUCCESS
Added: releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc.html (0 => 257985)
--- releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc.html (rev 0)
+++ releases/WebKitGTK/webkit-2.28/LayoutTests/fast/text/font-promises-gc.html 2020-03-06 14:28:18 UTC (rev 257985)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "Ahem";
+ src: url("../../resources/Ahem.ttf");
+}
+#box {
+ font: 100px "Ahem";
+}
+</style>
+</head>
+<body>
+<pre>This test should not time out, and should say "SUCCESS".</pre>
+<div id="box">Hello</div>
+<div id="result">Waiting...</div>
+<script>
+
+function gc()
+{
+ if (typeof GCController !== "undefined")
+ GCController.collect();
+}
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+var promises = [];
+
+document.fonts.forEach(function(f) {
+ console.log("Font status: " + f.status);
+ promises.push(f.loaded);
+ f.loaded.then(function() { console.log("Font loaded"); });
+ gc();
+});
+
+setInterval(gc, 10);
+
+Promise.all(promises).then(function() {
+ document.getElementById("result").textContent = "SUCCESS";
+ if (window.testRunner)
+ testRunner.notifyDone();
+});
+
+</script>
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog 2020-03-06 14:28:18 UTC (rev 257985)
@@ -1,3 +1,29 @@
+2020-02-28 Chris Dumez <cdu...@apple.com>
+
+ Garbage collection prevents FontFace.loaded promise from getting resolved
+ https://bugs.webkit.org/show_bug.cgi?id=208382
+
+ Reviewed by Ryosuke Niwa.
+
+ Make sure the FontFace JS wrapper stays alive long enough to resolve the
+ loaded promise when it is observable by the page's script.
+
+ Test: fast/text/font-promises-gc.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::CSSFontFace::document const):
+ * css/CSSFontFace.h:
+ * css/FontFace.cpp:
+ (WebCore::FontFace::FontFace):
+ (WebCore::FontFace::fontStateChanged):
+ (WebCore::FontFace::loadForBindings):
+ (WebCore::FontFace::loadedForBindings):
+ (WebCore::FontFace::activeDOMObjectName const):
+ (WebCore::FontFace::hasPendingActivity const):
+ (WebCore::FontFace::load): Deleted.
+ * css/FontFace.h:
+ * css/FontFace.idl:
+
2020-02-28 Dean Jackson <d...@apple.com>
updateCSSTransitionsForElementAndProperty should clone RenderStyles
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.cpp (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.cpp 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.cpp 2020-03-06 14:28:18 UTC (rev 257985)
@@ -449,6 +449,11 @@
ASSERT(!m_sourcesPopulated);
}
+Document* CSSFontFace::document() const
+{
+ return m_fontSelector ? m_fontSelector->document() : nullptr;
+}
+
AllowUserInstalledFonts CSSFontFace::allowUserInstalledFonts() const
{
if (m_fontSelector && m_fontSelector->document())
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.h (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.h 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/css/CSSFontFace.h 2020-03-06 14:28:18 UTC (rev 257985)
@@ -165,6 +165,8 @@
#endif
void setErrorState();
+ Document* document() const;
+
private:
CSSFontFace(CSSFontSelector*, StyleRuleFontFace*, FontFace*, bool isLocalFallback);
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.cpp (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.cpp 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.cpp 2020-03-06 14:28:18 UTC (rev 257985)
@@ -29,6 +29,7 @@
#include "CSSComputedStyleDeclaration.h"
#include "CSSFontFaceSource.h"
#include "CSSFontFeatureValue.h"
+#include "CSSFontSelector.h"
#include "CSSFontStyleValue.h"
#include "CSSParser.h"
#include "CSSPrimitiveValueMappings.h"
@@ -63,6 +64,7 @@
Ref<FontFace> FontFace::create(Document& document, const String& family, Source&& source, const Descriptors& descriptors)
{
auto result = adoptRef(*new FontFace(document.fontSelector()));
+ result->suspendIfNeeded();
bool dataRequiresAsynchronousLoading = true;
@@ -140,11 +142,14 @@
Ref<FontFace> FontFace::create(CSSFontFace& face)
{
- return adoptRef(*new FontFace(face));
+ auto fontFace = adoptRef(*new FontFace(face));
+ fontFace->suspendIfNeeded();
+ return fontFace;
}
FontFace::FontFace(CSSFontSelector& fontSelector)
- : m_backing(CSSFontFace::create(&fontSelector, nullptr, this))
+ : ActiveDOMObject(fontSelector.document())
+ , m_backing(CSSFontFace::create(&fontSelector, nullptr, this))
, m_loadedPromise(makeUniqueRef<LoadedPromise>(*this, &FontFace::loadedPromiseResolve))
{
m_backing->addClient(*this);
@@ -151,7 +156,8 @@
}
FontFace::FontFace(CSSFontFace& face)
- : m_backing(face)
+ : ActiveDOMObject(face.document())
+ , m_backing(face)
, m_loadedPromise(makeUniqueRef<LoadedPromise>(*this, &FontFace::loadedPromiseResolve))
{
m_backing->addClient(*this);
@@ -436,8 +442,6 @@
ASSERT_UNUSED(face, &face == m_backing.ptr());
switch (newState) {
case CSSFontFace::Status::Loading:
- // We still need to resolve promises when loading completes, even if all references to use have fallen out of scope.
- ref();
break;
case CSSFontFace::Status::TimedOut:
break;
@@ -446,7 +450,6 @@
// gone through a load cycle, we can sometimes come back through here and try to resolve the promise again.
if (!m_loadedPromise->isFulfilled())
m_loadedPromise->resolve(*this);
- deref();
return;
case CSSFontFace::Status::Failure:
// FIXME: This check should not be needed, but because FontFace's are sometimes adopted after they have already
@@ -453,7 +456,6 @@
// gone through a load cycle, we can sometimes come back through here and try to resolve the promise again.
if (!m_loadedPromise->isFulfilled())
m_loadedPromise->reject(Exception { NetworkError });
- deref();
return;
case CSSFontFace::Status::Pending:
ASSERT_NOT_REACHED();
@@ -461,15 +463,34 @@
}
}
-auto FontFace::load() -> LoadedPromise&
+auto FontFace::loadForBindings() -> LoadedPromise&
{
+ m_mayLoadedPromiseBeScriptObservable = true;
m_backing->load();
return m_loadedPromise.get();
}
+auto FontFace::loadedForBindings() -> LoadedPromise&
+{
+ m_mayLoadedPromiseBeScriptObservable = true;
+ return m_loadedPromise.get();
+}
+
FontFace& FontFace::loadedPromiseResolve()
{
return *this;
}
+const char* FontFace::activeDOMObjectName() const
+{
+ return "FontFace";
}
+
+bool FontFace::hasPendingActivity() const
+{
+ if (ActiveDOMObject::hasPendingActivity())
+ return true;
+ return !isContextStopped() && m_mayLoadedPromiseBeScriptObservable && !m_loadedPromise->isFulfilled();
+}
+
+}
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.h (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.h 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.h 2020-03-06 14:28:18 UTC (rev 257985)
@@ -25,6 +25,7 @@
#pragma once
+#include "ActiveDOMObject.h"
#include "CSSFontFace.h"
#include "CSSPropertyNames.h"
#include "IDLTypes.h"
@@ -41,7 +42,7 @@
template<typename IDLType> class DOMPromiseProxyWithResolveCallback;
-class FontFace final : public RefCounted<FontFace>, public CanMakeWeakPtr<FontFace>, private CSSFontFace::Client {
+class FontFace final : public RefCounted<FontFace>, public CanMakeWeakPtr<FontFace>, public ActiveDOMObject, private CSSFontFace::Client {
public:
struct Descriptors {
String style;
@@ -77,8 +78,8 @@
LoadStatus status() const;
using LoadedPromise = DOMPromiseProxyWithResolveCallback<IDLInterface<FontFace>>;
- LoadedPromise& loaded() { return m_loadedPromise.get(); }
- LoadedPromise& load();
+ LoadedPromise& loadedForBindings();
+ LoadedPromise& loadForBindings();
void adopt(CSSFontFace&);
@@ -91,14 +92,20 @@
void ref() final { RefCounted::ref(); }
void deref() final { RefCounted::deref(); }
+ bool hasPendingActivity() const final;
+
private:
explicit FontFace(CSSFontSelector&);
explicit FontFace(CSSFontFace&);
+
+ const char* activeDOMObjectName() const final;
+
// Callback for LoadedPromise.
FontFace& loadedPromiseResolve();
void setErrorState();
Ref<CSSFontFace> m_backing;
UniqueRef<LoadedPromise> m_loadedPromise;
+ bool m_mayLoadedPromiseBeScriptObservable { false };
};
}
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.idl (257984 => 257985)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.idl 2020-03-06 14:28:11 UTC (rev 257984)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/css/FontFace.idl 2020-03-06 14:28:18 UTC (rev 257985)
@@ -42,6 +42,7 @@
};
[
+ ActiveDOMObject,
ConstructorCallWith=Document,
Constructor(DOMString family, (DOMString or BinaryData) source, optional FontFaceDescriptors descriptors)
] interface FontFace {
@@ -55,6 +56,6 @@
readonly attribute FontFaceLoadStatus status;
- [ReturnsOwnPromise, PromiseProxy] Promise<FontFace> load();
- readonly attribute Promise<FontFace> loaded;
+ [ReturnsOwnPromise, PromiseProxy, ImplementedAs=loadForBindings] Promise<FontFace> load();
+ [ImplementedAs=loadedForBindings] readonly attribute Promise<FontFace> loaded;
};
