[Win] ASSERTION FAILED !m_ptr under AccessibilityController::winAddNotificationListener
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Jan 2015 06:24:09 +0000 (06:24 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Jan 2015 06:24:09 +0000 (06:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87426
<rdar://problem/11527899>

Reviewed by Darin Adler.

Source/WebCore:

Revise COMPtr to work better with our HashMap implementation:
(1) Add a specialization for IsSmartPtr.
(2) Remove PtrHash specialization.
(3) Refresh HashTrails specialization for COMPtr to match what we
do for RefPtr.

* platform/win/COMPtr.h:

Source/WebKit/win:

Revise COMPtr to work better with our HashMap implementation. Use
modern loop syntax.

* WebHistory.cpp:
(WebHistory::visitedURL): Adjust for new COMPtr changes.
* WebPreferences.cpp:
(WebPreferences::getInstanceForIdentifier): Ditto.
(WebPreferences::removeReferenceForIdentifier): Ditto.
* WebView.cpp:
(WebView::setEditable): Ditto.

Source/WTF:

Revise internal containers to use std::addressof in preference to
to using the '&' operator.

* wtf/Deque.h:
(WTF::inlineCapacity>::append):
(WTF::inlineCapacity>::prepend):
(WTF::inlineCapacity>::removeFirst):
(WTF::inlineCapacity>::removeLast):
(WTF::inlineCapacity>::remove):
(WTF::inlineCapacity>::after):
(WTF::inlineCapacity>::before):
* wtf/GetPtr.h:
* wtf/HashTable.h:
(WTF::HashTableBucketInitializer<false>::initialize):
* wtf/HashTraits.h:
(WTF::SimpleClassHashTraits::constructDeletedValue):
(WTF::CustomHashTraits::constructDeletedValue):
* wtf/ListHashSet.h:
(WTF::ListHashSetConstIterator::get):
* wtf/Vector.h:
(WTF::Vector::swap):
(WTF::OverflowHandler>::append):
(WTF::OverflowHandler>::tryAppend):
(WTF::OverflowHandler>::insert):

Tools:

Revise COMPtr to work better with our HashMap implementation. Use
modern loop syntax.

* DumpRenderTree/win/AccessibilityControllerWin.cpp:
(AccessibilityController::~AccessibilityController):
(AccessibilityController::winNotificationReceived):
* DumpRenderTree/win/DumpRenderTree.cpp:
(dumpBackForwardListForAllWindows):

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

16 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/Deque.h
Source/WTF/wtf/GetPtr.h
Source/WTF/wtf/HashTable.h
Source/WTF/wtf/HashTraits.h
Source/WTF/wtf/ListHashSet.h
Source/WTF/wtf/Vector.h
Source/WebCore/ChangeLog
Source/WebCore/platform/win/COMPtr.h
Source/WebKit/win/ChangeLog
Source/WebKit/win/WebHistory.cpp
Source/WebKit/win/WebPreferences.cpp
Source/WebKit/win/WebView.cpp
Tools/ChangeLog
Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp
Tools/DumpRenderTree/win/DumpRenderTree.cpp

index f8b9597298239b2cf862c5626a31f22ef8846373..9568cc82738deab71ce7f1292eb37f75c33a85bf 100644 (file)
@@ -1,3 +1,36 @@
+2015-01-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] ASSERTION FAILED !m_ptr under AccessibilityController::winAddNotificationListener
+        https://bugs.webkit.org/show_bug.cgi?id=87426
+        <rdar://problem/11527899>
+
+        Reviewed by Darin Adler.
+
+        Revise internal containers to use std::addressof in preference to
+        to using the '&' operator.
+
+        * wtf/Deque.h:
+        (WTF::inlineCapacity>::append):
+        (WTF::inlineCapacity>::prepend):
+        (WTF::inlineCapacity>::removeFirst):
+        (WTF::inlineCapacity>::removeLast):
+        (WTF::inlineCapacity>::remove):
+        (WTF::inlineCapacity>::after):
+        (WTF::inlineCapacity>::before):
+        * wtf/GetPtr.h:
+        * wtf/HashTable.h:
+        (WTF::HashTableBucketInitializer<false>::initialize):
+        * wtf/HashTraits.h:
+        (WTF::SimpleClassHashTraits::constructDeletedValue):
+        (WTF::CustomHashTraits::constructDeletedValue):
+        * wtf/ListHashSet.h:
+        (WTF::ListHashSetConstIterator::get):
+        * wtf/Vector.h:
+        (WTF::Vector::swap):
+        (WTF::OverflowHandler>::append):
+        (WTF::OverflowHandler>::tryAppend):
+        (WTF::OverflowHandler>::insert):
+
 2015-01-24  Chris Dumez  <cdumez@apple.com>
 
         Provide implementation for WTF::DefaultHash<bool>
index 1717007996ad6bffd2ced36fb1cc35bb696c1aba..b7855bb583a0fef4fc84c9411d66908ac5d28317 100644 (file)
@@ -411,7 +411,7 @@ namespace WTF {
     {
         checkValidity();
         expandCapacityIfNeeded();
-        new (NotNull, &m_buffer.buffer()[m_end]) T(std::forward<U>(value));
+        new (NotNull, std::addressof(m_buffer.buffer()[m_end])) T(std::forward<U>(value));
         if (m_end == m_buffer.capacity() - 1)
             m_end = 0;
         else
@@ -428,7 +428,7 @@ namespace WTF {
             m_start = m_buffer.capacity() - 1;
         else
             --m_start;
-        new (NotNull, &m_buffer.buffer()[m_start]) T(std::forward<U>(value));
+        new (NotNull, std::addressof(m_buffer.buffer()[m_start])) T(std::forward<U>(value));
         checkValidity();
     }
 
@@ -438,7 +438,7 @@ namespace WTF {
         checkValidity();
         invalidateIterators();
         ASSERT(!isEmpty());
-        TypeOperations::destruct(&m_buffer.buffer()[m_start], &m_buffer.buffer()[m_start + 1]);
+        TypeOperations::destruct(std::addressof(m_buffer.buffer()[m_start]), std::addressof(m_buffer.buffer()[m_start + 1]));
         if (m_start == m_buffer.capacity() - 1)
             m_start = 0;
         else
@@ -456,7 +456,7 @@ namespace WTF {
             m_end = m_buffer.capacity() - 1;
         else
             --m_end;
-        TypeOperations::destruct(&m_buffer.buffer()[m_end], &m_buffer.buffer()[m_end + 1]);
+        TypeOperations::destruct(std::addressof(m_buffer.buffer()[m_end]), std::addressof(m_buffer.buffer()[m_end + 1]));
         checkValidity();
     }
 
@@ -484,7 +484,7 @@ namespace WTF {
         invalidateIterators();
 
         T* buffer = m_buffer.buffer();
-        TypeOperations::destruct(&buffer[position], &buffer[position + 1]);
+        TypeOperations::destruct(std::addressof(buffer[position]), std::addressof(buffer[position + 1]));
 
         // Find which segment of the circular buffer contained the remove element, and only move elements in that part.
         if (position >= m_start) {
@@ -641,7 +641,7 @@ namespace WTF {
     {
         checkValidity();
         ASSERT(m_index != m_deque->m_end);
-        return &m_deque->m_buffer.buffer()[m_index];
+        return std::addressof(m_deque->m_buffer.buffer()[m_index]);
     }
 
     template<typename T, size_t inlineCapacity>
@@ -650,8 +650,8 @@ namespace WTF {
         checkValidity();
         ASSERT(m_index != m_deque->m_start);
         if (!m_index)
-            return &m_deque->m_buffer.buffer()[m_deque->m_buffer.capacity() - 1];
-        return &m_deque->m_buffer.buffer()[m_index - 1];
+            return std::addressof(m_deque->m_buffer.buffer()[m_deque->m_buffer.capacity() - 1]);
+        return std::addressof(m_deque->m_buffer.buffer()[m_index - 1]);
     }
 
 } // namespace WTF
index bd58df87fcf9e290e951d6cdba83cbe3986a3718..4fc5ff671973a92c113441ee8538175d0cd7f73e 100644 (file)
@@ -37,7 +37,7 @@ struct GetPtrHelperBase;
 template <typename T>
 struct GetPtrHelperBase<T, false /* isSmartPtr */> {
     typedef T* PtrType;
-    static T* getPtr(T& p) { return &p; }
+    static T* getPtr(T& p) { return std::addressof(p); }
 };
 
 template <typename T>
index 1621f73508932944ad32bd62df59919333626ca4..2c0811e54b6836b44957dca576e19073f38b2bca 100644 (file)
@@ -766,7 +766,7 @@ namespace WTF {
     template<> struct HashTableBucketInitializer<false> {
         template<typename Traits, typename Value> static void initialize(Value& bucket)
         {
-            new (NotNull, &bucket) Value(Traits::emptyValue());
+            new (NotNull, std::addressof(bucket)) Value(Traits::emptyValue());
         }
     };
 
index dd3078e3245b2e3a06b679a1ae47f4dd646765fc..e5feca95fdf3aff9a5039940fa324de70348dc1d 100644 (file)
@@ -105,7 +105,7 @@ template<typename P> struct HashTraits<P*> : GenericHashTraits<P*> {
 
 template<typename T> struct SimpleClassHashTraits : GenericHashTraits<T> {
     static const bool emptyValueIsZero = true;
-    static void constructDeletedValue(T& slot) { new (NotNull, &slot) T(HashTableDeletedValue); }
+    static void constructDeletedValue(T& slot) { new (NotNull, std::addressof(slot)) T(HashTableDeletedValue); }
     static bool isDeletedValue(const T& value) { return value.isHashTableDeletedValue(); }
 };
 
@@ -113,7 +113,7 @@ template<typename T, typename Deleter> struct HashTraits<std::unique_ptr<T, Dele
     typedef std::nullptr_t EmptyValueType;
     static EmptyValueType emptyValue() { return nullptr; }
 
-    static void constructDeletedValue(std::unique_ptr<T, Deleter>& slot) { new (NotNull, &slot) std::unique_ptr<T, Deleter> { reinterpret_cast<T*>(-1) }; }
+    static void constructDeletedValue(std::unique_ptr<T, Deleter>& slot) { new (NotNull, std::addressof(slot)) std::unique_ptr<T, Deleter> { reinterpret_cast<T*>(-1) }; }
     static bool isDeletedValue(const std::unique_ptr<T, Deleter>& value) { return value.get() == reinterpret_cast<T*>(-1); }
 
     typedef T* PeekType;
@@ -236,7 +236,7 @@ struct CustomHashTraits : public GenericHashTraits<T> {
     
     static void constructDeletedValue(T& slot)
     {
-        new (NotNull, &slot) T(T::DeletedValue);
+        new (NotNull, std::addressof(slot)) T(T::DeletedValue);
     }
     
     static bool isDeletedValue(const T& value)
index 72d3336170edfffa56344ed20d777f0b16b0127c..0a43210d87323deadf71ac686cd0eced9e986e57 100644 (file)
@@ -254,7 +254,7 @@ public:
 
     const ValueType* get() const
     {
-        return &m_position->m_value;
+        return std::addressof(m_position->m_value);
     }
 
     const ValueType& operator*() const { return *get(); }
index a1ee013b3b1bb162c2a1e8c96a30478bdf1a4b34..84a154a27edbf1c0e83ab2536ac0584485a104e7 100644 (file)
@@ -753,7 +753,7 @@ public:
     void swap(Vector<T, inlineCapacity, OverflowHandler>& other)
     {
 #if ASAN_ENABLED
-        if (this == &other) // ASan will crash if we try to restrict access to the same buffer twice.
+        if (this == std::addressof(other)) // ASan will crash if we try to restrict access to the same buffer twice.
             return;
 #endif
 
@@ -1170,7 +1170,7 @@ void Vector<T, inlineCapacity, OverflowHandler>::append(const U* data, size_t da
         CRASH();
     asanBufferSizeWillChangeTo(newSize);
     T* dest = end();
-    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, &data[dataSize], dest);
+    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, std::addressof(data[dataSize]), dest);
     m_size = newSize;
 }
 
@@ -1188,7 +1188,7 @@ bool Vector<T, inlineCapacity, OverflowHandler>::tryAppend(const U* data, size_t
         return false;
     asanBufferSizeWillChangeTo(newSize);
     T* dest = end();
-    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, &data[dataSize], dest);
+    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, std::addressof(data[dataSize]), dest);
     m_size = newSize;
     return true;
 }
@@ -1255,7 +1255,7 @@ void Vector<T, inlineCapacity, OverflowHandler>::insert(size_t position, const U
     asanBufferSizeWillChangeTo(newSize);
     T* spot = begin() + position;
     TypeOperations::moveOverlapping(spot, end(), spot + dataSize);
-    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, &data[dataSize], spot);
+    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, std::addressof(data[dataSize]), spot);
     m_size = newSize;
 }
  
index f4d5f880c296d4a2f36c7df74b8bf90f8f93c49f..a43bb4a3b2251f7ab213dfc603e191711e3b2718 100644 (file)
@@ -1,3 +1,19 @@
+2015-01-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] ASSERTION FAILED !m_ptr under AccessibilityController::winAddNotificationListener
+        https://bugs.webkit.org/show_bug.cgi?id=87426
+        <rdar://problem/11527899>
+
+        Reviewed by Darin Adler.
+
+        Revise COMPtr to work better with our HashMap implementation:
+        (1) Add a specialization for IsSmartPtr.
+        (2) Remove PtrHash specialization.
+        (3) Refresh HashTrails specialization for COMPtr to match what we
+        do for RefPtr.
+
+        * platform/win/COMPtr.h:
+
 2015-01-26  Sylvain Galineau  <galineau@adobe.com>
 
         The computed value of line-height:normal is incorrect
index f264b0872c17dc5adb77a1131ce98475fc3daa90..e595edf9fd56a0b2490b234429f33af152722239 100644 (file)
@@ -49,6 +49,7 @@ enum CreateTag { Create };
 
 template<typename T> class COMPtr {
 public:
+    typedef T* PtrType;
     COMPtr() : m_ptr(0) { }
     COMPtr(T* ptr) : m_ptr(ptr) { if (m_ptr) m_ptr->AddRef(); }
     COMPtr(AdoptCOMTag, T* ptr) : m_ptr(ptr) { }
@@ -223,22 +224,22 @@ template<typename T, typename U> inline bool operator!=(T* a, const COMPtr<U>& b
 
 namespace WTF {
 
-    template<typename P> struct HashTraits<COMPtr<P> > : GenericHashTraits<COMPtr<P> > {
-        static const bool emptyValueIsZero = true;
-        static void constructDeletedValue(COMPtr<P>& slot) { new (&slot) COMPtr<P>(HashTableDeletedValue); }
-        static bool isDeletedValue(const COMPtr<P>& value) { return value.isHashTableDeletedValue(); }
-    };
-
-    template<typename P> struct PtrHash<COMPtr<P> > : PtrHash<P*> {
-        using PtrHash<P*>::hash;
-        static unsigned hash(const COMPtr<P>& key) { return hash(key.get()); }
-        using PtrHash<P*>::equal;
-        static bool equal(const COMPtr<P>& a, const COMPtr<P>& b) { return a == b; }
-        static bool equal(P* a, const COMPtr<P>& b) { return a == b; }
-        static bool equal(const COMPtr<P>& a, P* b) { return a == b; }
-    };
-
-    template<typename P> struct DefaultHash<COMPtr<P> > { typedef PtrHash<COMPtr<P> > Hash; };
+template<typename P> struct IsSmartPtr<COMPtr<P>> {
+    static const bool value = true;
+};
+
+template<typename P> struct HashTraits<COMPtr<P> > : SimpleClassHashTraits<COMPtr<P>> {
+    static P* emptyValue() { return nullptr; }
+
+    typedef P* PeekType;
+    static PeekType peek(const COMPtr<P>& value) { return value.get(); }
+    static PeekType peek(P* value) { return value; }
+};
+
+template<typename P> struct DefaultHash<COMPtr<P>> {
+    typedef PtrHash<COMPtr<P>> Hash;
+};
+
 }
 
 #endif
index f60653b1aad07e1445578528b2bc75d2cf42e931..411c6012d33358ee2d8703b08b3908e082847b64 100644 (file)
@@ -1,3 +1,22 @@
+2015-01-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] ASSERTION FAILED !m_ptr under AccessibilityController::winAddNotificationListener
+        https://bugs.webkit.org/show_bug.cgi?id=87426
+        <rdar://problem/11527899>
+
+        Reviewed by Darin Adler.
+
+        Revise COMPtr to work better with our HashMap implementation. Use
+        modern loop syntax.
+
+        * WebHistory.cpp:
+        (WebHistory::visitedURL): Adjust for new COMPtr changes.
+        * WebPreferences.cpp:
+        (WebPreferences::getInstanceForIdentifier): Ditto.
+        (WebPreferences::removeReferenceForIdentifier): Ditto.
+        * WebView.cpp:
+        (WebView::setEditable): Ditto.
+
 2015-01-26  Chris Dumez  <cdumez@apple.com>
 
         Rename Document::body() to Document::bodyOrFrameset() for clarity
index 21444856a43d43b9c8b0afa2b231d1fa2d1ab0f1..a5b42d9ca0b3a8b875d15dd47e08db2c1e6eda95 100644 (file)
@@ -504,7 +504,7 @@ void WebHistory::visitedURL(const URL& url, const String& title, const String& h
     if (urlString.isEmpty())
         return;
 
-    IWebHistoryItem* entry = m_entriesByURL.get(urlString).get();
+    IWebHistoryItem* entry = m_entriesByURL.get(urlString);
     if (!entry) {
         COMPtr<WebHistoryItem> item(AdoptCOM, WebHistoryItem::createInstance());
         if (!item)
index 479722f096ac1e0364089996a391922b7d47758e..10dddb5ab5160d882a06f8ffb8b762a3f4e19721 100644 (file)
@@ -161,7 +161,7 @@ WebPreferences* WebPreferences::getInstanceForIdentifier(BSTR identifier)
     if (identifierString.isEmpty())
         return sharedStandardPreferences();
 
-    return webPreferencesInstances().get(identifierString).get();
+    return webPreferencesInstances().get(identifierString);
 }
 
 void WebPreferences::setInstance(WebPreferences* instance, BSTR identifier)
@@ -182,7 +182,7 @@ void WebPreferences::removeReferenceForIdentifier(BSTR identifier)
     WTF::String identifierString(identifier, SysStringLen(identifier));
     if (identifierString.isEmpty())
         return;
-    WebPreferences* webPreference = webPreferencesInstances().get(identifierString).get();
+    WebPreferences* webPreference = webPreferencesInstances().get(identifierString);
     if (webPreference && webPreference->m_refCount == 1)
         webPreferencesInstances().remove(identifierString);
 }
index a2a38e320ce3d69fca737253fe53c1034449e6c5..5764724e6a5354585e68bab066cb2993865751c3 100644 (file)
@@ -4195,7 +4195,7 @@ HRESULT WebView::setEditable(BOOL flag)
     if (!m_page)
         return S_OK;
 
-    if (m_page->isEditable() == flag)
+    if (m_page->isEditable() == static_cast<bool>(flag))
         return S_OK;
 
     m_page->setEditable(flag);
index 2503a4108c8223f169fa8bbb06fbe240367b733b..38c701267ca5bd4e3a6103851f61b1258c07adf0 100644 (file)
@@ -1,3 +1,20 @@
+2015-01-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] ASSERTION FAILED !m_ptr under AccessibilityController::winAddNotificationListener
+        https://bugs.webkit.org/show_bug.cgi?id=87426
+        <rdar://problem/11527899>
+
+        Reviewed by Darin Adler.
+
+        Revise COMPtr to work better with our HashMap implementation. Use
+        modern loop syntax.
+
+        * DumpRenderTree/win/AccessibilityControllerWin.cpp:
+        (AccessibilityController::~AccessibilityController):
+        (AccessibilityController::winNotificationReceived):
+        * DumpRenderTree/win/DumpRenderTree.cpp:
+        (dumpBackForwardListForAllWindows):
+
 2015-01-26  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         [Win] Enable JSC stress tests by default
index 1c9c87c7d1f3b85b7195227768af31a8ef47ddfe..fba90ac53065fb3b83c94608dd7591b541bc262e 100644 (file)
@@ -61,8 +61,8 @@ AccessibilityController::~AccessibilityController()
     if (m_notificationsEventHook)
         UnhookWinEvent(m_notificationsEventHook);
 
-    for (HashMap<PlatformUIElement, JSObjectRef>::iterator it = m_notificationListeners.begin(); it != m_notificationListeners.end(); ++it)
-        JSValueUnprotect(frame->globalContext(), it->value);
+    for (auto& listener : m_notificationListeners.values())
+        JSValueUnprotect(frame->globalContext(), listener);
 }
 
 AccessibilityUIElement AccessibilityController::elementAtPoint(int x, int y)
@@ -337,8 +337,8 @@ void AccessibilityController::removeNotificationListener()
 
 void AccessibilityController::winNotificationReceived(PlatformUIElement element, const string& eventName)
 {
-    for (HashMap<PlatformUIElement, JSObjectRef>::iterator it = m_notificationListeners.begin(); it != m_notificationListeners.end(); ++it) {
-        COMPtr<IServiceProvider> thisServiceProvider(Query, it->key);
+    for (auto& slot : m_notificationListeners) {
+        COMPtr<IServiceProvider> thisServiceProvider(Query, slot.key);
         if (!thisServiceProvider)
             continue;
 
@@ -361,7 +361,7 @@ void AccessibilityController::winNotificationReceived(PlatformUIElement element,
 
         JSRetainPtr<JSStringRef> jsNotification(Adopt, JSStringCreateWithUTF8CString(eventName.c_str()));
         JSValueRef argument = JSValueMakeString(frame->globalContext(), jsNotification.get());
-        JSObjectCallAsFunction(frame->globalContext(), it->value, 0, 1, &argument, 0);
+        JSObjectCallAsFunction(frame->globalContext(), slot.value, 0, 1, &argument, 0);
     }
 }
 
index c327f4b099cecdc620f3e593a09dc59720fe1655..546f467b65e8a16b193e19176cf2c5a5c8180c65 100644 (file)
@@ -658,7 +658,7 @@ static void dumpBackForwardListForAllWindows()
     unsigned count = openWindows().size();
     for (unsigned i = 0; i < count; i++) {
         HWND window = openWindows()[i];
-        IWebView* webView = windowToWebViewMap().get(window).get();
+        IWebView* webView = windowToWebViewMap().get(window);
         dumpBackForwardList(webView);
     }
 }