Address review feedback from https://bugs.webkit.org/show_bug.cgi?id=175246.
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 03:24:31 +0000 (03:24 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 03:24:31 +0000 (03:24 +0000)
* bindings/js/DOMPromiseProxy.h:
* css/FontFaceSet.cpp:
* css/FontFaceSet.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220440 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/DOMPromiseProxy.h
Source/WebCore/css/FontFaceSet.cpp
Source/WebCore/css/FontFaceSet.h

index b8f5d0d..f075404 100644 (file)
@@ -1,3 +1,11 @@
+2017-08-08  Sam Weinig  <sam@webkit.org>
+
+        Address review feedback from https://bugs.webkit.org/show_bug.cgi?id=175246.
+
+        * bindings/js/DOMPromiseProxy.h:
+        * css/FontFaceSet.cpp:
+        * css/FontFaceSet.h:
+
 2017-08-08  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Canvas: support editing WebGL shaders
index 6cdd07e..de89ee8 100644 (file)
@@ -46,7 +46,7 @@ public:
 
     JSC::JSValue promise(JSC::ExecState&, JSDOMGlobalObject&);
 
-    void reset();
+    void clear();
 
     bool isFulfilled() const;
 
@@ -70,7 +70,7 @@ public:
 
     JSC::JSValue promise(JSC::ExecState&, JSDOMGlobalObject&);
 
-    void reset();
+    void clear();
 
     bool isFulfilled() const;
 
@@ -82,6 +82,10 @@ private:
     Vector<Ref<DeferredPromise>, 1> m_deferredPromises;
 };
 
+// Instead of storing the value of the resolution directly, DOMPromiseProxyWithResolveCallback
+// allows the owner to specify callback to be called when the resolved value is needed. This is
+// needed to avoid reference cycles when the resolved value is the owner, such as is the case with
+// FontFace and FontFaceSet.
 template<typename IDLType>
 class DOMPromiseProxyWithResolveCallback {
 public:
@@ -99,7 +103,7 @@ public:
 
     JSC::JSValue promise(JSC::ExecState&, JSDOMGlobalObject&);
 
-    void reset();
+    void clear();
 
     bool isFulfilled() const;
 
@@ -143,7 +147,7 @@ inline JSC::JSValue DOMPromiseProxy<IDLType>::promise(JSC::ExecState& state, JSD
 }
 
 template<typename IDLType>
-inline void DOMPromiseProxy<IDLType>::reset()
+inline void DOMPromiseProxy<IDLType>::clear()
 {
     m_valueOrException = std::nullopt;
     m_deferredPromises.clear();
@@ -212,7 +216,7 @@ inline JSC::JSValue DOMPromiseProxy<IDLVoid>::promise(JSC::ExecState& state, JSD
     return result;
 }
 
-inline void DOMPromiseProxy<IDLVoid>::reset()
+inline void DOMPromiseProxy<IDLVoid>::clear()
 {
     m_valueOrException = std::nullopt;
     m_deferredPromises.clear();
@@ -280,7 +284,7 @@ inline JSC::JSValue DOMPromiseProxyWithResolveCallback<IDLType>::promise(JSC::Ex
 }
 
 template<typename IDLType>
-inline void DOMPromiseProxyWithResolveCallback<IDLType>::reset()
+inline void DOMPromiseProxyWithResolveCallback<IDLType>::clear()
 {
     m_valueOrException = std::nullopt;
     m_deferredPromises.clear();
index efaaaed..25f8763 100644 (file)
@@ -189,14 +189,12 @@ bool FontFaceSet::canSuspendForDocumentSuspension() const
 void FontFaceSet::startedLoading()
 {
     // FIXME: Fire a "loading" event asynchronously.
-    m_isReady = false;
-    m_readyPromise.reset();
+    m_readyPromise.clear();
 }
 
 void FontFaceSet::completedLoading()
 {
     m_readyPromise.resolve(*this);
-    m_isReady = true;
 }
 
 void FontFaceSet::faceFinished(CSSFontFace& face, CSSFontFace::Status newStatus)
index b3170dd..ffcb8e6 100644 (file)
@@ -113,7 +113,6 @@ private:
     Ref<CSSFontFaceSet> m_backing;
     HashMap<RefPtr<FontFace>, Vector<Ref<PendingPromise>>> m_pendingPromises;
     ReadyPromise m_readyPromise;
-    bool m_isReady { true };
 };
 
 }