Add specialized template declarations of HashTraits and DefaultHash to detect misuse
authorHironori.Fujii@sony.com <Hironori.Fujii@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2018 05:20:52 +0000 (05:20 +0000)
committerHironori.Fujii@sony.com <Hironori.Fujii@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2018 05:20:52 +0000 (05:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189044

Reviewed by Yusuke Suzuki.

Source/WebCore:

Some classes have a separate header for specializations of
WTF::HashTraits and WTF::DefaultHash to reduce the number of
header files included. For example, ColorHash.h and Color.h.

If someone is mistakenly using HashSet or HashMap without
including the specialization header, unexpected template
instantiation can cause subtle bugs. For example, MSVC linker
would silently produce an broken executable (Bug 188893).

By applying this change, I found three misuse cases in
DebugPageOverlays.cpp, AVVideoCaptureSource.h and WebPage.h. As
far as I analyzed, I concluded that these misuses don't introduce
any bugs at the moment, and they are not testable (Bug 189044 Comment 9),
except the MSVC issue (Bug 188893).

No new tests (Covered by existing tests).

* Modules/webdatabase/SQLResultSetRowList.h: Removed unused #include <wtf/HashTraits.h>.
* bindings/js/SerializedScriptValue.cpp: Ditto.
* page/DebugPageOverlays.cpp: Added #include "ColorHash.h" to instantiate HashMap<String, Color>.
* platform/URLHash.h: Added DefaultHash<URL> specialization definition.
* platform/URL.h: Added specialized template declarations.
* platform/graphics/Color.h: Ditto.
* platform/graphics/FloatSize.h: Ditto.
* platform/graphics/IntPoint.h: Ditto.
* platform/graphics/IntSize.h: Ditto.
* platform/network/ProtectionSpace.h: Ditto.
* platform/network/ProtectionSpaceHash.h: Removed unnecessary DefaultHash declaration.
* platform/mediastream/mac/AVVideoCaptureSource.h:
Added #include "IntSizeHash.h" to instantiate HashMap<String, IntSize>.

Source/WebKit:

* WebProcess/WebPage/WebPage.h: Added #include <WebCore/IntPointHash.h> to instantiate HashMap<std::pair<IntSize, double>, IntPoint>.

Source/WTF:

* wtf/BitVector.h: Removed unnecessary HashTraits declaration.
* wtf/MetaAllocatorPtr.h: Ditto.
* wtf/IndexSparseSet.h: Removed unused #include <wtf/HashTraits.h>.
* wtf/LoggingHashMap.h: Removed unused #include <wtf/LoggingHashTraits.h>.
* wtf/StackShot.h: Added #include <wtf/HashTraits.h> because this header uses SimpleClassHashTraits.
Removed unnecessary HashTraits declaration.

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

21 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/BitVector.h
Source/WTF/wtf/IndexSparseSet.h
Source/WTF/wtf/LoggingHashMap.h
Source/WTF/wtf/MetaAllocatorPtr.h
Source/WTF/wtf/StackShot.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/webdatabase/SQLResultSetRowList.h
Source/WebCore/bindings/js/SerializedScriptValue.cpp
Source/WebCore/page/DebugPageOverlays.cpp
Source/WebCore/platform/URL.h
Source/WebCore/platform/URLHash.h
Source/WebCore/platform/graphics/Color.h
Source/WebCore/platform/graphics/FloatSize.h
Source/WebCore/platform/graphics/IntPoint.h
Source/WebCore/platform/graphics/IntSize.h
Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h
Source/WebCore/platform/network/ProtectionSpace.h
Source/WebCore/platform/network/ProtectionSpaceHash.h
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/WebPage.h

index 1b80c24..fb553bd 100644 (file)
@@ -1,3 +1,17 @@
+2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        Add specialized template declarations of HashTraits and DefaultHash to detect misuse
+        https://bugs.webkit.org/show_bug.cgi?id=189044
+
+        Reviewed by Yusuke Suzuki.
+
+        * wtf/BitVector.h: Removed unnecessary HashTraits declaration.
+        * wtf/MetaAllocatorPtr.h: Ditto.
+        * wtf/IndexSparseSet.h: Removed unused #include <wtf/HashTraits.h>.
+        * wtf/LoggingHashMap.h: Removed unused #include <wtf/LoggingHashTraits.h>.
+        * wtf/StackShot.h: Added #include <wtf/HashTraits.h> because this header uses SimpleClassHashTraits.
+        Removed unnecessary HashTraits declaration.
+
 2018-09-07  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r235784.
index 8c8bdd7..c389e04 100644 (file)
@@ -482,7 +482,6 @@ template<> struct DefaultHash<BitVector> {
     typedef BitVectorHash Hash;
 };
 
-template<typename T> struct HashTraits;
 template<> struct HashTraits<BitVector> : public CustomHashTraits<BitVector> { };
 
 } // namespace WTF
index bce4588..f0e9394 100644 (file)
@@ -26,7 +26,6 @@
 #ifndef IndexSparseSet_h
 #define IndexSparseSet_h
 
-#include <wtf/HashTraits.h>
 #include <wtf/Vector.h>
 
 namespace WTF {
index ff8e790..5b6e621 100644 (file)
@@ -28,7 +28,6 @@
 #include <wtf/DataLog.h>
 #include <wtf/HashMap.h>
 #include <wtf/LoggingHashID.h>
-#include <wtf/LoggingHashTraits.h>
 
 namespace WTF {
 
index 5be74a0..d22e611 100644 (file)
@@ -114,7 +114,6 @@ template<PtrTag tag> struct DefaultHash<MetaAllocatorPtr<tag>> {
     typedef MetaAllocatorPtrHash<tag> Hash;
 };
 
-template<typename T> struct HashTraits;
 template<PtrTag tag> struct HashTraits<MetaAllocatorPtr<tag>> : public CustomHashTraits<MetaAllocatorPtr<tag>> { };
 
 } // namespace WTF
index 7e32a35..7b3ad92 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include <wtf/Assertions.h>
+#include <wtf/HashTraits.h>
 #include <wtf/UniqueArray.h>
 
 namespace WTF {
@@ -123,7 +124,6 @@ template<> struct DefaultHash<StackShot> {
     typedef StackShotHash Hash;
 };
 
-template<typename T> struct HashTraits;
 template<> struct HashTraits<StackShot> : SimpleClassHashTraits<StackShot> { };
 
 } // namespace WTF
index 36b7635..e46dbe3 100644 (file)
@@ -1,5 +1,43 @@
 2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
 
+        Add specialized template declarations of HashTraits and DefaultHash to detect misuse
+        https://bugs.webkit.org/show_bug.cgi?id=189044
+
+        Reviewed by Yusuke Suzuki.
+
+        Some classes have a separate header for specializations of
+        WTF::HashTraits and WTF::DefaultHash to reduce the number of
+        header files included. For example, ColorHash.h and Color.h.
+
+        If someone is mistakenly using HashSet or HashMap without
+        including the specialization header, unexpected template
+        instantiation can cause subtle bugs. For example, MSVC linker
+        would silently produce an broken executable (Bug 188893).
+
+        By applying this change, I found three misuse cases in
+        DebugPageOverlays.cpp, AVVideoCaptureSource.h and WebPage.h. As
+        far as I analyzed, I concluded that these misuses don't introduce
+        any bugs at the moment, and they are not testable (Bug 189044 Comment 9),
+        except the MSVC issue (Bug 188893).
+
+        No new tests (Covered by existing tests).
+
+        * Modules/webdatabase/SQLResultSetRowList.h: Removed unused #include <wtf/HashTraits.h>.
+        * bindings/js/SerializedScriptValue.cpp: Ditto.
+        * page/DebugPageOverlays.cpp: Added #include "ColorHash.h" to instantiate HashMap<String, Color>.
+        * platform/URLHash.h: Added DefaultHash<URL> specialization definition.
+        * platform/URL.h: Added specialized template declarations.
+        * platform/graphics/Color.h: Ditto.
+        * platform/graphics/FloatSize.h: Ditto.
+        * platform/graphics/IntPoint.h: Ditto.
+        * platform/graphics/IntSize.h: Ditto.
+        * platform/network/ProtectionSpace.h: Ditto.
+        * platform/network/ProtectionSpaceHash.h: Removed unnecessary DefaultHash declaration.
+        * platform/mediastream/mac/AVVideoCaptureSource.h:
+        Added #include "IntSizeHash.h" to instantiate HashMap<String, IntSize>.
+
+2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
+
         [Win][Clang] Add FloatRect(const RECT&) constructor
         https://bugs.webkit.org/show_bug.cgi?id=189398
 
index b6fd798..14c4448 100644 (file)
@@ -30,7 +30,6 @@
 
 #include "ExceptionOr.h"
 #include "SQLValue.h"
-#include <wtf/HashTraits.h>
 
 namespace WebCore {
 
index fea9c38..ba945cd 100644 (file)
@@ -81,7 +81,6 @@
 #include <JavaScriptCore/TypedArrays.h>
 #include <JavaScriptCore/WasmModule.h>
 #include <limits>
-#include <wtf/HashTraits.h>
 #include <wtf/MainThread.h>
 #include <wtf/RunLoop.h>
 #include <wtf/Vector.h>
index 595aa41..ed027fa 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "DebugPageOverlays.h"
 
+#include "ColorHash.h"
 #include "ElementIterator.h"
 #include "FrameView.h"
 #include "GraphicsContext.h"
index e96a41e..775044a 100644 (file)
@@ -424,11 +424,6 @@ WTF::TextStream& operator<<(WTF::TextStream&, const URL&);
 } // namespace WebCore
 
 namespace WTF {
-
-// URLHash is the default hash for String
-template<typename T> struct DefaultHash;
-template<> struct DefaultHash<WebCore::URL> {
-    typedef WebCore::URLHash Hash;
-};
-
+template<> struct DefaultHash<WebCore::URL>;
+template<> struct HashTraits<WebCore::URL>;
 } // namespace WTF
index cedc4f3..41d0ab9 100644 (file)
@@ -50,7 +50,12 @@ namespace WebCore {
 
 namespace WTF {
 
-    template<> struct HashTraits<WebCore::URL> : SimpleClassHashTraits<WebCore::URL> { };
+// URLHash is the default hash for String
+template<> struct DefaultHash<WebCore::URL> {
+    using Hash = WebCore::URLHash;
+};
+
+template<> struct HashTraits<WebCore::URL> : SimpleClassHashTraits<WebCore::URL> { };
 
 } // namespace WTF
 
index 66609cc..e584b43 100644 (file)
@@ -470,3 +470,8 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const Color&);
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ColorSpace);
 
 } // namespace WebCore
+
+namespace WTF {
+template<> struct DefaultHash<WebCore::Color>;
+template<> struct HashTraits<WebCore::Color>;
+}
index 1c3d7f1..b75fb98 100644 (file)
@@ -256,3 +256,7 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const FloatSize&);
 
 } // namespace WebCore
 
+namespace WTF {
+template<> struct DefaultHash<WebCore::FloatSize>;
+template<> struct HashTraits<WebCore::FloatSize>;
+}
index 77f1589..39f1d59 100644 (file)
@@ -215,3 +215,7 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const IntPoint&);
 
 } // namespace WebCore
 
+namespace WTF {
+template<> struct DefaultHash<WebCore::IntPoint>;
+template<> struct HashTraits<WebCore::IntPoint>;
+}
index 7ac64ca..3fbcb6b 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include <algorithm>
+#include <wtf/Forward.h>
 
 #if PLATFORM(MAC) && defined __OBJC__
 #import <Foundation/NSGeometry.h>
@@ -219,3 +220,7 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const IntSize&);
 
 } // namespace WebCore
 
+namespace WTF {
+template<> struct DefaultHash<WebCore::IntSize>;
+template<> struct HashTraits<WebCore::IntSize>;
+}
index f55fef9..9f2fc34 100644 (file)
@@ -27,6 +27,7 @@
 
 #if ENABLE(MEDIA_STREAM) && USE(AVFOUNDATION)
 
+#include "IntSizeHash.h"
 #include "OrientationNotifier.h"
 #include "RealtimeMediaSource.h"
 #include <wtf/text/StringHash.h>
index 769de80..bcd3162 100644 (file)
@@ -48,4 +48,9 @@ public:
 
 } // namespace WebCore
 
+namespace WTF {
+template<> struct DefaultHash<WebCore::ProtectionSpace>;
+template<> struct HashTraits<WebCore::ProtectionSpace>;
+}
+
 #endif
index 4526cd4..e843335 100644 (file)
@@ -59,9 +59,8 @@ namespace WTF {
 
     template<> struct HashTraits<WebCore::ProtectionSpace> : SimpleClassHashTraits<WebCore::ProtectionSpace> { };
 
-    template<typename T> struct DefaultHash;
     template<> struct DefaultHash<WebCore::ProtectionSpace> {
-        typedef WebCore::ProtectionSpaceHash Hash;
+        using Hash = WebCore::ProtectionSpaceHash;
     };
 
 } // namespace WTF
index ae6fcae..3331acc 100644 (file)
@@ -1,5 +1,14 @@
 2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
 
+        Add specialized template declarations of HashTraits and DefaultHash to detect misuse
+        https://bugs.webkit.org/show_bug.cgi?id=189044
+
+        Reviewed by Yusuke Suzuki.
+
+        * WebProcess/WebPage/WebPage.h: Added #include <WebCore/IntPointHash.h> to instantiate HashMap<std::pair<IntSize, double>, IntPoint>.
+
+2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
+
         [MSVC] X86Assembler.h(108): error C2666: 'WebCore::operator -': 7 overloads have similar conversions
         https://bugs.webkit.org/show_bug.cgi?id=189467
 
index 1449948..7b27e75 100644 (file)
@@ -92,6 +92,7 @@
 #if PLATFORM(IOS)
 #include "GestureTypes.h"
 #include "WebPageMessages.h"
+#include <WebCore/IntPointHash.h>
 #include <WebCore/ViewportConfiguration.h>
 #endif