WeakPtr breaks vtables when upcasting to base classes
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 May 2019 21:15:54 +0000 (21:15 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 May 2019 21:15:54 +0000 (21:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188799

Reviewed by Youenn Fablet.

Source/WebCore:

* Modules/encryptedmedia/MediaKeySession.cpp:
(WebCore::MediaKeySession::MediaKeySession):
* Modules/encryptedmedia/MediaKeySession.h: Adopted modern WeakPtr APIs.
Removed redundant WeakPtrFactory.

* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::existingWrapper):
* css/CSSFontFace.h: Moved functions out of line to avoid #include
explosion for .get().

* dom/ContainerNode.h:
* dom/Document.h:
* dom/Element.h: Moved CanMakeWeakPtr to ContainerNode because all
subclasses except for DocumentFragment were already so, and we have
code that uses WeakPtr<ContainerNode>, which, now that WeakPtr is
type-safe, is awkward to do when ContainerNode isn't CanMakeWeakPtr.

* dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::fullscreenRenderer const):
* dom/FullscreenManager.h:
(WebCore::FullscreenManager::fullscreenRenderer const): Deleted.
* html/FormAssociatedElement.cpp:
(WebCore::FormAssociatedElement::form const):
* html/FormAssociatedElement.h:
(WebCore::FormAssociatedElement::form const): Deleted. Moved functions
out of line to avoid #include explosion for .get().

* html/HTMLMediaElement.h: It takes an extra using declaration
to disambiguate multiple CanMakeWeakPtr base classes now.

* loader/MediaResourceLoader.cpp:
(WebCore::MediaResourceLoader::requestResource): Removed redundant .get().

* page/DOMWindowProperty.cpp:
(WebCore::DOMWindowProperty::window const):
* page/DOMWindowProperty.h:
(WebCore::DOMWindowProperty::window const): Deleted.
* page/FrameViewLayoutContext.cpp:
(WebCore::FrameViewLayoutContext::subtreeLayoutRoot const):
* page/FrameViewLayoutContext.h:
(WebCore::FrameViewLayoutContext::subtreeLayoutRoot const): Deleted.
* page/UndoItem.cpp:
(WebCore::UndoItem::undoManager const):
* page/UndoItem.h:
(WebCore::UndoItem::undoManager const): Deleted. Moved functions out of
line to avoid #include explosion for .get().

* platform/ScrollView.h: It takes an extra using declaration
to disambiguate multiple CanMakeWeakPtr base classes now.

* platform/Widget.cpp:
(WebCore::Widget::parent const):
* platform/Widget.h:
(WebCore::Widget::parent const): Deleted. Moved functions out of line to avoid #include
explosion for .get().

* platform/encryptedmedia/CDMInstanceSession.h: Made
CDMInstanceSessionClient CanMakeWeakPtr because we use WeakPtr<CDMInstanceSessionClient>.

* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
CanMakeWeakPtr is inherited now.

* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
(WebCore::MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC):
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::cdmSession const): Deleted.
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::createWeakPtr): Deleted.
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::CMTimebaseEffectiveRateChangedCallback):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::durationChanged):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::cdmSession const):
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC):
(WebCore::SourceBufferPrivateAVFObjC::trackDidChangeEnabled):
(WebCore::SourceBufferPrivateAVFObjC::setCDMSession):
(WebCore::SourceBufferPrivateAVFObjC::flushVideo):
(WebCore::SourceBufferPrivateAVFObjC::enqueueSample):
(WebCore::SourceBufferPrivateAVFObjC::notifyClientWhenReadyForMoreSamples):
(WebCore::SourceBufferPrivateAVFObjC::setVideoLayer):
(WebCore::SourceBufferPrivateAVFObjC::setDecompressionSession): Modernized WeakPtr API usage.

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::multiColumnFlowSlowCase const):
* rendering/RenderBlockFlow.h:
(WebCore::RenderBlockFlow::multiColumnFlow const):
* rendering/RenderMultiColumnFlow.cpp:
(WebCore::RenderMultiColumnFlow::findColumnSpannerPlaceholder const):
* rendering/RenderMultiColumnFlow.h:
* rendering/RenderTable.cpp:
(WebCore::RenderTable::header const):
(WebCore::RenderTable::footer const):
(WebCore::RenderTable::firstBody const):
(WebCore::RenderTable::topSection const):
* rendering/RenderTable.h:
(WebCore::RenderTable::header const): Deleted.
(WebCore::RenderTable::footer const): Deleted.
(WebCore::RenderTable::firstBody const): Deleted.
(WebCore::RenderTable::topSection const): Deleted. Moved functions out
of line to avoid #include explosion for .get().

Source/WebKit:

* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::networkSession):
* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
* Shared/WebBackForwardListItem.cpp:
(WebKit::WebBackForwardListItem::suspendedPage const):
* Shared/WebBackForwardListItem.h:
(WebKit::WebBackForwardListItem::suspendedPage const): Deleted. Moved
functions out of line to avoid #include explosion for .get().

* UIProcess/Authentication/cocoa/SecKeyProxyStore.h:
(WebKit::SecKeyProxyStore::get const):
(WebKit::SecKeyProxyStore::weakPtrFactory const): Deleted. Adopted
CanMakeWeakPtr.

* UIProcess/WebAuthentication/AuthenticatorManager.h:
* UIProcess/WebProcessProxy.cpp: It takes an extra using declaration
to disambiguate multiple CanMakeWeakPtr base classes now.

(WebKit::WebProcessProxy::processPool const):
* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::processPool const): Deleted. Moved
functions out of line to avoid #include explosion for .get().

Source/WTF:

This patch switches from reinterpret_cast to static_cast when loading
from WeakReference storage.

We know which type to cast *to* because it's specified by the type of
the calling WeakPtr.

We know which type to cast *from* because it's specified by a typedef
in CanMakeWeakPtr.

(Our convention is that we store a pointer to the class that derives
from CanMakeWeakPtr. We cast from that pointer to derived pointers when
we get(). This means that #include of the derived type header is now
required in order to get() the pointer.)

* wtf/WeakHashSet.h:
(WTF::HashTraits<Ref<WeakReference>>::isReleasedWeakValue): Definition
is now eagerly required because WeakReference is not a template anymore.

(WTF::WeakHashSet::WeakHashSetConstIterator::get const):
(WTF::WeakHashSet::WeakHashSetConstIterator::skipEmptyBuckets):
(WTF::WeakHashSet::remove):
(WTF::WeakHashSet::contains const):
(WTF::WeakHashSet::computesEmpty const):
(WTF::WeakHashSet::hasNullReferences const):
(WTF::WeakHashSet::computeSize const):
(WTF::HashTraits<Ref<WeakReference<T>>>::isReleasedWeakValue): Deleted.
Updated for new WeakReference get() API.

* wtf/WeakPtr.h: Use a macro for TestAPI support. We can't use template
specialization because WeakReference is not a class template anymore.
(Or maybe we could have kept it working with a dummy template argument?
Felt weird, so I switched.)

(WTF::WeakReference::create):
(WTF::WeakReference::~WeakReference):
(WTF::WeakReference::get const):
(WTF::WeakReference::operator bool const):
(WTF::WeakReference::WeakReference): WeakReference is just a void* now.
It's the caller's responsibility, when creating and getting, to use
a consistent storage type. We ensure a canonical storage type using a
typedef inside CanMakeWeakPtr.

(WTF::WeakPtr::WeakPtr):
(WTF::WeakPtr::get const):
(WTF::WeakPtr::operator bool const):
(WTF::WeakPtr::operator-> const):
(WTF::WeakPtr::operator* const): Adopted new WeakReference API.

(WTF::WeakPtrFactory::createWeakPtr const): No need for reinterpret_cast.

(WTF::weak_reference_cast): This isn't required for correctness, but it's
nice to show a complier error at WeakPtr construction sites when you know
that the types won't work. Otherwise, you get compiler errors at
dereference sites, which are slightly more mysterious ways of saying that
you constructed your WeakPtr incorrectly.

(WTF::WeakPtr<T>::WeakPtr):
(WTF::=):
(WTF::makeWeakPtr):
(WTF::weak_reference_upcast): Deleted.
(WTF::weak_reference_downcast): Deleted.

Tools:

* TestWebKitAPI/Tests/WTF/WeakPtr.cpp: Adopt the new macro API instead
of template specialization for observing weak references.

(TestWebKitAPI::Int::Int):
(TestWebKitAPI::Int::operator int const):
(TestWebKitAPI::Int::operator== const): Use a class for integer tests
because WeakPtr doesn't naturally support pointing to non-class objects
now.

(TestWebKitAPI::Base::foo):
(TestWebKitAPI::Derived::foo): Inherit from CanMakeWeakPtr to enable
deduction of the weak pointer type.

(TestWebKitAPI::TEST): Updated to use Int.

(TestWebKitAPI::Base::weakPtrFactory const): Deleted.
(WTF::WeakReference<TestWebKitAPI::Base>::WeakReference): Deleted.
(WTF::WeakReference<TestWebKitAPI::Base>::~WeakReference): Deleted.

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

53 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/WeakHashSet.h
Source/WTF/wtf/WeakPtr.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/encryptedmedia/MediaKeySession.cpp
Source/WebCore/Modules/encryptedmedia/MediaKeySession.h
Source/WebCore/css/CSSFontFace.cpp
Source/WebCore/css/CSSFontFace.h
Source/WebCore/css/parser/CSSDeferredParser.cpp
Source/WebCore/css/parser/CSSDeferredParser.h
Source/WebCore/dom/ContainerNode.h
Source/WebCore/dom/Document.h
Source/WebCore/dom/Element.h
Source/WebCore/dom/FullscreenManager.cpp
Source/WebCore/dom/FullscreenManager.h
Source/WebCore/html/FormAssociatedElement.cpp
Source/WebCore/html/FormAssociatedElement.h
Source/WebCore/html/HTMLMediaElement.h
Source/WebCore/loader/MediaResourceLoader.cpp
Source/WebCore/page/DOMWindowProperty.cpp
Source/WebCore/page/DOMWindowProperty.h
Source/WebCore/page/FrameViewLayoutContext.cpp
Source/WebCore/page/FrameViewLayoutContext.h
Source/WebCore/page/UndoItem.cpp
Source/WebCore/page/UndoItem.h
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/Widget.cpp
Source/WebCore/platform/Widget.h
Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/rendering/RenderMultiColumnFlow.cpp
Source/WebCore/rendering/RenderMultiColumnFlow.h
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTable.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
Source/WebKit/Shared/WebBackForwardListItem.cpp
Source/WebKit/Shared/WebBackForwardListItem.h
Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp
Source/WebKit/UIProcess/Authentication/cocoa/SecKeyProxyStore.h
Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp

index 810e18d..c66e793 100644 (file)
@@ -1,3 +1,72 @@
+2019-05-29  Geoffrey Garen  <ggaren@apple.com>
+
+        WeakPtr breaks vtables when upcasting to base classes
+        https://bugs.webkit.org/show_bug.cgi?id=188799
+
+        Reviewed by Youenn Fablet.
+
+        This patch switches from reinterpret_cast to static_cast when loading
+        from WeakReference storage.
+
+        We know which type to cast *to* because it's specified by the type of
+        the calling WeakPtr.
+
+        We know which type to cast *from* because it's specified by a typedef
+        in CanMakeWeakPtr.
+
+        (Our convention is that we store a pointer to the class that derives
+        from CanMakeWeakPtr. We cast from that pointer to derived pointers when
+        we get(). This means that #include of the derived type header is now
+        required in order to get() the pointer.)
+
+        * wtf/WeakHashSet.h:
+        (WTF::HashTraits<Ref<WeakReference>>::isReleasedWeakValue): Definition
+        is now eagerly required because WeakReference is not a template anymore.
+
+        (WTF::WeakHashSet::WeakHashSetConstIterator::get const):
+        (WTF::WeakHashSet::WeakHashSetConstIterator::skipEmptyBuckets):
+        (WTF::WeakHashSet::remove):
+        (WTF::WeakHashSet::contains const):
+        (WTF::WeakHashSet::computesEmpty const):
+        (WTF::WeakHashSet::hasNullReferences const):
+        (WTF::WeakHashSet::computeSize const):
+        (WTF::HashTraits<Ref<WeakReference<T>>>::isReleasedWeakValue): Deleted.
+        Updated for new WeakReference get() API.
+
+        * wtf/WeakPtr.h: Use a macro for TestAPI support. We can't use template
+        specialization because WeakReference is not a class template anymore.
+        (Or maybe we could have kept it working with a dummy template argument?
+        Felt weird, so I switched.)
+
+        (WTF::WeakReference::create):
+        (WTF::WeakReference::~WeakReference):
+        (WTF::WeakReference::get const):
+        (WTF::WeakReference::operator bool const):
+        (WTF::WeakReference::WeakReference): WeakReference is just a void* now.
+        It's the caller's responsibility, when creating and getting, to use
+        a consistent storage type. We ensure a canonical storage type using a
+        typedef inside CanMakeWeakPtr.
+
+        (WTF::WeakPtr::WeakPtr):
+        (WTF::WeakPtr::get const):
+        (WTF::WeakPtr::operator bool const):
+        (WTF::WeakPtr::operator-> const):
+        (WTF::WeakPtr::operator* const): Adopted new WeakReference API.
+
+        (WTF::WeakPtrFactory::createWeakPtr const): No need for reinterpret_cast.
+
+        (WTF::weak_reference_cast): This isn't required for correctness, but it's
+        nice to show a complier error at WeakPtr construction sites when you know
+        that the types won't work. Otherwise, you get compiler errors at
+        dereference sites, which are slightly more mysterious ways of saying that
+        you constructed your WeakPtr incorrectly.
+
+        (WTF::WeakPtr<T>::WeakPtr):
+        (WTF::=):
+        (WTF::makeWeakPtr):
+        (WTF::weak_reference_upcast): Deleted.
+        (WTF::weak_reference_downcast): Deleted.
+
 2019-05-29  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r245857.
index 092011c..0bba1e0 100644 (file)
 
 namespace WTF {
 
+template<> struct HashTraits<Ref<WeakReference>> : RefHashTraits<WeakReference> {
+    static const bool hasIsReleasedWeakValueFunction = true;
+    static bool isReleasedWeakValue(const Ref<WeakReference>& value)
+    {
+        return !value.isHashTableDeletedValue() && !value.isHashTableEmptyValue() && !value.get();
+    }
+};
+
 template <typename T>
 class WeakHashSet {
 public:
-    typedef HashSet<Ref<WeakReference<T>>> WeakReferenceSet;
+    typedef HashSet<Ref<WeakReference>> WeakReferenceSet;
 
     class WeakHashSetConstIterator : public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, const T*, const T&> {
     private:
@@ -46,7 +54,7 @@ public:
         }
 
     public:
-        T* get() const { return m_position->get().get(); }
+        T* get() const { return m_position->get().template get<T, typename T::WeakValueType>(); }
         T& operator*() const { return *get(); }
         T* operator->() const { return get(); }
 
@@ -60,7 +68,7 @@ public:
 
         void skipEmptyBuckets()
         {
-            while (m_position != m_endPosition && !m_position->get().get())
+            while (m_position != m_endPosition && !get())
                 ++m_position;
         }
 
@@ -96,42 +104,33 @@ public:
     template <typename U>
     bool remove(const U& value)
     {
-        auto* weakReference = weak_reference_downcast<T>(value.weakPtrFactory().m_ref.get());
-        if (!weakReference)
+        auto& weakReference = value.weakPtrFactory().m_ref;
+        if (!weakReference || !*weakReference)
             return false;
-        return m_set.remove(weakReference);
+        return m_set.remove(*weakReference);
     }
 
     template <typename U>
     bool contains(const U& value) const
     {
-        auto* weakReference = weak_reference_downcast<T>(value.weakPtrFactory().m_ref.get());
-        if (!weakReference)
+        auto& weakReference = value.weakPtrFactory().m_ref;
+        if (!weakReference || !*weakReference)
             return false;
-        return m_set.contains(weakReference);
+        return m_set.contains(*weakReference);
     }
 
     unsigned capacity() const { return m_set.capacity(); }
 
-    bool computesEmpty() const
-    {
-        if (m_set.isEmpty())
-            return true;
-        for (auto& value : m_set) {
-            if (value->get())
-                return false;
-        }
-        return true;
-    }
+    bool computesEmpty() const { return begin() == end(); }
 
     bool hasNullReferences() const
     {
-        return WTF::anyOf(m_set, [] (auto& value) { return !value->get(); });
+        return WTF::anyOf(m_set, [] (auto& value) { return !value.get(); });
     }
 
     unsigned computeSize() const
     {
-        const_cast<WeakReferenceSet&>(m_set).removeIf([] (auto& value) { return !value->get(); });
+        const_cast<WeakReferenceSet&>(m_set).removeIf([] (auto& value) { return !value.get(); });
         return m_set.size();
     }
 
@@ -145,14 +144,6 @@ private:
     WeakReferenceSet m_set;
 };
 
-template<typename T> struct HashTraits<Ref<WeakReference<T>>> : RefHashTraits<WeakReference<T>> {
-    static const bool hasIsReleasedWeakValueFunction = true;
-    static bool isReleasedWeakValue(const Ref<WeakReference<T>>& value)
-    {
-        return !value.isHashTableDeletedValue() && !value.isHashTableEmptyValue() && !value.get().get();
-    }
-};
-
 } // namespace WTF
 
 using WTF::WeakHashSet;
index 2715d1f..fc3ac9c 100644 (file)
 
 namespace WTF {
 
+// Testing interface for TestWebKitAPI
+#ifndef DID_CREATE_WEAK_REFERENCE
+#define DID_CREATE_WEAK_REFERENCE(p)
+#endif
+#ifndef WILL_DESTROY_WEAK_REFERENCE
+#define WILL_DESTROY_WEAK_REFERENCE(p)
+#endif
+
 template<typename> class WeakHashSet;
 template<typename> class WeakPtr;
 template<typename> class WeakPtrFactory;
 
 // Note: WeakReference is an implementation detail, and should not be used directly.
-template<typename T>
-class WeakReference : public ThreadSafeRefCounted<WeakReference<T>> {
-    WTF_MAKE_NONCOPYABLE(WeakReference<T>);
+class WeakReference : public ThreadSafeRefCounted<WeakReference> {
+    WTF_MAKE_NONCOPYABLE(WeakReference);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    ~WeakReference() { } // So that we can use a template specialization for testing purposes to detect leaks.
+    template<typename T> static Ref<WeakReference> create(T* ptr) { return adoptRef(*new WeakReference(ptr)); }
+
+    ~WeakReference()
+    {
+        WILL_DESTROY_WEAK_REFERENCE(m_ptr);
+    }
 
-    T* get() const { return m_ptr; }
+    template<typename T, typename WeakValueType> T* get() const { return static_cast<T*>(static_cast<WeakValueType*>(m_ptr)); }
+    explicit operator bool() const { return m_ptr; }
 
     void clear() { m_ptr = nullptr; }
 
 private:
-    friend class WeakPtr<T>;
-    friend class WeakPtrFactory<T>;
-
-    static Ref<WeakReference<T>> create(T* ptr) { return adoptRef(*new WeakReference(ptr)); }
-
-    explicit WeakReference(T* ptr)
+    template<typename T> explicit WeakReference(T* ptr)
         : m_ptr(ptr)
     {
+        DID_CREATE_WEAK_REFERENCE(ptr);
     }
 
-    T* m_ptr;
+    void* m_ptr;
 };
 
 template<typename T>
@@ -69,28 +78,29 @@ class WeakPtr {
 public:
     WeakPtr() { }
     WeakPtr(std::nullptr_t) { }
-    WeakPtr(Ref<WeakReference<T>>&& ref) : m_ref(std::forward<Ref<WeakReference<T>>>(ref)) { }
     template<typename U> WeakPtr(const WeakPtr<U>&);
     template<typename U> WeakPtr(WeakPtr<U>&&);
 
-    T* get() const { return m_ref ? m_ref->get() : nullptr; }
-    explicit operator bool() const { return m_ref && m_ref->get(); }
+    T* get() const { return m_ref ? m_ref->template get<T, typename T::WeakValueType>() : nullptr; }
+    explicit operator bool() const { return m_ref && *m_ref; }
 
     WeakPtr& operator=(std::nullptr_t) { m_ref = nullptr; return *this; }
     template<typename U> WeakPtr& operator=(const WeakPtr<U>&);
     template<typename U> WeakPtr& operator=(WeakPtr<U>&&);
 
-    T* operator->() const { return m_ref->get(); }
-    T& operator*() const { return *m_ref->get(); }
+    T* operator->() const { return get(); }
+    T& operator*() const { return *get(); }
 
     void clear() { m_ref = nullptr; }
 
 private:
+    explicit WeakPtr(Ref<WeakReference>&& ref) : m_ref(std::move(ref)) { }
     template<typename> friend class WeakHashSet;
     template<typename> friend class WeakPtr;
+    template<typename> friend class WeakPtrFactory;
     template<typename U> friend WeakPtr<U> makeWeakPtr(U&);
 
-    RefPtr<WeakReference<T>> m_ref;
+    RefPtr<WeakReference> m_ref;
 };
 
 // Note: you probably want to inherit from CanMakeWeakPtr rather than use this directly.
@@ -110,15 +120,15 @@ public:
     WeakPtr<T> createWeakPtr(T& ptr) const
     {
         if (!m_ref)
-            m_ref = WeakReference<T>::create(&ptr);
-        return { makeRef(*m_ref) };
+            m_ref = WeakReference::create(&ptr);
+        return WeakPtr<T>(makeRef(*m_ref));
     }
 
     WeakPtr<const T> createWeakPtr(const T& ptr) const
     {
         if (!m_ref)
-            m_ref = WeakReference<T>::create(const_cast<T*>(&ptr));
-        return { makeRef(reinterpret_cast<WeakReference<const T>&>(*m_ref)) };
+            m_ref = WeakReference::create(const_cast<T*>(&ptr));
+        return WeakPtr<T>(makeRef(*m_ref));
     }
 
     void revokeAll()
@@ -133,11 +143,13 @@ public:
 private:
     template<typename> friend class WeakHashSet;
 
-    mutable RefPtr<WeakReference<T>> m_ref;
+    mutable RefPtr<WeakReference> m_ref;
 };
 
 template<typename T> class CanMakeWeakPtr {
 public:
+    typedef T WeakValueType;
+
     const WeakPtrFactory<T>& weakPtrFactory() const { return m_weakFactory; }
     WeakPtrFactory<T>& weakPtrFactory() { return m_weakFactory; }
 
@@ -145,43 +157,37 @@ private:
     WeakPtrFactory<T> m_weakFactory;
 };
 
-template<typename T, typename U> inline WeakReference<T>* weak_reference_upcast(WeakReference<U>* weakReference)
-{
-    static_assert(std::is_convertible<U*, T*>::value, "U* must be convertible to T*");
-    return reinterpret_cast<WeakReference<T>*>(weakReference);
-}
-
-template<typename T, typename U> inline WeakReference<T>* weak_reference_downcast(WeakReference<U>* weakReference)
+template<typename T, typename U> inline WeakReference* weak_reference_cast(WeakReference* weakReference)
 {
-    static_assert(std::is_convertible<T*, U*>::value, "T* must be convertible to U*");
-    return reinterpret_cast<WeakReference<T>*>(weakReference);
+    UNUSED_VARIABLE(static_cast<T*>(static_cast<typename U::WeakValueType*>(nullptr))); // Verify that casting is valid.
+    return weakReference;
 }
 
 template<typename T> template<typename U> inline WeakPtr<T>::WeakPtr(const WeakPtr<U>& o)
-    : m_ref(weak_reference_upcast<T>(o.m_ref.get()))
+    : m_ref(weak_reference_cast<T, U>(o.m_ref.get()))
 {
 }
 
 template<typename T> template<typename U> inline WeakPtr<T>::WeakPtr(WeakPtr<U>&& o)
-    : m_ref(adoptRef(weak_reference_upcast<T>(o.m_ref.leakRef())))
+    : m_ref(adoptRef(weak_reference_cast<T, U>(o.m_ref.leakRef())))
 {
 }
 
 template<typename T> template<typename U> inline WeakPtr<T>& WeakPtr<T>::operator=(const WeakPtr<U>& o)
 {
-    m_ref = weak_reference_upcast<T>(o.m_ref.get());
+    m_ref = weak_reference_cast<T, U>(o.m_ref.get());
     return *this;
 }
 
 template<typename T> template<typename U> inline WeakPtr<T>& WeakPtr<T>::operator=(WeakPtr<U>&& o)
 {
-    m_ref = adoptRef(weak_reference_upcast<T>(o.m_ref.leakRef()));
+    m_ref = adoptRef(weak_reference_cast<T, U>(o.m_ref.leakRef()));
     return *this;
 }
 
 template<typename T> inline WeakPtr<T> makeWeakPtr(T& ref)
 {
-    return { adoptRef(*weak_reference_downcast<T>(ref.weakPtrFactory().createWeakPtr(ref).m_ref.leakRef())) };
+    return { ref.weakPtrFactory().createWeakPtr(ref) };
 }
 
 template<typename T> inline WeakPtr<T> makeWeakPtr(T* ptr)
index db976d1..cce9f6e 100644 (file)
@@ -1,3 +1,116 @@
+2019-05-29  Geoffrey Garen  <ggaren@apple.com>
+
+        WeakPtr breaks vtables when upcasting to base classes
+        https://bugs.webkit.org/show_bug.cgi?id=188799
+
+        Reviewed by Youenn Fablet.
+
+        * Modules/encryptedmedia/MediaKeySession.cpp:
+        (WebCore::MediaKeySession::MediaKeySession):
+        * Modules/encryptedmedia/MediaKeySession.h: Adopted modern WeakPtr APIs.
+        Removed redundant WeakPtrFactory.
+
+        * css/CSSFontFace.cpp:
+        (WebCore::CSSFontFace::existingWrapper):
+        * css/CSSFontFace.h: Moved functions out of line to avoid #include
+        explosion for .get().
+
+        * dom/ContainerNode.h:
+        * dom/Document.h:
+        * dom/Element.h: Moved CanMakeWeakPtr to ContainerNode because all
+        subclasses except for DocumentFragment were already so, and we have
+        code that uses WeakPtr<ContainerNode>, which, now that WeakPtr is
+        type-safe, is awkward to do when ContainerNode isn't CanMakeWeakPtr.
+
+        * dom/FullscreenManager.cpp:
+        (WebCore::FullscreenManager::fullscreenRenderer const):
+        * dom/FullscreenManager.h:
+        (WebCore::FullscreenManager::fullscreenRenderer const): Deleted.
+        * html/FormAssociatedElement.cpp:
+        (WebCore::FormAssociatedElement::form const):
+        * html/FormAssociatedElement.h:
+        (WebCore::FormAssociatedElement::form const): Deleted. Moved functions
+        out of line to avoid #include explosion for .get().
+
+        * html/HTMLMediaElement.h: It takes an extra using declaration
+        to disambiguate multiple CanMakeWeakPtr base classes now.
+
+        * loader/MediaResourceLoader.cpp:
+        (WebCore::MediaResourceLoader::requestResource): Removed redundant .get().
+
+        * page/DOMWindowProperty.cpp:
+        (WebCore::DOMWindowProperty::window const):
+        * page/DOMWindowProperty.h:
+        (WebCore::DOMWindowProperty::window const): Deleted.
+        * page/FrameViewLayoutContext.cpp:
+        (WebCore::FrameViewLayoutContext::subtreeLayoutRoot const):
+        * page/FrameViewLayoutContext.h:
+        (WebCore::FrameViewLayoutContext::subtreeLayoutRoot const): Deleted.
+        * page/UndoItem.cpp:
+        (WebCore::UndoItem::undoManager const):
+        * page/UndoItem.h:
+        (WebCore::UndoItem::undoManager const): Deleted. Moved functions out of
+        line to avoid #include explosion for .get().
+
+        * platform/ScrollView.h: It takes an extra using declaration
+        to disambiguate multiple CanMakeWeakPtr base classes now.
+
+        * platform/Widget.cpp:
+        (WebCore::Widget::parent const):
+        * platform/Widget.h:
+        (WebCore::Widget::parent const): Deleted. Moved functions out of line to avoid #include
+        explosion for .get().
+
+        * platform/encryptedmedia/CDMInstanceSession.h: Made
+        CDMInstanceSessionClient CanMakeWeakPtr because we use WeakPtr<CDMInstanceSessionClient>.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+        CanMakeWeakPtr is inherited now.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC):
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::cdmSession const): Deleted.
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::createWeakPtr): Deleted.
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
+        (WebCore::CMTimebaseEffectiveRateChangedCallback):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::durationChanged):
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::cdmSession const):
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+        (WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC):
+        (WebCore::SourceBufferPrivateAVFObjC::trackDidChangeEnabled):
+        (WebCore::SourceBufferPrivateAVFObjC::setCDMSession):
+        (WebCore::SourceBufferPrivateAVFObjC::flushVideo):
+        (WebCore::SourceBufferPrivateAVFObjC::enqueueSample):
+        (WebCore::SourceBufferPrivateAVFObjC::notifyClientWhenReadyForMoreSamples):
+        (WebCore::SourceBufferPrivateAVFObjC::setVideoLayer):
+        (WebCore::SourceBufferPrivateAVFObjC::setDecompressionSession): Modernized WeakPtr API usage.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::multiColumnFlowSlowCase const):
+        * rendering/RenderBlockFlow.h:
+        (WebCore::RenderBlockFlow::multiColumnFlow const):
+        * rendering/RenderMultiColumnFlow.cpp:
+        (WebCore::RenderMultiColumnFlow::findColumnSpannerPlaceholder const):
+        * rendering/RenderMultiColumnFlow.h:
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::header const):
+        (WebCore::RenderTable::footer const):
+        (WebCore::RenderTable::firstBody const):
+        (WebCore::RenderTable::topSection const):
+        * rendering/RenderTable.h:
+        (WebCore::RenderTable::header const): Deleted.
+        (WebCore::RenderTable::footer const): Deleted.
+        (WebCore::RenderTable::firstBody const): Deleted.
+        (WebCore::RenderTable::topSection const): Deleted. Moved functions out
+        of line to avoid #include explosion for .get().
+
 2019-05-29  Antoine Quint  <graouts@apple.com>
 
         [Pointer Events] toElement and fromElement should be null
index c58adf6..612a2bd 100644 (file)
@@ -94,7 +94,7 @@ MediaKeySession::MediaKeySession(ScriptExecutionContext& context, WeakPtr<MediaK
     UNUSED_PARAM(m_closed);
     UNUSED_PARAM(m_uninitialized);
 
-    m_instanceSession->setClient(m_cdmInstanceSessionClientWeakPtrFactory.createWeakPtr(*this));
+    m_instanceSession->setClient(makeWeakPtr(*this));
 }
 
 MediaKeySession::~MediaKeySession()
index c9f60f1..ded2dde 100644 (file)
@@ -52,7 +52,7 @@ class MediaKeyStatusMap;
 class MediaKeys;
 class SharedBuffer;
 
-class MediaKeySession final : public RefCounted<MediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, public CanMakeWeakPtr<MediaKeySession>, public CDMInstanceSessionClient {
+class MediaKeySession final : public RefCounted<MediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, public CDMInstanceSessionClient {
     WTF_MAKE_ISO_ALLOCATED(MediaKeySession);
 public:
     static Ref<MediaKeySession> create(ScriptExecutionContext&, WeakPtr<MediaKeys>&&, MediaKeySessionType, bool useDistinctiveIdentifier, Ref<CDM>&&, Ref<CDMInstanceSession>&&);
@@ -120,7 +120,6 @@ private:
     double m_firstDecryptTime { 0 };
     double m_latestDecryptTime { 0 };
     Vector<std::pair<Ref<SharedBuffer>, MediaKeyStatus>> m_statuses;
-    WeakPtrFactory<CDMInstanceSessionClient> m_cdmInstanceSessionClientWeakPtrFactory;
 };
 
 } // namespace WebCore
index 0f3e03f..7377adb 100644 (file)
@@ -122,6 +122,11 @@ bool CSSFontFace::setFamilies(CSSValue& family)
     return true;
 }
 
+FontFace* CSSFontFace::existingWrapper()
+{
+    return m_wrapper.get();
+}
+
 static FontSelectionRange calculateWeightRange(CSSValue& value)
 {
     if (value.isValueList()) {
index ad06d19..a39a926 100644 (file)
@@ -150,7 +150,7 @@ public:
     // We don't guarantee that the FontFace wrapper will be the same every time you ask for it.
     Ref<FontFace> wrapper();
     void setWrapper(FontFace&);
-    FontFace* existingWrapper() { return m_wrapper.get(); }
+    FontFace* existingWrapper();
 
     struct FontLoadTiming {
         Seconds blockPeriod;
index d34c19b..0f98a98 100644 (file)
@@ -39,6 +39,11 @@ CSSDeferredParser::CSSDeferredParser(const CSSParserContext& context, const Stri
 {
 }
 
+StyleSheetContents* CSSDeferredParser::styleSheet() const
+{
+    return m_styleSheet.get();
+}
+
 Ref<ImmutableStyleProperties> CSSDeferredParser::parseDeclaration(const CSSParserTokenRange& range)
 {
     return CSSParserImpl::parseDeferredDeclaration(range, m_context, m_styleSheet.get());
index 07e9c92..bf84f27 100644 (file)
@@ -47,7 +47,7 @@ public:
     CSSParserMode mode() const { return m_context.mode; }
 
     const CSSParserContext& context() const { return m_context; }
-    StyleSheetContents* styleSheet() const { return m_styleSheet.get(); }
+    StyleSheetContents* styleSheet() const;
 
     Ref<ImmutableStyleProperties> parseDeclaration(const CSSParserTokenRange&);
     void parseRuleList(const CSSParserTokenRange&, Vector<RefPtr<StyleRuleBase>>&);
index 4a9bfdb..3f0571e 100644 (file)
@@ -25,6 +25,7 @@
 
 #include "CollectionType.h"
 #include "Node.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -35,7 +36,7 @@ class RenderElement;
 const int initialNodeVectorSize = 11; // Covers 99.5%. See webkit.org/b/80706
 typedef Vector<Ref<Node>, initialNodeVectorSize> NodeVector;
 
-class ContainerNode : public Node {
+class ContainerNode : public CanMakeWeakPtr<ContainerNode>, public Node {
     WTF_MAKE_ISO_ALLOCATED(ContainerNode);
 public:
     virtual ~ContainerNode();
index 16d0410..69ef47b 100644 (file)
@@ -347,7 +347,6 @@ class Document
     , public TreeScope
     , public ScriptExecutionContext
     , public FontSelectorClient
-    , public CanMakeWeakPtr<Document>
     , public FrameDestructionObserver
     , public Supplementable<Document>
     , public Logger::Observer {
index c6016e0..8ded545 100644 (file)
@@ -77,7 +77,7 @@ enum SpellcheckAttributeState {
 enum class TouchAction : uint8_t;
 #endif
 
-class Element : public ContainerNode , public CanMakeWeakPtr<Element> {
+class Element : public ContainerNode {
     WTF_MAKE_ISO_ALLOCATED(Element);
 public:
     static Ref<Element> create(const QualifiedName&, Document&);
index b55ed2b..6f92c90 100644 (file)
@@ -439,6 +439,11 @@ void FullscreenManager::setFullscreenRenderer(RenderTreeBuilder& builder, Render
     m_fullscreenRenderer = makeWeakPtr(renderer);
 }
 
+RenderFullScreen* FullscreenManager::fullscreenRenderer() const
+{
+    return m_fullscreenRenderer.get();
+}
+
 void FullscreenManager::dispatchFullscreenChangeEvents()
 {
     // Since we dispatch events in this function, it's possible that the
index 7ab5a7b..ebdaa95 100644 (file)
@@ -79,7 +79,7 @@ public:
     WEBCORE_EXPORT void didExitFullscreen();
 
     void setFullscreenRenderer(RenderTreeBuilder&, RenderFullScreen&);
-    RenderFullScreen* fullscreenRenderer() const { return m_fullscreenRenderer.get(); }
+    RenderFullScreen* fullscreenRenderer() const;
 
     void dispatchFullscreenChangeEvents();
     bool fullscreenIsAllowedForElement(Element&) const;
index 25220cb..bd42622 100644 (file)
@@ -119,6 +119,11 @@ HTMLFormElement* FormAssociatedElement::findAssociatedForm(const HTMLElement* el
     return currentAssociatedForm;
 }
 
+HTMLFormElement* FormAssociatedElement::form() const
+{
+    return m_form.get();
+}
+
 void FormAssociatedElement::formOwnerRemovedFromTree(const Node& formRoot)
 {
     ASSERT(m_form);
index 5c84a19..88639de 100644 (file)
@@ -48,7 +48,7 @@ public:
     void deref() { derefFormAssociatedElement(); }
 
     static HTMLFormElement* findAssociatedForm(const HTMLElement*, HTMLFormElement*);
-    HTMLFormElement* form() const { return m_form.get(); }
+    WEBCORE_EXPORT HTMLFormElement* form() const;
     ValidityState* validity();
 
     virtual bool isFormControlElement() const = 0;
index 2fff363..ed88e3c 100644 (file)
@@ -149,6 +149,9 @@ class HTMLMediaElement
 {
     WTF_MAKE_ISO_ALLOCATED(HTMLMediaElement);
 public:
+    typedef HTMLElement::WeakValueType WeakValueType;
+    using HTMLElement::weakPtrFactory;
+
     RefPtr<MediaPlayer> player() const { return m_player; }
 
     virtual bool isVideo() const { return false; }
@@ -575,8 +578,6 @@ public:
 
     enum class AutoplayEventPlaybackState { None, PreventedAutoplay, StartedWithUserGesture, StartedWithoutUserGesture };
 
-    using HTMLElement::weakPtrFactory;
-
 protected:
     HTMLMediaElement(const QualifiedName&, Document&, bool createdByParser);
     virtual void finishInitialization();
index db29143..06e847d 100644 (file)
@@ -97,7 +97,7 @@ RefPtr<PlatformMediaResource> MediaResourceLoader::requestResource(ResourceReque
     loaderOptions.destination = m_mediaElement && !m_mediaElement->isVideo() ? FetchOptions::Destination::Audio : FetchOptions::Destination::Video;
     auto cachedRequest = createPotentialAccessControlRequest(WTFMove(request), *m_document, m_crossOriginMode, WTFMove(loaderOptions));
     if (m_mediaElement)
-        cachedRequest.setInitiator(*m_mediaElement.get());
+        cachedRequest.setInitiator(*m_mediaElement);
 
     auto resource = m_document->cachedResourceLoader().requestMedia(WTFMove(cachedRequest)).value_or(nullptr);
     if (!resource)
index b87f2e5..7bb61c2 100644 (file)
@@ -42,4 +42,9 @@ Frame* DOMWindowProperty::frame() const
     return m_window ? m_window->frame() : nullptr;
 }
 
+DOMWindow* DOMWindowProperty::window() const
+{
+    return m_window.get();
+}
+
 }
index fb77175..0aaafb3 100644 (file)
@@ -35,7 +35,7 @@ class Frame;
 class DOMWindowProperty {
 public:
     Frame* frame() const;
-    DOMWindow* window() const { return m_window.get(); }
+    DOMWindow* window() const;
 
 protected:
     explicit DOMWindowProperty(DOMWindow*);
index fd1fa60..327b6ea 100644 (file)
@@ -458,6 +458,11 @@ void FrameViewLayoutContext::layoutTimerFired()
     layout();
 }
 
+RenderElement* FrameViewLayoutContext::subtreeLayoutRoot() const
+{
+    return m_subtreeLayoutRoot.get();
+}
+
 void FrameViewLayoutContext::convertSubtreeLayoutToFullLayout()
 {
     ASSERT(subtreeLayoutRoot());
index d16f6c3..b2ca37b 100644 (file)
@@ -27,7 +27,6 @@
 
 #include "LayoutUnit.h"
 #include "Timer.h"
-
 #include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -82,7 +81,7 @@ public:
 
     unsigned layoutCount() const { return m_layoutCount; }
 
-    RenderElement* subtreeLayoutRoot() const { return m_subtreeLayoutRoot.get(); }
+    RenderElement* subtreeLayoutRoot() const;
     void clearSubtreeLayoutRoot() { m_subtreeLayoutRoot.clear(); }
     void convertSubtreeLayoutToFullLayout();
 
index 2b32137..e03f1f0 100644 (file)
@@ -33,6 +33,11 @@ namespace WebCore {
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(UndoItem);
 
+UndoManager* UndoItem::undoManager() const
+{
+    return m_undoManager.get();
+}
+
 void UndoItem::setUndoManager(UndoManager* undoManager)
 {
     m_undoManager = makeWeakPtr(undoManager);
index 6f0b5a1..386d476 100644 (file)
@@ -55,7 +55,7 @@ public:
 
     Document* document() const;
 
-    UndoManager* undoManager() const { return m_undoManager.get(); }
+    UndoManager* undoManager() const;
     void setUndoManager(UndoManager*);
 
     const String& label() const { return m_label; }
index 7d824c4..d847a20 100644 (file)
@@ -65,6 +65,9 @@ class ScrollView : public Widget, public ScrollableArea {
 public:
     virtual ~ScrollView();
 
+    typedef Widget::WeakValueType WeakValueType;
+    using Widget::weakPtrFactory;
+
     // ScrollableArea functions.
     int scrollSize(ScrollbarOrientation) const final;
     int scrollOffset(ScrollbarOrientation) const final;
@@ -74,8 +77,6 @@ public:
 
     virtual void notifyPageThatContentAreaWillPaint() const;
 
-    using Widget::weakPtrFactory;
-
     IntPoint locationOfContents() const;
 
     // NOTE: This should only be called by the overridden setScrollOffset from ScrollableArea.
index 48570aa..6cc645e 100644 (file)
@@ -42,6 +42,11 @@ void Widget::init(PlatformWidget widget)
         retainPlatformWidget();
 }
 
+ScrollView* Widget::parent() const
+{
+    return m_parent.get();
+}
+
 void Widget::setParent(ScrollView* view)
 {
     ASSERT(!view || !m_parent);
index 2aeaeab..f2879cc 100644 (file)
@@ -140,7 +140,7 @@ public:
 
     WEBCORE_EXPORT void removeFromParent();
     WEBCORE_EXPORT virtual void setParent(ScrollView* view);
-    ScrollView* parent() const { return m_parent.get(); }
+    WEBCORE_EXPORT ScrollView* parent() const;
     FrameView* root() const;
 
     virtual void handleEvent(Event&) { }
index 8d12a01..a71ab8e 100644 (file)
@@ -39,7 +39,7 @@ namespace WebCore {
 
 class SharedBuffer;
 
-class CDMInstanceSessionClient {
+class CDMInstanceSessionClient : public CanMakeWeakPtr<CDMInstanceSessionClient> {
 public:
     virtual ~CDMInstanceSessionClient() = default;
 
index 7587ad5..2a709ff 100644 (file)
@@ -337,7 +337,6 @@ private:
     bool performTaskAtMediaTime(WTF::Function<void()>&&, MediaTime) final;
     void setShouldObserveTimeControlStatus(bool);
 
-    WeakPtrFactory<MediaPlayerPrivateAVFoundationObjC> m_weakPtrFactory;
     RetainPtr<AVURLAsset> m_avAsset;
     RetainPtr<AVPlayer> m_avPlayer;
     RetainPtr<AVPlayerItem> m_avPlayerItem;
index c123ed5..94365fe 100644 (file)
@@ -359,14 +359,14 @@ MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC(MediaPlay
     : MediaPlayerPrivateAVFoundation(player)
     , m_videoFullscreenLayerManager(std::make_unique<VideoFullscreenLayerManagerObjC>())
     , m_videoFullscreenGravity(MediaPlayer::VideoGravityResizeAspect)
-    , m_objcObserver(adoptNS([[WebCoreAVFMovieObserver alloc] initWithPlayer:m_weakPtrFactory.createWeakPtr(*this)]))
+    , m_objcObserver(adoptNS([[WebCoreAVFMovieObserver alloc] initWithPlayer:makeWeakPtr(*this)]))
     , m_videoFrameHasDrawn(false)
     , m_haveCheckedPlayability(false)
 #if HAVE(AVFOUNDATION_VIDEO_OUTPUT)
-    , m_videoOutputDelegate(adoptNS([[WebCoreAVFPullDelegate alloc] initWithPlayer:m_weakPtrFactory.createWeakPtr(*this)]))
+    , m_videoOutputDelegate(adoptNS([[WebCoreAVFPullDelegate alloc] initWithPlayer:makeWeakPtr(*this)]))
 #endif
 #if HAVE(AVFOUNDATION_LOADER_DELEGATE)
-    , m_loaderDelegate(adoptNS([[WebCoreAVFLoaderDelegate alloc] initWithPlayer:m_weakPtrFactory.createWeakPtr(*this)]))
+    , m_loaderDelegate(adoptNS([[WebCoreAVFLoaderDelegate alloc] initWithPlayer:makeWeakPtr(*this)]))
 #endif
     , m_currentTextTrack(0)
     , m_cachedRate(0)
@@ -387,7 +387,7 @@ MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC(MediaPlay
 
 MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC()
 {
-    m_weakPtrFactory.revokeAll();
+    weakPtrFactory().revokeAll();
 
 #if HAVE(AVFOUNDATION_LOADER_DELEGATE)
     [[m_avAsset.get() resourceLoader] setDelegate:nil queue:0];
index 5180c9f..bf38721 100644 (file)
@@ -59,7 +59,8 @@ class WebCoreDecompressionSession;
 
 
 class MediaPlayerPrivateMediaSourceAVFObjC
-    : public MediaPlayerPrivateInterface
+    : public CanMakeWeakPtr<MediaPlayerPrivateMediaSourceAVFObjC>
+    , public MediaPlayerPrivateInterface
 #if !RELEASE_LOG_DISABLED
     , private LoggerHelper
 #endif
@@ -121,7 +122,7 @@ public:
     AVStreamSession *streamSession();
 #endif
     void setCDMSession(LegacyCDMSession*) override;
-    CDMSessionMediaSourceAVFObjC* cdmSession() const { return m_session.get(); }
+    CDMSessionMediaSourceAVFObjC* cdmSession() const;
 #endif
 
 #if ENABLE(ENCRYPTED_MEDIA)
@@ -147,8 +148,6 @@ public:
     const Vector<ContentType>& mediaContentTypesRequiringHardwareSupport() const;
     bool shouldCheckHardwareSupport() const;
 
-    WeakPtr<MediaPlayerPrivateMediaSourceAVFObjC> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(*this); }
-
 #if !RELEASE_LOG_DISABLED
     const Logger& logger() const final { return m_logger.get(); }
     const char* logClassName() const override { return "MediaPlayerPrivateMediaSourceAVFObjC"; }
@@ -282,7 +281,6 @@ private:
     std::unique_ptr<PendingSeek> m_pendingSeek;
 
     MediaPlayer* m_player;
-    WeakPtrFactory<MediaPlayerPrivateMediaSourceAVFObjC> m_weakPtrFactory;
     WeakPtrFactory<MediaPlayerPrivateMediaSourceAVFObjC> m_sizeChangeObserverWeakPtrFactory;
     RefPtr<MediaSourcePrivateAVFObjC> m_mediaSourcePrivate;
     RetainPtr<AVAsset> m_asset;
index 2513b4b..78e6247 100644 (file)
@@ -52,6 +52,7 @@
 #import <wtf/FileSystem.h>
 #import <wtf/MainThread.h>
 #import <wtf/NeverDestroyed.h>
+#import <wtf/WeakPtr.h>
 
 #import "CoreVideoSoftLink.h"
 #import <pal/cf/CoreMediaSoftLink.h>
@@ -87,10 +88,10 @@ String convertEnumerationToString(MediaPlayerPrivateMediaSourceAVFObjC::SeekStat
 static void CMTimebaseEffectiveRateChangedCallback(CMNotificationCenterRef, const void *listener, CFStringRef, const void *, CFTypeRef)
 {
     MediaPlayerPrivateMediaSourceAVFObjC* player = (MediaPlayerPrivateMediaSourceAVFObjC*)const_cast<void*>(listener);
-    callOnMainThread([weakThis = player->createWeakPtr()] {
+    callOnMainThread([weakThis = makeWeakPtr(player)] {
         if (!weakThis)
             return;
-        weakThis.get()->effectiveRateChanged();
+        weakThis->effectiveRateChanged();
     });
 }
 
@@ -120,7 +121,7 @@ MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC(Media
 
     // addPeriodicTimeObserverForInterval: throws an exception if you pass a non-numeric CMTime, so just use
     // an arbitrarily large time value of once an hour:
-    __block auto weakThis = createWeakPtr();
+    __block auto weakThis = makeWeakPtr(*this);
     m_timeJumpedObserver = [m_synchronizer addPeriodicTimeObserverForInterval:PAL::toCMTime(MediaTime::createWithDouble(3600)) queue:dispatch_get_main_queue() usingBlock:^(CMTime time) {
 #if LOG_DISABLED
         UNUSED_PARAM(time);
@@ -286,7 +287,7 @@ PlatformLayer* MediaPlayerPrivateMediaSourceAVFObjC::platformLayer() const
 void MediaPlayerPrivateMediaSourceAVFObjC::play()
 {
     ALWAYS_LOG(LOGIDENTIFIER);
-    callOnMainThread([weakThis = createWeakPtr()] {
+    callOnMainThread([weakThis = makeWeakPtr(*this)] {
         if (!weakThis)
             return;
         weakThis.get()->playInternal();
@@ -307,7 +308,7 @@ void MediaPlayerPrivateMediaSourceAVFObjC::playInternal()
 void MediaPlayerPrivateMediaSourceAVFObjC::pause()
 {
     ALWAYS_LOG(LOGIDENTIFIER);
-    callOnMainThread([weakThis = createWeakPtr()] {
+    callOnMainThread([weakThis = makeWeakPtr(*this)] {
         if (!weakThis)
             return;
         weakThis.get()->pauseInternal();
@@ -407,7 +408,6 @@ void MediaPlayerPrivateMediaSourceAVFObjC::seekWithTolerance(const MediaTime& ti
     INFO_LOG(LOGIDENTIFIER, "time = ", time, ", negativeThreshold = ", negativeThreshold, ", positiveThreshold = ", positiveThreshold);
 
     m_seeking = true;
-    auto weakThis = createWeakPtr();
     m_pendingSeek = std::make_unique<PendingSeek>(time, negativeThreshold, positiveThreshold);
 
     if (m_seekTimer.isActive())
@@ -858,14 +858,13 @@ void MediaPlayerPrivateMediaSourceAVFObjC::durationChanged()
         return;
 
     MediaTime duration = m_mediaSourcePrivate->duration();
-    auto weakThis = createWeakPtr();
     NSArray* times = @[[NSValue valueWithCMTime:PAL::toCMTime(duration)]];
 
     auto logSiteIdentifier = LOGIDENTIFIER;
     DEBUG_LOG(logSiteIdentifier, duration);
     UNUSED_PARAM(logSiteIdentifier);
 
-    m_durationObserver = [m_synchronizer addBoundaryTimeObserverForTimes:times queue:dispatch_get_main_queue() usingBlock:[weakThis, duration, logSiteIdentifier, this] {
+    m_durationObserver = [m_synchronizer addBoundaryTimeObserverForTimes:times queue:dispatch_get_main_queue() usingBlock:[weakThis = makeWeakPtr(*this), duration, logSiteIdentifier, this] {
         if (!weakThis)
             return;
 
@@ -955,6 +954,11 @@ AVStreamSession* MediaPlayerPrivateMediaSourceAVFObjC::streamSession()
 }
 #endif
 
+CDMSessionMediaSourceAVFObjC* MediaPlayerPrivateMediaSourceAVFObjC::cdmSession() const
+{
+    return m_session.get();
+}
+
 void MediaPlayerPrivateMediaSourceAVFObjC::setCDMSession(LegacyCDMSession* session)
 {
     if (session == m_session)
index a73f920..705082e 100644 (file)
@@ -175,13 +175,10 @@ private:
     void flush(AVSampleBufferAudioRenderer *);
     ALLOW_NEW_API_WITHOUT_GUARDS_END
 
-    WeakPtr<SourceBufferPrivateAVFObjC> createWeakPtr() { return m_weakFactory.createWeakPtr(*this); }
-
     Vector<RefPtr<VideoTrackPrivateMediaSourceAVFObjC>> m_videoTracks;
     Vector<RefPtr<AudioTrackPrivateMediaSourceAVFObjC>> m_audioTracks;
     Vector<SourceBufferPrivateAVFObjCErrorClient*> m_errorClients;
 
-    WeakPtrFactory<SourceBufferPrivateAVFObjC> m_weakFactory;
     WeakPtrFactory<SourceBufferPrivateAVFObjC> m_appendWeakFactory;
 
     RetainPtr<AVStreamDataParser> m_parser;
index 00be37a..4b877b9 100644 (file)
@@ -465,8 +465,8 @@ Ref<SourceBufferPrivateAVFObjC> SourceBufferPrivateAVFObjC::create(MediaSourcePr
 
 SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC(MediaSourcePrivateAVFObjC* parent)
     : m_parser(adoptNS([PAL::allocAVStreamDataParserInstance() init]))
-    , m_delegate(adoptNS([[WebAVStreamDataParserListener alloc] initWithParser:m_parser.get() parent:createWeakPtr()]))
-    , m_errorListener(adoptNS([[WebAVSampleBufferErrorListener alloc] initWithParent:createWeakPtr()]))
+    , m_delegate(adoptNS([[WebAVStreamDataParserListener alloc] initWithParser:m_parser.get() parent:makeWeakPtr(*this)]))
+    , m_errorListener(adoptNS([[WebAVSampleBufferErrorListener alloc] initWithParent:makeWeakPtr(*this)]))
     , m_isAppendingGroup(adoptOSObject(dispatch_group_create()))
     , m_mediaSource(parent)
     , m_mapID(nextMapID())
@@ -884,7 +884,7 @@ void SourceBufferPrivateAVFObjC::trackDidChangeEnabled(AudioTrackPrivateMediaSou
         ALLOW_NEW_API_WITHOUT_GUARDS_END
         if (!m_audioRenderers.contains(trackID)) {
             renderer = adoptNS([PAL::allocAVSampleBufferAudioRendererInstance() init]);
-            auto weakThis = createWeakPtr();
+            auto weakThis = makeWeakPtr(*this);
             [renderer requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^{
                 if (weakThis)
                     weakThis->didBecomeReadyForMoreSamples(trackID);
@@ -920,8 +920,7 @@ void SourceBufferPrivateAVFObjC::setCDMSession(CDMSessionMediaSourceAVFObjC* ses
         }
 
         if (m_hdcpError) {
-            WeakPtr<SourceBufferPrivateAVFObjC> weakThis = createWeakPtr();
-            callOnMainThread([weakThis] {
+            callOnMainThread([weakThis = makeWeakPtr(*this)] {
                 if (!weakThis || !weakThis->m_session || !weakThis->m_hdcpError)
                     return;
 
@@ -1065,7 +1064,7 @@ void SourceBufferPrivateAVFObjC::flushVideo()
 
     if (m_decompressionSession) {
         m_decompressionSession->flush();
-        m_decompressionSession->notifyWhenHasAvailableVideoFrame([weakThis = createWeakPtr()] {
+        m_decompressionSession->notifyWhenHasAvailableVideoFrame([weakThis = makeWeakPtr(*this)] {
             if (weakThis && weakThis->m_mediaSource)
                 weakThis->m_mediaSource->player()->setHasAvailableVideoFrame(true);
         });
@@ -1138,7 +1137,7 @@ void SourceBufferPrivateAVFObjC::enqueueSample(Ref<MediaSample>&& sample, const
 #endif
             } else {
                 [m_displayLayer enqueueSampleBuffer:platformSample.sample.cmSampleBuffer];
-                [m_displayLayer prerollDecodeWithCompletionHandler:[weakThis = createWeakPtr()] (BOOL success) mutable {
+                [m_displayLayer prerollDecodeWithCompletionHandler:[weakThis = makeWeakPtr(*this)] (BOOL success) mutable {
                     if (!success || !weakThis)
                         return;
 
@@ -1237,14 +1236,14 @@ void SourceBufferPrivateAVFObjC::notifyClientWhenReadyForMoreSamples(const Atomi
             });
         }
         if (m_displayLayer) {
-            auto weakThis = createWeakPtr();
+            auto weakThis = makeWeakPtr(*this);
             [m_displayLayer requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^ {
                 if (weakThis)
                     weakThis->didBecomeReadyForMoreSamples(trackID);
             }];
         }
     } else if (m_audioRenderers.contains(trackID)) {
-        auto weakThis = createWeakPtr();
+        auto weakThis = makeWeakPtr(*this);
         [m_audioRenderers.get(trackID) requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^ {
             if (weakThis)
                 weakThis->didBecomeReadyForMoreSamples(trackID);
@@ -1278,7 +1277,7 @@ void SourceBufferPrivateAVFObjC::setVideoLayer(AVSampleBufferDisplayLayer* layer
     m_displayLayer = layer;
 
     if (m_displayLayer) {
-        auto weakThis = createWeakPtr();
+        auto weakThis = makeWeakPtr(*this);
         [m_displayLayer requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^ {
             if (weakThis)
                 weakThis->didBecomeReadyForMoreSamples(m_enabledVideoTrackID);
@@ -1306,12 +1305,11 @@ void SourceBufferPrivateAVFObjC::setDecompressionSession(WebCoreDecompressionSes
     if (!m_decompressionSession)
         return;
 
-    WeakPtr<SourceBufferPrivateAVFObjC> weakThis = createWeakPtr();
-    m_decompressionSession->requestMediaDataWhenReady([weakThis] {
+    m_decompressionSession->requestMediaDataWhenReady([weakThis = makeWeakPtr(*this)] {
         if (weakThis)
             weakThis->didBecomeReadyForMoreSamples(weakThis->m_enabledVideoTrackID);
     });
-    m_decompressionSession->notifyWhenHasAvailableVideoFrame([weakThis = createWeakPtr()] {
+    m_decompressionSession->notifyWhenHasAvailableVideoFrame([weakThis = makeWeakPtr(*this)] {
         if (weakThis && weakThis->m_mediaSource)
             weakThis->m_mediaSource->player()->setHasAvailableVideoFrame(true);
     });
index 042c2cc..ddb821b 100644 (file)
@@ -159,6 +159,11 @@ void RenderBlockFlow::willBeDestroyed()
     RenderBox::willBeDestroyed();
 }
 
+RenderMultiColumnFlow* RenderBlockFlow::multiColumnFlowSlowCase() const
+{
+    return rareBlockFlowData()->m_multiColumnFlow.get();
+}
+
 RenderBlockFlow* RenderBlockFlow::previousSiblingWithOverhangingFloats(bool& parentHasFloats) const
 {
     // Attempt to locate a previous sibling with overhanging floats. We skip any elements that are
index bdbbe3c..42db8e9 100644 (file)
@@ -264,7 +264,8 @@ public:
     }
     void layoutLineGridBox();
 
-    RenderMultiColumnFlow* multiColumnFlow() const { return hasRareBlockFlowData() ? rareBlockFlowData()->m_multiColumnFlow.get() : nullptr; }
+    RenderMultiColumnFlow* multiColumnFlow() const { return hasRareBlockFlowData() ? multiColumnFlowSlowCase() : nullptr; }
+    RenderMultiColumnFlow* multiColumnFlowSlowCase() const;
     void setMultiColumnFlow(RenderMultiColumnFlow&);
     void clearMultiColumnFlow();
     bool willCreateColumns(Optional<unsigned> desiredColumnCount = WTF::nullopt) const;
index c866c34..9bb9a53 100644 (file)
@@ -107,6 +107,11 @@ RenderBox* RenderMultiColumnFlow::previousColumnSetOrSpannerSiblingOf(const Rend
     return nullptr;
 }
 
+RenderMultiColumnSpannerPlaceholder* RenderMultiColumnFlow::findColumnSpannerPlaceholder(RenderBox* spanner) const
+{
+    return m_spannerMap->get(spanner).get();
+}
+
 void RenderMultiColumnFlow::layout()
 {
     ASSERT(!m_inLayout);
index a48a789..bca4d94 100644 (file)
@@ -48,7 +48,7 @@ public:
     static RenderBox* nextColumnSetOrSpannerSiblingOf(const RenderBox*);
     static RenderBox* previousColumnSetOrSpannerSiblingOf(const RenderBox*);
 
-    RenderMultiColumnSpannerPlaceholder* findColumnSpannerPlaceholder(RenderBox* spanner) const { return m_spannerMap->get(spanner).get(); }
+    RenderMultiColumnSpannerPlaceholder* findColumnSpannerPlaceholder(RenderBox* spanner) const;
 
     void layout() override;
 
index 221851f..186ca87 100644 (file)
@@ -96,6 +96,31 @@ RenderTable::RenderTable(Document& document, RenderStyle&& style)
 
 RenderTable::~RenderTable() = default;
 
+RenderTableSection* RenderTable::header() const
+{
+    return m_head.get();
+}
+
+RenderTableSection* RenderTable::footer() const
+{
+    return m_foot.get();
+}
+
+RenderTableSection* RenderTable::firstBody() const
+{
+    return m_firstBody.get();
+}
+
+RenderTableSection* RenderTable::topSection() const
+{
+    ASSERT(!needsSectionRecalc());
+    if (m_head)
+        return m_head.get();
+    if (m_firstBody)
+        return m_firstBody.get();
+    return m_foot.get();
+}
+
 void RenderTable::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderBlock::styleDidChange(diff, oldStyle);
index 346cf5d..a71e269 100644 (file)
@@ -152,9 +152,9 @@ public:
         m_columnPos[index] = position;
     }
 
-    RenderTableSection* header() const { return m_head.get(); }
-    RenderTableSection* footer() const { return m_foot.get(); }
-    RenderTableSection* firstBody() const { return m_firstBody.get(); }
+    RenderTableSection* header() const;
+    RenderTableSection* footer() const;
+    RenderTableSection* firstBody() const;
 
     // This function returns 0 if the table has no section.
     RenderTableSection* topSection() const;
@@ -370,16 +370,6 @@ private:
     bool m_inRecursiveSectionMovedWithPagination { false };
 };
 
-inline RenderTableSection* RenderTable::topSection() const
-{
-    ASSERT(!needsSectionRecalc());
-    if (m_head)
-        return m_head.get();
-    if (m_firstBody)
-        return m_firstBody.get();
-    return m_foot.get();
-}
-
 inline bool isDirectionSame(const RenderBox* tableItem, const RenderBox* otherTableItem) { return tableItem && otherTableItem ? tableItem->style().direction() == otherTableItem->style().direction() : true; }
 
 inline RenderPtr<RenderBox> RenderTable::createAnonymousBoxWithSameTypeAs(const RenderBox& renderer) const
index a5d1997..24651d5 100644 (file)
@@ -1,3 +1,33 @@
+2019-05-29  Geoffrey Garen  <ggaren@apple.com>
+
+        WeakPtr breaks vtables when upcasting to base classes
+        https://bugs.webkit.org/show_bug.cgi?id=188799
+
+        Reviewed by Youenn Fablet.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::networkSession):
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
+        * Shared/WebBackForwardListItem.cpp:
+        (WebKit::WebBackForwardListItem::suspendedPage const):
+        * Shared/WebBackForwardListItem.h:
+        (WebKit::WebBackForwardListItem::suspendedPage const): Deleted. Moved
+        functions out of line to avoid #include explosion for .get().
+
+        * UIProcess/Authentication/cocoa/SecKeyProxyStore.h:
+        (WebKit::SecKeyProxyStore::get const):
+        (WebKit::SecKeyProxyStore::weakPtrFactory const): Deleted. Adopted
+        CanMakeWeakPtr.
+
+        * UIProcess/WebAuthentication/AuthenticatorManager.h:
+        * UIProcess/WebProcessProxy.cpp: It takes an extra using declaration
+        to disambiguate multiple CanMakeWeakPtr base classes now.
+
+        (WebKit::WebProcessProxy::processPool const):
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::processPool const): Deleted. Moved
+        functions out of line to avoid #include explosion for .get().
+
 2019-05-29  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r245857.
index 622430c..914d35b 100644 (file)
@@ -1016,6 +1016,11 @@ void WebResourceLoadStatisticsStore::notifyResourceLoadStatisticsProcessed()
         m_networkSession->notifyResourceLoadStatisticsProcessed();
 }
 
+NetworkSession* WebResourceLoadStatisticsStore::networkSession()
+{
+    return m_networkSession.get();
+}
+
 void WebResourceLoadStatisticsStore::deleteWebsiteDataForRegistrableDomains(OptionSet<WebsiteDataType> dataTypes, HashMap<RegistrableDomain, WebsiteDataToRemove>&& domainsToRemoveWebsiteDataFor, bool shouldNotifyPage, CompletionHandler<void(const HashSet<RegistrableDomain>&)>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
index 79cf246..a8c23e0 100644 (file)
@@ -175,7 +175,7 @@ public:
 
     void notifyResourceLoadStatisticsProcessed();
 
-    NetworkSession* networkSession() { return m_networkSession.get(); }
+    NetworkSession* networkSession();
 
     void sendDiagnosticMessageWithValue(const String& message, const String& description, unsigned value, unsigned sigDigits, WebCore::ShouldSample) const;
     void notifyPageStatisticsTelemetryFinished(unsigned totalPrevalentResources, unsigned totalPrevalentResourcesWithUserInteraction, unsigned top3SubframeUnderTopFrameOrigins) const;
index d900a16..db72ca1 100644 (file)
@@ -168,6 +168,11 @@ void WebBackForwardListItem::setSuspendedPage(SuspendedPageProxy* page)
     m_suspendedPage = makeWeakPtr(page);
 }
 
+SuspendedPageProxy* WebBackForwardListItem::suspendedPage() const
+{
+    return m_suspendedPage.get();
+}
+
 void WebBackForwardListItem::removeSuspendedPageFromProcessPool()
 {
     if (!m_suspendedPage)
index 44af7c6..5b2d2e0 100644 (file)
@@ -75,7 +75,7 @@ public:
     void setSnapshot(RefPtr<ViewSnapshot>&& snapshot) { m_itemState.snapshot = WTFMove(snapshot); }
 #endif
     void setSuspendedPage(SuspendedPageProxy*);
-    SuspendedPageProxy* suspendedPage() const { return m_suspendedPage.get(); }
+    SuspendedPageProxy* suspendedPage() const;
 
 #if !LOG_DISABLED
     const char* loggingString();
index c29212e..288555f 100644 (file)
@@ -24,6 +24,7 @@
 #include "WebFrameProxy.h"
 #include "WebKitURIRequest.h"
 #include "WebKitWebResourcePrivate.h"
+#include "WebPageProxy.h"
 #include <glib/gi18n-lib.h>
 #include <wtf/glib/GRefPtr.h>
 #include <wtf/glib/WTFGType.h>
index bec75cf..60e4a1b 100644 (file)
@@ -39,7 +39,7 @@ class Credential;
 
 namespace WebKit {
 
-class SecKeyProxyStore : public RefCounted<SecKeyProxyStore> {
+class SecKeyProxyStore : public RefCounted<SecKeyProxyStore>, public CanMakeWeakPtr<SecKeyProxyStore> {
 public:
     static Ref<SecKeyProxyStore> create() { return adoptRef(* new SecKeyProxyStore()); }
 
@@ -47,12 +47,10 @@ public:
     bool isInitialized() const { return !!m_secKeyProxy; }
 
     auto* get() const { return m_secKeyProxy.get(); }
-    auto& weakPtrFactory() const { return m_weakPtrFactory; }
 
 private:
     SecKeyProxyStore() = default;
 
-    WeakPtrFactory<SecKeyProxyStore> m_weakPtrFactory;
     RetainPtr<SecKeyProxy> m_secKeyProxy;
 };
 
index b03fe57..ce06c50 100644 (file)
@@ -49,6 +49,7 @@ public:
     using TransportSet = HashSet<WebCore::AuthenticatorTransport, WTF::IntHash<WebCore::AuthenticatorTransport>, WTF::StrongEnumHashTraits<WebCore::AuthenticatorTransport>>;
 
     using AuthenticatorTransportService::Observer::weakPtrFactory;
+    typedef AuthenticatorTransportService::Observer::WeakValueType WeakValueType;
 
     AuthenticatorManager();
     virtual ~AuthenticatorManager() = default;
index 67831e2..5603c8a 100644 (file)
@@ -1513,6 +1513,12 @@ void WebProcessProxy::decrementSuspendedPageCount()
         send(Messages::WebProcess::SetHasSuspendedPageProxy(false), 0);
 }
 
+WebProcessPool& WebProcessProxy::processPool() const
+{
+    ASSERT(m_processPool);
+    return *m_processPool.get();
+}
+
 #if PLATFORM(WATCHOS)
 
 void WebProcessProxy::takeBackgroundActivityTokenForFullscreenInput()
index ad3f038..063e861 100644 (file)
@@ -119,7 +119,7 @@ public:
     void incrementSuspendedPageCount();
     void decrementSuspendedPageCount();
 
-    WebProcessPool& processPool() const { ASSERT(m_processPool); return *m_processPool.get(); }
+    WebProcessPool& processPool() const;
 
     WebCore::RegistrableDomain registrableDomain() const { return m_registrableDomain.valueOr(WebCore::RegistrableDomain { }); }
     void setIsInProcessCache(bool);
index a2ccc8f..be614c1 100644 (file)
@@ -1,3 +1,29 @@
+2019-05-29  Geoffrey Garen  <ggaren@apple.com>
+
+        WeakPtr breaks vtables when upcasting to base classes
+        https://bugs.webkit.org/show_bug.cgi?id=188799
+
+        Reviewed by Youenn Fablet.
+
+        * TestWebKitAPI/Tests/WTF/WeakPtr.cpp: Adopt the new macro API instead
+        of template specialization for observing weak references.
+
+        (TestWebKitAPI::Int::Int):
+        (TestWebKitAPI::Int::operator int const):
+        (TestWebKitAPI::Int::operator== const): Use a class for integer tests
+        because WeakPtr doesn't naturally support pointing to non-class objects
+        now.
+
+        (TestWebKitAPI::Base::foo):
+        (TestWebKitAPI::Derived::foo): Inherit from CanMakeWeakPtr to enable
+        deduction of the weak pointer type.
+
+        (TestWebKitAPI::TEST): Updated to use Int.
+
+        (TestWebKitAPI::Base::weakPtrFactory const): Deleted.
+        (WTF::WeakReference<TestWebKitAPI::Base>::WeakReference): Deleted.
+        (WTF::WeakReference<TestWebKitAPI::Base>::~WeakReference): Deleted.
+
 2019-05-29  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r245857.
index e0e7c31..62ce632 100644 (file)
 
 #include "config.h"
 
+static unsigned s_baseWeakReferences = 0;
+
+#define DID_CREATE_WEAK_REFERENCE(p) do { \
+    ++s_baseWeakReferences; \
+} while (0);
+
+#define WILL_DESTROY_WEAK_REFERENCE(p) do { \
+    --s_baseWeakReferences; \
+} while (0);
+
 #include "Test.h"
 #include <wtf/HashSet.h>
 #include <wtf/WeakHashSet.h>
 #include <wtf/WeakPtr.h>
 
-static unsigned s_baseWeakReferences = 0;
-
 namespace TestWebKitAPI {
 
-class Base {
+struct Int : public CanMakeWeakPtr<Int> {
+    Int(int i) : m_i(i) { }
+    operator int() const { return m_i; }
+    bool operator==(const Int& other) const { return m_i == other.m_i; }
+    int m_i;
+};
+
+class Base : public CanMakeWeakPtr<Base> {
 public:
     Base() { }
 
@@ -43,16 +58,15 @@ public:
         return 0;
     }
 
-    auto& weakPtrFactory() const { return m_weakPtrFactory; }
-
-private:
-    WeakPtrFactory<Base> m_weakPtrFactory;
+    int dummy; // Prevent empty base class optimization, to make testing more interesting.
 };
 
 class Derived : public Base {
 public:
     Derived() { }
 
+    virtual ~Derived() { } // Force a pointer fixup when casting Base <-> Derived
+
     int foo()
     {
         return 1;
@@ -61,31 +75,15 @@ public:
 
 }
 
-namespace WTF {
-
-template<>
-WeakReference<TestWebKitAPI::Base>::WeakReference(TestWebKitAPI::Base* ptr)
-    : m_ptr(ptr)
-{
-    ++s_baseWeakReferences;
-}
-template<>
-WeakReference<TestWebKitAPI::Base>::~WeakReference()
-{
-    --s_baseWeakReferences;
-}
-
-}
-
 namespace TestWebKitAPI {
 
 TEST(WTF_WeakPtr, Basic)
 {
-    int dummy = 5;
-    WeakPtrFactory<int>* factory = new WeakPtrFactory<int>();
-    WeakPtr<int> weakPtr1 = factory->createWeakPtr(dummy);
-    WeakPtr<int> weakPtr2 = factory->createWeakPtr(dummy);
-    WeakPtr<int> weakPtr3 = factory->createWeakPtr(dummy);
+    Int dummy(5);
+    WeakPtrFactory<Int>* factory = new WeakPtrFactory<Int>();
+    WeakPtr<Int> weakPtr1 = factory->createWeakPtr(dummy);
+    WeakPtr<Int> weakPtr2 = factory->createWeakPtr(dummy);
+    WeakPtr<Int> weakPtr3 = factory->createWeakPtr(dummy);
     EXPECT_EQ(weakPtr1.get(), &dummy);
     EXPECT_EQ(weakPtr2.get(), &dummy);
     EXPECT_EQ(weakPtr3.get(), &dummy);
@@ -106,10 +104,10 @@ TEST(WTF_WeakPtr, Basic)
 
 TEST(WTF_WeakPtr, Assignment)
 {
-    int dummy = 5;
-    WeakPtr<int> weakPtr;
+    Int dummy(5);
+    WeakPtr<Int> weakPtr;
     {
-        WeakPtrFactory<int> factory;
+        WeakPtrFactory<Int> factory;
         EXPECT_NULL(weakPtr.get());
         weakPtr = factory.createWeakPtr(dummy);
         EXPECT_EQ(weakPtr.get(), &dummy);
@@ -119,12 +117,12 @@ TEST(WTF_WeakPtr, Assignment)
 
 TEST(WTF_WeakPtr, MultipleFactories)
 {
-    int dummy1 = 5;
-    int dummy2 = 7;
-    WeakPtrFactory<int>* factory1 = new WeakPtrFactory<int>();
-    WeakPtrFactory<int>* factory2 = new WeakPtrFactory<int>();
-    WeakPtr<int> weakPtr1 = factory1->createWeakPtr(dummy1);
-    WeakPtr<int> weakPtr2 = factory2->createWeakPtr(dummy2);
+    Int dummy1(5);
+    Int dummy2(7);
+    WeakPtrFactory<Int>* factory1 = new WeakPtrFactory<Int>();
+    WeakPtrFactory<Int>* factory2 = new WeakPtrFactory<Int>();
+    WeakPtr<Int> weakPtr1 = factory1->createWeakPtr(dummy1);
+    WeakPtr<Int> weakPtr2 = factory2->createWeakPtr(dummy2);
     EXPECT_EQ(weakPtr1.get(), &dummy1);
     EXPECT_EQ(weakPtr2.get(), &dummy2);
     EXPECT_TRUE(weakPtr1 != weakPtr2);
@@ -139,11 +137,11 @@ TEST(WTF_WeakPtr, MultipleFactories)
 
 TEST(WTF_WeakPtr, RevokeAll)
 {
-    int dummy = 5;
-    WeakPtrFactory<int> factory;
-    WeakPtr<int> weakPtr1 = factory.createWeakPtr(dummy);
-    WeakPtr<int> weakPtr2 = factory.createWeakPtr(dummy);
-    WeakPtr<int> weakPtr3 = factory.createWeakPtr(dummy);
+    Int dummy(5);
+    WeakPtrFactory<Int> factory;
+    WeakPtr<Int> weakPtr1 = factory.createWeakPtr(dummy);
+    WeakPtr<Int> weakPtr2 = factory.createWeakPtr(dummy);
+    WeakPtr<Int> weakPtr3 = factory.createWeakPtr(dummy);
     EXPECT_EQ(weakPtr1.get(), &dummy);
     EXPECT_EQ(weakPtr2.get(), &dummy);
     EXPECT_EQ(weakPtr3.get(), &dummy);
@@ -153,7 +151,7 @@ TEST(WTF_WeakPtr, RevokeAll)
     EXPECT_NULL(weakPtr3.get());
 }
 
-struct Foo {
+struct Foo : public CanMakeWeakPtr<Foo> {
     void bar() { };
 };
 
@@ -185,13 +183,13 @@ TEST(WTF_WeakPtr, Operators)
 
 TEST(WTF_WeakPtr, Forget)
 {
-    int dummy = 5;
-    int dummy2 = 7;
+    Int dummy(5);
+    Int dummy2(7);
 
-    WeakPtrFactory<int> outerFactory;
-    WeakPtr<int> weakPtr1, weakPtr2, weakPtr3, weakPtr4;
+    WeakPtrFactory<Int> outerFactory;
+    WeakPtr<Int> weakPtr1, weakPtr2, weakPtr3, weakPtr4;
     {
-        WeakPtrFactory<int> innerFactory;
+        WeakPtrFactory<Int> innerFactory;
         weakPtr1 = innerFactory.createWeakPtr(dummy);
         weakPtr2 = innerFactory.createWeakPtr(dummy);
         weakPtr3 = innerFactory.createWeakPtr(dummy);
@@ -217,7 +215,7 @@ TEST(WTF_WeakPtr, Forget)
         EXPECT_EQ(weakPtr2.get(), &dummy);
         EXPECT_EQ(weakPtr4.get(), &dummy);
 
-        WeakPtr<int> weakPtr5 = weakPtr2;
+        WeakPtr<Int> weakPtr5 = weakPtr2;
         EXPECT_EQ(weakPtr2.get(), &dummy);
         EXPECT_EQ(weakPtr5.get(), &dummy);
         weakPtr5.clear();
@@ -233,16 +231,16 @@ TEST(WTF_WeakPtr, Forget)
     EXPECT_NULL(weakPtr2.get());
     EXPECT_EQ(weakPtr4.get(), &dummy2);
 
-    WeakPtr<int> weakPtr5 = weakPtr4;
+    WeakPtr<Int> weakPtr5 = weakPtr4;
     EXPECT_EQ(weakPtr4.get(), &dummy2);
     EXPECT_EQ(weakPtr5.get(), &dummy2);
     weakPtr5.clear();
     EXPECT_NULL(weakPtr5.get());
-    WeakPtr<int> weakPtr6 = weakPtr5;
+    WeakPtr<Int> weakPtr6 = weakPtr5;
     EXPECT_NULL(weakPtr6.get());
     EXPECT_EQ(weakPtr5.get(), weakPtr6.get());
 
-    WeakPtr<int> weakPtr7 = outerFactory.createWeakPtr(dummy2);
+    WeakPtr<Int> weakPtr7 = outerFactory.createWeakPtr(dummy2);
     EXPECT_EQ(weakPtr7.get(), &dummy2);
     weakPtr7 = nullptr;
     EXPECT_NULL(weakPtr7.get());
@@ -250,8 +248,8 @@ TEST(WTF_WeakPtr, Forget)
 
 TEST(WTF_WeakPtr, Downcasting)
 {
-    int dummy0 = 0;
-    int dummy1 = 1;
+    int dummy0(0);
+    int dummy1(1);
 
     WeakPtr<Base> baseWeakPtr;
     WeakPtr<Derived> derivedWeakPtr;