Merge r173765 - Allow DOM methods to return references instead of poi…

…nters https://bugs.webkit.org/show_bug.cgi?id=136931 Source/WebCore: Reviewed by Sam Weinig. It is common practice in WebKit to have methods return a reference instead of a pointer if the pointer can never be null. However, this unfortunately did not work for DOM methods (functions called by JS bindings). This prevented further refactoring. This patch brings support for having DOM methods to return references instead of pointers when the pointer cannot be null. The generated bindings were calling WTF::getPtr() on the pointer type returned by the implementation already (in case it was a smart pointer type). This patch leverages this by having WTF::getPtr() convert reference arguments into raw pointers. This patch also updates a few DOM methods on Document and Element classes to return a reference instead of a pointer, to test the change. There are likely more DOM methods that can be updated though. No new tests, no behavior change. * accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::classList): * bindings/js/JSDOMBinding.h: (WTF::getPtr): Deleted. * dom/Document.cpp: (WebCore::Document::implementation): (WebCore::Document::webkitGetNamedFlows): (WebCore::Document::namedFlows): (WebCore::Document::setXMLVersion): (WebCore::Document::setXMLStandalone): (WebCore::Document::securityPolicy): (WebCore::Document::styleSheets): * dom/Document.h: (WebCore::Document::timing): * dom/Element.cpp: (WebCore::Element::classList): (WebCore::Element::dataset): * dom/Element.h: * html/shadow/MediaControlElements.cpp: (WebCore::MediaControlPanelElement::setPosition): (WebCore::MediaControlPanelElement::resetPosition): (WebCore::MediaControlClosedCaptionsTrackListElement::updateDisplay): * html/track/VTTRegion.cpp: (WebCore::VTTRegion::displayLastTextTrackCueBox): (WebCore::VTTRegion::willRemoveTextTrackCueBox): * inspector/InspectorCSSAgent.cpp: (WebCore::InspectorCSSAgent::getAllStyleSheets): (WebCore::InspectorCSSAgent::getNamedFlowCollection): * page/PerformanceTiming.cpp: (WebCore::PerformanceTiming::documentTiming): * rendering/FlowThreadController.cpp: (WebCore::FlowThreadController::ensureRenderFlowThreadWithName): Source/WTF: Add support for having WTF::getPtr() transform reference arguments into raw pointers so that DOM methods can now return references when appropriate and so that the generated bindings code can handle this via WTF::getPtr(). This patch had to alter the way getPtr() was overloaded for smart pointer types so that we don't call &p on smart pointers but p.get(). This was needed because the new WTF::getPtr(T&) was being called for RefPtr<T> arguments instead of the getPtr(const RefPtr<T>&) overload. This was addressed using traits and template specialization to distinguish WTF smart pointers from other types. Reviewed by Sam Weinig. * wtf/GetPtr.h: (WTF::getPtr): * wtf/OwnPtr.h: (WTF::getPtr): Deleted. * wtf/PassOwnPtr.h: (WTF::getPtr): Deleted. * wtf/PassRefPtr.h: (WTF::getPtr): Deleted. * wtf/Ref.h: * wtf/RefPtr.h: (WTF::getPtr): Deleted. * wtf/gobject/GRefPtr.h: (WTF::getPtr): Deleted. Canonical link: https://commits.webkit.org/154760.131@webkitgtk/2.6 git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@175747 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Nov 7, 2014 · 76f8476 · 76f8476
1 parent 95ec178
commit 76f8476
Show file tree

Hide file tree

Showing 20 changed files with 207 additions and 84 deletions.
diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
@@ -1,3 +1,36 @@
+2014-09-19  Chris Dumez  <cdumez@apple.com>
+
+        Allow DOM methods to return references instead of pointers
+        https://bugs.webkit.org/show_bug.cgi?id=136931
+
+        Add support for having WTF::getPtr() transform reference arguments
+        into raw pointers so that DOM methods can now return references when
+        appropriate and so that the generated bindings code can handle this
+        via WTF::getPtr().
+
+        This patch had to alter the way getPtr() was overloaded for smart
+        pointer types so that we don't call &p on smart pointers but p.get().
+        This was needed because the new WTF::getPtr(T&) was being called for
+        RefPtr<T> arguments instead of the getPtr(const RefPtr<T>&) overload.
+        This was addressed using traits and template specialization to
+        distinguish WTF smart pointers from other types.
+
+        Reviewed by Sam Weinig.
+
+        * wtf/GetPtr.h:
+        (WTF::getPtr):
+        * wtf/OwnPtr.h:
+        (WTF::getPtr): Deleted.
+        * wtf/PassOwnPtr.h:
+        (WTF::getPtr): Deleted.
+        * wtf/PassRefPtr.h:
+        (WTF::getPtr): Deleted.
+        * wtf/Ref.h:
+        * wtf/RefPtr.h:
+        (WTF::getPtr): Deleted.
+        * wtf/gobject/GRefPtr.h:
+        (WTF::getPtr): Deleted.
+
 2014-10-17  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GLIB] Add API to GMainLoopSource to schedule sources after a delay in microseconds

diff --git a/Source/WTF/wtf/GetPtr.h b/Source/WTF/wtf/GetPtr.h
@@ -23,10 +23,38 @@
 
 namespace WTF {
 
-    template <typename T> inline T* getPtr(T* p)
-    {
-        return p;
-    }
+template <typename T> inline T* getPtr(T* p) { return p; }
+
+template <typename T> struct IsSmartPtr {
+    static const bool value = false;
+};
+
+template <typename T, bool isSmartPtr>
+struct GetPtrHelper;
+
+template <typename T>
+struct GetPtrHelper<T, false /* isSmartPtr */> {
+    typedef T* PtrType;
+    static T* getPtr(T& p) { return &p; }
+};
+
+template <typename T>
+struct GetPtrHelper<T, true /* isSmartPtr */> {
+    typedef typename T::PtrType PtrType;
+    static PtrType getPtr(const T& p) { return p.get(); }
+};
+
+template <typename T>
+inline typename GetPtrHelper<T, IsSmartPtr<T>::value>::PtrType getPtr(T& p)
+{
+    return GetPtrHelper<T, IsSmartPtr<T>::value>::getPtr(p);
+}
+
+template <typename T>
+inline typename GetPtrHelper<T, IsSmartPtr<T>::value>::PtrType getPtr(const T& p)
+{
+    return GetPtrHelper<T, IsSmartPtr<T>::value>::getPtr(p);
+}
 
 } // namespace WTF
 

diff --git a/Source/WTF/wtf/OwnPtr.h b/Source/WTF/wtf/OwnPtr.h
@@ -23,6 +23,7 @@
 
 #include <wtf/Assertions.h>
 #include <wtf/Atomics.h>
+#include <wtf/GetPtr.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/OwnPtrCommon.h>
 #include <algorithm>
@@ -193,10 +194,9 @@ namespace WTF {
         return a != b.get(); 
     }
 
-    template<typename T> inline typename OwnPtr<T>::PtrType getPtr(const OwnPtr<T>& p)
-    {
-        return p.get();
-    }
+    template <typename T> struct IsSmartPtr<OwnPtr<T>> {
+        static const bool value = true;
+    };
 
     template<typename T> template<typename... Args> inline void OwnPtr<T>::createTransactionally(Args... args)
     {

diff --git a/Source/WTF/wtf/PassOwnPtr.h b/Source/WTF/wtf/PassOwnPtr.h
@@ -28,6 +28,7 @@
 
 #include <cstddef>
 #include <wtf/Assertions.h>
+#include <wtf/GetPtr.h>
 #include <wtf/OwnPtrCommon.h>
 #include <type_traits>
 
@@ -156,10 +157,9 @@ namespace WTF {
         return adoptPtr(static_cast<T*>(p.leakPtr()));
     }
 
-    template<typename T> inline T* getPtr(const PassOwnPtr<T>& p)
-    {
-        return p.get();
-    }
+    template <typename T> struct IsSmartPtr<PassOwnPtr<T>> {
+        static const bool value = true;
+    };
 
 } // namespace WTF
 

diff --git a/Source/WTF/wtf/PassRefPtr.h b/Source/WTF/wtf/PassRefPtr.h
@@ -21,7 +21,8 @@
 #ifndef WTF_PassRefPtr_h
 #define WTF_PassRefPtr_h
 
-#include "PassRef.h"
+#include <wtf/GetPtr.h>
+#include <wtf/PassRef.h>
 
 namespace WTF {
 
@@ -41,6 +42,9 @@ namespace WTF {
 
     template<typename T> class PassRefPtr {
     public:
+        typedef T ValueType;
+        typedef ValueType* PtrType;
+
         PassRefPtr() : m_ptr(nullptr) { }
         PassRefPtr(T* ptr) : m_ptr(ptr) { refIfNotNull(ptr); }
         // It somewhat breaks the type system to allow transfer of ownership out of
@@ -153,10 +157,9 @@ namespace WTF {
         return adoptRef(static_cast<T*>(p.leakRef())); 
     }
 
-    template<typename T> inline T* getPtr(const PassRefPtr<T>& p)
-    {
-        return p.get();
-    }
+    template <typename T> struct IsSmartPtr<PassRefPtr<T>> {
+        static const bool value = true;
+    };
 
 } // namespace WTF
 

diff --git a/Source/WTF/wtf/Ref.h b/Source/WTF/wtf/Ref.h
@@ -26,7 +26,8 @@
 #ifndef WTF_Ref_h
 #define WTF_Ref_h
 
-#include "Noncopyable.h"
+#include <wtf/GetPtr.h>
+#include <wtf/Noncopyable.h>
 
 namespace WTF {
 
@@ -73,6 +74,12 @@ template<typename T> template<typename U> inline PassRef<T> Ref<T>::replace(Pass
     return oldReference;
 }
 
+template <typename T>
+struct GetPtrHelper<Ref<T>, false /* isSmartPtr */> {
+    typedef T* PtrType;
+    static T* getPtr(const Ref<T>& p) { return const_cast<T*>(&p.get()); }
+};
+
 } // namespace WTF
 
 using WTF::Ref;

diff --git a/Source/WTF/wtf/RefPtr.h b/Source/WTF/wtf/RefPtr.h
@@ -23,10 +23,11 @@
 #ifndef WTF_RefPtr_h
 #define WTF_RefPtr_h
 
-#include "FastMalloc.h"
-#include "PassRefPtr.h"
 #include <algorithm>
 #include <utility>
+#include <wtf/FastMalloc.h>
+#include <wtf/GetPtr.h>
+#include <wtf/PassRefPtr.h>
 
 namespace WTF {
 
@@ -35,6 +36,9 @@ namespace WTF {
     template<typename T> class RefPtr {
         WTF_MAKE_FAST_ALLOCATED;
     public:
+        typedef T ValueType;
+        typedef ValueType* PtrType;
+
         ALWAYS_INLINE RefPtr() : m_ptr(nullptr) { }
         ALWAYS_INLINE RefPtr(T* ptr) : m_ptr(ptr) { refIfNotNull(ptr); }
         ALWAYS_INLINE RefPtr(const RefPtr& o) : m_ptr(o.m_ptr) { refIfNotNull(m_ptr); }
@@ -204,10 +208,9 @@ namespace WTF {
         return RefPtr<T>(static_cast<T*>(p.get())); 
     }
 
-    template<typename T> inline T* getPtr(const RefPtr<T>& p)
-    {
-        return p.get();
-    }
+    template <typename T> struct IsSmartPtr<RefPtr<T>> {
+        static const bool value = true;
+    };
 
 } // namespace WTF
 

diff --git a/Source/WTF/wtf/gobject/GRefPtr.h b/Source/WTF/wtf/gobject/GRefPtr.h
@@ -25,6 +25,7 @@
 
 #if USE(GLIB)
 
+#include <wtf/GetPtr.h>
 #include <wtf/RefPtr.h>
 #include <algorithm>
 
@@ -41,6 +42,9 @@ template <typename T> GRefPtr<T> adoptGRef(T*);
 
 template <typename T> class GRefPtr {
 public:
+    typedef T ValueType;
+    typedef ValueType* PtrType;
+
     GRefPtr() : m_ptr(0) { }
 
     GRefPtr(T* ptr)
@@ -204,10 +208,9 @@ template <typename T, typename U> inline GRefPtr<T> const_pointer_cast(const GRe
     return GRefPtr<T>(const_cast<T*>(p.get()));
 }
 
-template <typename T> inline T* getPtr(const GRefPtr<T>& p)
-{
-    return p.get();
-}
+template <typename T> struct IsSmartPtr<GRefPtr<T>> {
+    static const bool value = true;
+};
 
 template <typename T> GRefPtr<T> adoptGRef(T* p)
 {

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,61 @@
+2014-09-19  Chris Dumez  <cdumez@apple.com>
+
+        Allow DOM methods to return references instead of pointers
+        https://bugs.webkit.org/show_bug.cgi?id=136931
+
+        Reviewed by Sam Weinig.
+
+        It is common practice in WebKit to have methods return a reference
+        instead of a pointer if the pointer can never be null. However, this
+        unfortunately did not work for DOM methods (functions called by JS
+        bindings). This prevented further refactoring.
+
+        This patch brings support for having DOM methods to return references
+        instead of pointers when the pointer cannot be null. The generated
+        bindings were calling WTF::getPtr() on the pointer type returned by
+        the implementation already (in case it was a smart pointer type).
+        This patch leverages this by having WTF::getPtr() convert reference
+        arguments into raw pointers.
+
+        This patch also updates a few DOM methods on Document and Element
+        classes to return a reference instead of a pointer, to test the change.
+        There are likely more DOM methods that can be updated though.
+
+        No new tests, no behavior change.
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::classList):
+        * bindings/js/JSDOMBinding.h:
+        (WTF::getPtr): Deleted.
+        * dom/Document.cpp:
+        (WebCore::Document::implementation):
+        (WebCore::Document::webkitGetNamedFlows):
+        (WebCore::Document::namedFlows):
+        (WebCore::Document::setXMLVersion):
+        (WebCore::Document::setXMLStandalone):
+        (WebCore::Document::securityPolicy):
+        (WebCore::Document::styleSheets):
+        * dom/Document.h:
+        (WebCore::Document::timing):
+        * dom/Element.cpp:
+        (WebCore::Element::classList):
+        (WebCore::Element::dataset):
+        * dom/Element.h:
+        * html/shadow/MediaControlElements.cpp:
+        (WebCore::MediaControlPanelElement::setPosition):
+        (WebCore::MediaControlPanelElement::resetPosition):
+        (WebCore::MediaControlClosedCaptionsTrackListElement::updateDisplay):
+        * html/track/VTTRegion.cpp:
+        (WebCore::VTTRegion::displayLastTextTrackCueBox):
+        (WebCore::VTTRegion::willRemoveTextTrackCueBox):
+        * inspector/InspectorCSSAgent.cpp:
+        (WebCore::InspectorCSSAgent::getAllStyleSheets):
+        (WebCore::InspectorCSSAgent::getNamedFlowCollection):
+        * page/PerformanceTiming.cpp:
+        (WebCore::PerformanceTiming::documentTiming):
+        * rendering/FlowThreadController.cpp:
+        (WebCore::FlowThreadController::ensureRenderFlowThreadWithName):
+
 2014-10-17  Chris Dumez  <cdumez@apple.com>
 
         Avoid unnecessary isSVGFont() check in SimpleFontData::applyTransforms()

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -2122,12 +2122,10 @@ void AccessibilityObject::classList(Vector<String>& classList) const
         return;
 
     Element* element = toElement(node);
-    DOMTokenList* list = element->classList();
-    if (!list)
-        return;
-    unsigned length = list->length();
+    DOMTokenList& list = element->classList();
+    unsigned length = list.length();
     for (unsigned k = 0; k < length; k++)
-        classList.append(list->item(k).string());
+        classList.append(list.item(k).string());
 }
 
 

diff --git a/Source/WebCore/bindings/js/JSDOMBinding.h b/Source/WebCore/bindings/js/JSDOMBinding.h
@@ -45,24 +45,14 @@
 #include <runtime/TypedArrayInlines.h>
 #include <runtime/TypedArrays.h>
 #include <wtf/Forward.h>
+#include <wtf/GetPtr.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
 
 namespace JSC {
 class HashEntry;
 }
 
-#if ENABLE(GAMEPAD)
-namespace WTF {
-
-template<typename T> inline T* getPtr(const Ref<T>& p)
-{
-    return const_cast<T*>(&p.get());
-}
-
-}
-#endif
-
 namespace WebCore {
 
 class CachedScript;