Attachment 370988 Details for Bug 198390 – Patch

[patch] Patch

bug-198390-20190530155632.patch (text/plain), 18.98 KB, created by Geoffrey Garen on 2019-05-30 15:56:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Geoffrey Garen

Created: 2019-05-30 15:56:32 PDT

Size: 18.98 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 245906)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,84 @@
>+2019-05-30 Geoffrey Garen <ggaren@apple.com>
>+
>+ Some WeakPtr cleanup
>+ https://bugs.webkit.org/show_bug.cgi?id=198390
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ * wtf/WeakHashSet.h:
>+ (WTF::HashTraits<Ref<WeakPtrImpl>>::isReleasedWeakValue):
>+ (WTF::WeakHashSet::WeakHashSetConstIterator::WeakHashSetConstIterator):
>+ Updated for rename to WeakPtrImpl.
>+
>+ (WTF::WeakHashSet::WeakHashSetConstIterator::get const): Updated for new
>+ get() interface. Also, switched to iterator operator* to help clarify
>+ the double dereference here.
>+
>+ (WTF::WeakHashSet::add):
>+ (WTF::WeakHashSet::remove):
>+ (WTF::WeakHashSet::contains const):
>+ (WTF::WeakHashSet::computeSize const):
>+ (WTF::HashTraits<Ref<WeakReference>>::isReleasedWeakValue): Deleted.
>+ Updated for rename to WeakPtrImpl.
>+
>+ * wtf/WeakPtr.h:
>+ (WTF::WeakPtrImpl::create):
>+ (WTF::WeakPtrImpl::~WeakPtrImpl): Renamed WeakReference to WeakPtrImpl.
>+ Now we don't need a comment explaining that this class is the backing
>+ implementation of WeakPtr.
>+
>+ (WTF::WeakPtrImpl::get): Return the pointer type we stored, rather than
>+ the pointer type requested by our client. It's a little too surprising
>+ for a field to store one pointer type and load another.
>+
>+ (WTF::WeakPtrImpl::WeakPtrImpl): Fixed a theoretical type safety bug.
>+ Make sure to store T::WeakValueType* instead of T*, since they might
>+ not be the same pointer value. (In practice, T and T::WeakValueType*
>+ are always the same type in this constructor because WeakPtrFactory
>+ makes them so, but it's best not to depend on implementation details
>+ across classes.)
>+
>+ (WTF::WeakPtr::get const): Updated for new get() interface.
>+
>+ (WTF::WeakPtr::operator bool const):
>+ (WTF::WeakPtr::operator=):
>+ (WTF::WeakPtr::clear):
>+ (WTF::WeakPtr::WeakPtr): Updated for WeakPtrImpl rename.
>+
>+ (WTF::WeakPtrFactory::~WeakPtrFactory): Updated for WeakPtrImpl rename.
>+
>+ (WTF::WeakPtrFactory::createWeakPtr const): ASSERT that the passed-in
>+ pointer is equal to the stored pointer. As a space optimization, we
>+ require our client to remind us what we point to each time a weak
>+ pointer is created -- but nothing guarantees that our client will do
>+ this correctly.
>+
>+ (WTF::WeakPtrFactory::revokeAll): Updated for WeakPtrImpl rename.
>+
>+ (WTF::CanMakeWeakPtr::weakPtrFactory const):
>+ (WTF::CanMakeWeakPtr::weakPtrFactory): Use idiomatic accessor naming.
>+
>+ (WTF::weak_ptr_impl_cast): Fixed a theoretical type safety bug.
>+ Previously, if Base and Derived both inherited CanMakeWeakPtr, and
>+ you casted WeakPtr<Base> to WeakPtr<Derived> (or vice versa), and
>+ casting Base <-> Derived required pointer fixup, the previous
>+ compile-time check would accept the cast, even though the stored pointer
>+ would be wrong.
>+
>+ (WTF::WeakPtr<T>::WeakPtr):
>+ (WTF::=):
>+ (WTF::makeWeakPtr):
>+ (WTF::WeakReference::create): Deleted.
>+ (WTF::WeakReference::~WeakReference): Deleted.
>+ (WTF::WeakReference::get const): Deleted.
>+ (WTF::WeakReference::operator bool const): Deleted.
>+ (WTF::WeakReference::clear): Deleted.
>+ (WTF::WeakReference::WeakReference): Deleted.
>+ (WTF::weak_reference_cast): Deleted. Updated for rename to WeakPtrImpl.
>+
>+ Don't export WeakPtrImpl because it's an implmenetation detail and
>+ it shouldn't be easy to use outside WTF.
>+
> 2019-05-30 Keith Rollin <krollin@apple.com>
> 
> Fix yet more deprecated uses of -[UIApplication interfaceOrientation]
>Index: Source/WTF/wtf/WeakHashSet.h
>===================================================================
>--- Source/WTF/wtf/WeakHashSet.h	(revision 245906)
>+++ Source/WTF/wtf/WeakHashSet.h	(working copy)
>@@ -32,9 +32,9 @@
> 
> namespace WTF {
> 
>-template<> struct HashTraits<Ref<WeakReference>> : RefHashTraits<WeakReference> {
>+template<> struct HashTraits<Ref<WeakPtrImpl>> : RefHashTraits<WeakPtrImpl> {
> static const bool hasIsReleasedWeakValueFunction = true;
>- static bool isReleasedWeakValue(const Ref<WeakReference>& value)
>+ static bool isReleasedWeakValue(const Ref<WeakPtrImpl>& value)
> {
> return !value.isHashTableDeletedValue() && !value.isHashTableEmptyValue() && !value.get();
> }
>@@ -43,18 +43,18 @@ template<> struct HashTraits<Ref<WeakRef
> template <typename T>
> class WeakHashSet {
> public:
>- typedef HashSet<Ref<WeakReference>> WeakReferenceSet;
>+ typedef HashSet<Ref<WeakPtrImpl>> WeakPtrImplSet;
> 
> class WeakHashSetConstIterator : public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, const T*, const T&> {
> private:
>- WeakHashSetConstIterator(const WeakReferenceSet& set, typename WeakReferenceSet::const_iterator position)
>+ WeakHashSetConstIterator(const WeakPtrImplSet& set, typename WeakPtrImplSet::const_iterator position)
> : m_position(position), m_endPosition(set.end())
> {
> skipEmptyBuckets();
> }
> 
> public:
>- T* get() const { return m_position->get().template get<T, typename T::WeakValueType>(); }
>+ T* get() const { return static_cast<T*>((*m_position)->template get<T>()); }
> T& operator*() const { return *get(); }
> T* operator->() const { return get(); }
> 
>@@ -85,8 +85,8 @@ public:
> private:
> template <typename> friend class WeakHashSet;
> 
>- typename WeakReferenceSet::const_iterator m_position;
>- typename WeakReferenceSet::const_iterator m_endPosition;
>+ typename WeakPtrImplSet::const_iterator m_position;
>+ typename WeakPtrImplSet::const_iterator m_endPosition;
> };
> typedef WeakHashSetConstIterator const_iterator;
> 
>@@ -98,25 +98,25 @@ public:
> template <typename U>
> void add(const U& value)
> {
>- m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_ref);
>+ m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_impl);
> }
> 
> template <typename U>
> bool remove(const U& value)
> {
>- auto& weakReference = value.weakPtrFactory().m_ref;
>- if (!weakReference || !*weakReference)
>+ auto& weakPtrImpl = value.weakPtrFactory().m_impl;
>+ if (!weakPtrImpl || !*weakPtrImpl)
> return false;
>- return m_set.remove(*weakReference);
>+ return m_set.remove(*weakPtrImpl);
> }
> 
> template <typename U>
> bool contains(const U& value) const
> {
>- auto& weakReference = value.weakPtrFactory().m_ref;
>- if (!weakReference || !*weakReference)
>+ auto& weakPtrImpl = value.weakPtrFactory().m_impl;
>+ if (!weakPtrImpl || !*weakPtrImpl)
> return false;
>- return m_set.contains(*weakReference);
>+ return m_set.contains(*weakPtrImpl);
> }
> 
> unsigned capacity() const { return m_set.capacity(); }
>@@ -130,7 +130,7 @@ public:
> 
> unsigned computeSize() const
> {
>- const_cast<WeakReferenceSet&>(m_set).removeIf([] (auto& value) { return !value.get(); });
>+ const_cast<WeakPtrImplSet&>(m_set).removeIf([] (auto& value) { return !value.get(); });
> return m_set.size();
> }
> 
>@@ -141,7 +141,7 @@ public:
> #endif
> 
> private:
>- WeakReferenceSet m_set;
>+ WeakPtrImplSet m_set;
> };
> 
> } // namespace WTF
>Index: Source/WTF/wtf/WeakPtr.h
>===================================================================
>--- Source/WTF/wtf/WeakPtr.h	(revision 245906)
>+++ Source/WTF/wtf/WeakPtr.h	(working copy)
>@@ -34,39 +34,44 @@
> namespace WTF {
> 
> // Testing interface for TestWebKitAPI
>-#ifndef DID_CREATE_WEAK_REFERENCE
>-#define DID_CREATE_WEAK_REFERENCE(p)
>+#ifndef DID_CREATE_WEAK_PTR_IMPL
>+#define DID_CREATE_WEAK_PTR_IMPL(p)
> #endif
>-#ifndef WILL_DESTROY_WEAK_REFERENCE
>-#define WILL_DESTROY_WEAK_REFERENCE(p)
>+#ifndef WILL_DESTROY_WEAK_PTR_IMPL
>+#define WILL_DESTROY_WEAK_PTR_IMPL(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.
>-class WeakReference : public ThreadSafeRefCounted<WeakReference> {
>- WTF_MAKE_NONCOPYABLE(WeakReference);
>+class WeakPtrImpl : public ThreadSafeRefCounted<WeakPtrImpl> {
>+ WTF_MAKE_NONCOPYABLE(WeakPtrImpl);
> WTF_MAKE_FAST_ALLOCATED;
> public:
>- template<typename T> static Ref<WeakReference> create(T* ptr) { return adoptRef(*new WeakReference(ptr)); }
>+ template<typename T> static Ref<WeakPtrImpl> create(T* ptr)
>+ {
>+ return adoptRef(*new WeakPtrImpl(ptr));
>+ }
> 
>- ~WeakReference()
>+ ~WeakPtrImpl()
> {
>- WILL_DESTROY_WEAK_REFERENCE(m_ptr);
>+ WILL_DESTROY_WEAK_PTR_IMPL(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; }
>+ template<typename T> typename T::WeakValueType* get()
>+ {
>+ return static_cast<typename T::WeakValueType*>(m_ptr);
>+ }
> 
>+ explicit operator bool() const { return m_ptr; }
> void clear() { m_ptr = nullptr; }
> 
> private:
>- template<typename T> explicit WeakReference(T* ptr)
>- : m_ptr(ptr)
>+ template<typename T> explicit WeakPtrImpl(T* ptr)
>+ : m_ptr(static_cast<typename T::WeakValueType*>(ptr))
> {
>- DID_CREATE_WEAK_REFERENCE(ptr);
>+ DID_CREATE_WEAK_PTR_IMPL(ptr);
> }
> 
> void* m_ptr;
>@@ -81,26 +86,26 @@ public:
> template<typename U> WeakPtr(const WeakPtr&);
> template<typename U> WeakPtr(WeakPtr&&);
> 
>- T* get() const { return m_ref ? m_ref->template get<T, typename T::WeakValueType>() : nullptr; }
>- explicit operator bool() const { return m_ref && *m_ref; }
>+ T* get() const { return m_impl ? static_cast<T*>(m_impl->get<T>()) : nullptr; }
>+ explicit operator bool() const { return m_impl && *m_impl; }
> 
>- WeakPtr& operator=(std::nullptr_t) { m_ref = nullptr; return *this; }
>+ WeakPtr& operator=(std::nullptr_t) { m_impl = nullptr; return *this; }
> template<typename U> WeakPtr& operator=(const WeakPtr&);
> template<typename U> WeakPtr& operator=(WeakPtr&&);
> 
> T* operator->() const { return get(); }
> T& operator*() const { return *get(); }
> 
>- void clear() { m_ref = nullptr; }
>+ void clear() { m_impl = nullptr; }
> 
> private:
>- explicit WeakPtr(Ref<WeakReference>&& ref) : m_ref(std::move(ref)) { }
>+ explicit WeakPtr(Ref<WeakPtrImpl>&& ref) : m_impl(WTFMove(ref)) { }
> template<typename> friend class WeakHashSet;
> template<typename> friend class WeakPtr;
> template<typename> friend class WeakPtrFactory;
> template<typename U> friend WeakPtr makeWeakPtr(U&);
> 
>- RefPtr<WeakReference> m_ref;
>+ RefPtr<WeakPtrImpl> m_impl;
> };
> 
> // Note: you probably want to inherit from CanMakeWeakPtr rather than use this directly.
>@@ -112,82 +117,86 @@ public:
> WeakPtrFactory() = default;
> ~WeakPtrFactory()
> {
>- if (!m_ref)
>+ if (!m_impl)
> return;
>- m_ref->clear();
>+ m_impl->clear();
> }
> 
>- WeakPtr<T> createWeakPtr(T& ptr) const
>+ WeakPtr<T> createWeakPtr(T& object) const
> {
>- if (!m_ref)
>- m_ref = WeakReference::create(&ptr);
>- return WeakPtr<T>(makeRef(*m_ref));
>+ if (!m_impl)
>+ m_impl = WeakPtrImpl::create(&object);
>+
>+ ASSERT(&object == m_impl->get<T>());
>+ return WeakPtr<T>(makeRef(*m_impl));
> }
> 
>- WeakPtr<const T> createWeakPtr(const T& ptr) const
>+ WeakPtr<const T> createWeakPtr(const T& object) const
> {
>- if (!m_ref)
>- m_ref = WeakReference::create(const_cast<T*>(&ptr));
>- return WeakPtr<T>(makeRef(*m_ref));
>+ if (!m_impl)
>+ m_impl = WeakPtrImpl::create(const_cast<T*>(&object));
>+
>+ ASSERT(&object == m_impl->get<T>());
>+ return WeakPtr<T>(makeRef(*m_impl));
> }
> 
> void revokeAll()
> {
>- if (!m_ref)
>+ if (!m_impl)
> return;
> 
>- m_ref->clear();
>- m_ref = nullptr;
>+ m_impl->clear();
>+ m_impl = nullptr;
> }
> 
> private:
> template<typename> friend class WeakHashSet;
> 
>- mutable RefPtr<WeakReference> m_ref;
>+ mutable RefPtr<WeakPtrImpl> m_impl;
> };
> 
> template<typename T> class CanMakeWeakPtr {
> public:
> typedef T WeakValueType;
> 
>- const WeakPtrFactory<T>& weakPtrFactory() const { return m_weakFactory; }
>- WeakPtrFactory<T>& weakPtrFactory() { return m_weakFactory; }
>+ const WeakPtrFactory<T>& weakPtrFactory() const { return m_weakPtrFactory; }
>+ WeakPtrFactory<T>& weakPtrFactory() { return m_weakPtrFactory; }
> 
> private:
>- WeakPtrFactory<T> m_weakFactory;
>+ WeakPtrFactory<T> m_weakPtrFactory;
> };
> 
>-template<typename T, typename U> inline WeakReference* weak_reference_cast(WeakReference* weakReference)
>+template<typename T, typename U> inline WeakPtrImpl* weak_ptr_impl_cast(WeakPtrImpl* impl)
> {
>- UNUSED_VARIABLE(static_cast<T*>(static_cast<typename U::WeakValueType*>(nullptr))); // Verify that casting is valid.
>- return weakReference;
>+ static_assert(std::is_same<typename T::WeakValueType, typename U::WeakValueType>::value, "Invalid weak pointer cast");
>+ return impl;
> }
> 
> template<typename T> template<typename U> inline WeakPtr<T>::WeakPtr(const WeakPtr& o)
>- : m_ref(weak_reference_cast<T, U>(o.m_ref.get()))
>+ : m_impl(weak_ptr_impl_cast<T, U>(o.m_impl.get()))
> {
> }
> 
> template<typename T> template<typename U> inline WeakPtr<T>::WeakPtr(WeakPtr&& o)
>- : m_ref(adoptRef(weak_reference_cast<T, U>(o.m_ref.leakRef())))
>+ : m_impl(adoptRef(weak_ptr_impl_cast<T, U>(o.m_impl.leakRef())))
> {
> }
> 
> template<typename T> template<typename U> inline WeakPtr<T>& WeakPtr<T>::operator=(const WeakPtr& o)
> {
>- m_ref = weak_reference_cast<T, U>(o.m_ref.get());
>+ m_impl = weak_ptr_impl_cast<T, U>(o.m_impl.get());
> return *this;
> }
> 
> template<typename T> template<typename U> inline WeakPtr<T>& WeakPtr<T>::operator=(WeakPtr&& o)
> {
>- m_ref = adoptRef(weak_reference_cast<T, U>(o.m_ref.leakRef()));
>+ m_impl = adoptRef(weak_ptr_impl_cast<T, U>(o.m_impl.leakRef()));
> return *this;
> }
> 
>-template<typename T> inline WeakPtr<T> makeWeakPtr(T& ref)
>+template<typename T> inline WeakPtr<T> makeWeakPtr(T& object)
> {
>- return { ref.weakPtrFactory().createWeakPtr(ref) };
>+ return { object.weakPtrFactory().createWeakPtr(object) };
> }
> 
> template<typename T> inline WeakPtr<T> makeWeakPtr(T* ptr)
>@@ -232,5 +241,4 @@ template<typename T, typename U> inline 
> using WTF::CanMakeWeakPtr;
> using WTF::WeakPtr;
> using WTF::WeakPtrFactory;
>-using WTF::WeakReference;
> using WTF::makeWeakPtr;
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 245906)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2019-05-30 Geoffrey Garen <ggaren@apple.com>
>+
>+ Some WeakPtr cleanup
>+ https://bugs.webkit.org/show_bug.cgi?id=198390
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ * Modules/indexeddb/shared/InProcessIDBServer.cpp:
>+ (WebCore::storageQuotaManagerGetter): Dereference the weak pointer
>+ directly instead of using a weak pointer to guard a raw pointer. It's
>+ safer and more idiomatic to use weak pointers directly.
>+
>+ * Modules/indexeddb/shared/InProcessIDBServer.h: Use our base clase
>+ weakPtrFactory() definition instead of writing our own. Declare
>+ WeakValueType so we can dereference the weak pointer we create (above).
>+
> 2019-05-30 Justin Fan <justin_fan@apple.com>
> 
> [Web GPU] Vertex Buffers/Input State API updates
>Index: Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp
>===================================================================
>--- Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp	(revision 245906)
>+++ Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp	(working copy)
>@@ -69,8 +69,8 @@ StorageQuotaManager* InProcessIDBServer:
> 
> static inline IDBServer::IDBServer::QuotaManagerGetter storageQuotaManagerGetter(InProcessIDBServer& server)
> {
>- return [&server, weakServer = makeWeakPtr(server)](PAL::SessionID, const auto& origin) {
>- return weakServer ? server.quotaManager(origin) : nullptr;
>+ return [weakServer = makeWeakPtr(server)](PAL::SessionID, const auto& origin) {
>+ return weakServer ? weakServer->quotaManager(origin) : nullptr;
> };
> }
> 
>Index: Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.h
>===================================================================
>--- Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.h	(revision 245906)
>+++ Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.h	(working copy)
>@@ -53,6 +53,9 @@ class IDBServer;
> 
> class InProcessIDBServer final : public IDBClient::IDBConnectionToServerDelegate, public IDBServer::IDBConnectionToClientDelegate, public RefCounted<InProcessIDBServer>, public IDBServer::IDBBackingStoreTemporaryFileHandler {
> public:
>+ using IDBClient::IDBConnectionToServerDelegate::weakPtrFactory;
>+ typedef IDBClient::IDBConnectionToServerDelegate::WeakValueType WeakValueType;
>+
> WEBCORE_EXPORT static Ref<InProcessIDBServer> create(PAL::SessionID);
> WEBCORE_EXPORT static Ref<InProcessIDBServer> create(PAL::SessionID, const String& databaseDirectoryPath);
> 
>@@ -124,8 +127,6 @@ public:
> 
> StorageQuotaManager* quotaManager(const ClientOrigin&);
> 
>- const WeakPtrFactory<IDBClient::IDBConnectionToServerDelegate>& weakPtrFactory() const { return IDBClient::IDBConnectionToServerDelegate::weakPtrFactory(); }
>-
> private:
> explicit InProcessIDBServer(PAL::SessionID);
> InProcessIDBServer(PAL::SessionID, const String& databaseDirectoryPath);
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 245906)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,12 @@
>+2019-05-30 Geoffrey Garen <ggaren@apple.com>
>+
>+ Some WeakPtr cleanup
>+ https://bugs.webkit.org/show_bug.cgi?id=198390
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ * TestWebKitAPI/Tests/WTF/WeakPtr.cpp: Updated for rename.
>+
> 2019-05-30 Sihui Liu <sihui_liu@apple.com>
> 
> Stop StorageManager when network process is ready to suspend
>Index: Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp	(revision 245906)
>+++ Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp	(working copy)
>@@ -27,11 +27,11 @@
> 
> static unsigned s_baseWeakReferences = 0;
> 
>-#define DID_CREATE_WEAK_REFERENCE(p) do { \
>+#define DID_CREATE_WEAK_PTR_IMPL(p) do { \
> ++s_baseWeakReferences; \
> } while (0);
> 
>-#define WILL_DESTROY_WEAK_REFERENCE(p) do { \
>+#define WILL_DESTROY_WEAK_PTR_IMPL(p) do { \
> --s_baseWeakReferences; \
> } while (0);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 198390: 370988 | 371031