Title: [287077] trunk/Source/WebCore
Revision
287077
Author
cdu...@apple.com
Date
2021-12-15 09:10:36 -0800 (Wed, 15 Dec 2021)

Log Message

http/tests/security/basic-auth-subresource.html and some other http auth tests are flaky
https://bugs.webkit.org/show_bug.cgi?id=234314
<rdar://85150486>

Reviewed by Darin Adler.

http/tests/security/basic-auth-subresource.html and some other http auth tests are flaky.

No new tests, I will be able to unskip those layout tests in internal once this lands.

* platform/network/ProtectionSpaceBase.cpp:
(WebCore::ProtectionSpaceBase::ProtectionSpaceBase):
(WebCore::ProtectionSpaceBase::host const): Deleted.
(WebCore::ProtectionSpaceBase::port const): Deleted.
(WebCore::ProtectionSpaceBase::serverType const): Deleted.
(WebCore::ProtectionSpaceBase::realm const): Deleted.
(WebCore::ProtectionSpaceBase::authenticationScheme const): Deleted.
* platform/network/ProtectionSpaceBase.h:
(WebCore::ProtectionSpaceBase::host const):
(WebCore::ProtectionSpaceBase::port const):
(WebCore::ProtectionSpaceBase::serverType const):
(WebCore::ProtectionSpaceBase::realm const):
(WebCore::ProtectionSpaceBase::authenticationScheme const):
Clean up / modernise the ProtectionSpaceBase class.

* platform/network/ProtectionSpaceHash.h:
(WebCore::ProtectionSpaceHash::hash):
- Use Hasher in ProtectionSpaceHash::hash() as it is less error-prone. I believe the
  previous implementation was wrong because it was calling
  `StringHasher::hashMemory(hashCodes, codeCount)` instead of
  `StringHasher::hashMemory(hashCodes, codeCount * sizeof(unsigned))`.
  This could have resulted in inefficiencies I believe since we were not hashing the
  whole array memory.
- Fix ProtectionSpace<ProtectionSpace> so that emptyValueIsZero is false instead of
  true. This was a bug since the ProtectionSpaceBase constructor initializes data
  members to non-zero values.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287076 => 287077)


--- trunk/Source/WebCore/ChangeLog	2021-12-15 16:59:20 UTC (rev 287076)
+++ trunk/Source/WebCore/ChangeLog	2021-12-15 17:10:36 UTC (rev 287077)
@@ -1,3 +1,42 @@
+2021-12-15  Chris Dumez  <cdu...@apple.com>
+
+        http/tests/security/basic-auth-subresource.html and some other http auth tests are flaky
+        https://bugs.webkit.org/show_bug.cgi?id=234314
+        <rdar://85150486>
+
+        Reviewed by Darin Adler.
+
+        http/tests/security/basic-auth-subresource.html and some other http auth tests are flaky.
+
+        No new tests, I will be able to unskip those layout tests in internal once this lands.
+
+        * platform/network/ProtectionSpaceBase.cpp:
+        (WebCore::ProtectionSpaceBase::ProtectionSpaceBase):
+        (WebCore::ProtectionSpaceBase::host const): Deleted.
+        (WebCore::ProtectionSpaceBase::port const): Deleted.
+        (WebCore::ProtectionSpaceBase::serverType const): Deleted.
+        (WebCore::ProtectionSpaceBase::realm const): Deleted.
+        (WebCore::ProtectionSpaceBase::authenticationScheme const): Deleted.
+        * platform/network/ProtectionSpaceBase.h:
+        (WebCore::ProtectionSpaceBase::host const):
+        (WebCore::ProtectionSpaceBase::port const):
+        (WebCore::ProtectionSpaceBase::serverType const):
+        (WebCore::ProtectionSpaceBase::realm const):
+        (WebCore::ProtectionSpaceBase::authenticationScheme const):
+        Clean up / modernise the ProtectionSpaceBase class.
+
+        * platform/network/ProtectionSpaceHash.h:
+        (WebCore::ProtectionSpaceHash::hash):
+        - Use Hasher in ProtectionSpaceHash::hash() as it is less error-prone. I believe the
+          previous implementation was wrong because it was calling
+          `StringHasher::hashMemory(hashCodes, codeCount)` instead of
+          `StringHasher::hashMemory(hashCodes, codeCount * sizeof(unsigned))`.
+          This could have resulted in inefficiencies I believe since we were not hashing the
+          whole array memory.
+        - Fix ProtectionSpace<ProtectionSpace> so that emptyValueIsZero is false instead of
+          true. This was a bug since the ProtectionSpaceBase constructor initializes data
+          members to non-zero values.
+
 2021-12-15  Gavin Phillips  <gavi...@apple.com>
 
         Fix SVG resource invalidation logic causing incorrect layout state.

Modified: trunk/Source/WebCore/platform/network/ProtectionSpaceBase.cpp (287076 => 287077)


--- trunk/Source/WebCore/platform/network/ProtectionSpaceBase.cpp	2021-12-15 16:59:20 UTC (rev 287076)
+++ trunk/Source/WebCore/platform/network/ProtectionSpaceBase.cpp	2021-12-15 17:10:36 UTC (rev 287077)
@@ -33,46 +33,18 @@
 #endif
 
 namespace WebCore {
-
-// Need to enforce empty, non-null strings due to the pickiness of the String == String operator
-// combined with the semantics of the String(NSString*) constructor
-ProtectionSpaceBase::ProtectionSpaceBase()
-    : m_host(emptyString())
-    , m_port(0)
-    , m_serverType(ProtectionSpaceServerHTTP)
-    , m_realm(emptyString())
-    , m_authenticationScheme(ProtectionSpaceAuthenticationSchemeDefault)
-    , m_isHashTableDeletedValue(false)
-{
-}
  
 // Need to enforce empty, non-null strings due to the pickiness of the String == String operator
 // combined with the semantics of the String(NSString*) constructor
 ProtectionSpaceBase::ProtectionSpaceBase(const String& host, int port, ProtectionSpaceServerType serverType, const String& realm, ProtectionSpaceAuthenticationScheme authenticationScheme)
     : m_host(host.length() ? host : emptyString())
+    , m_realm(realm.length() ? realm : emptyString())
     , m_port(port)
     , m_serverType(serverType)
-    , m_realm(realm.length() ? realm : emptyString())
     , m_authenticationScheme(authenticationScheme)
-    , m_isHashTableDeletedValue(false)
 {    
 }
-    
-const String& ProtectionSpaceBase::host() const
-{
-    return m_host; 
-}
 
-int ProtectionSpaceBase::port() const
-{
-    return m_port; 
-}
-
-ProtectionSpaceServerType ProtectionSpaceBase::serverType() const
-{
-    return m_serverType;
-}
-
 bool ProtectionSpaceBase::isProxy() const
 {
     return (m_serverType == ProtectionSpaceProxyHTTP ||
@@ -81,16 +53,6 @@
             m_serverType == ProtectionSpaceProxySOCKS);
 }
 
-const String& ProtectionSpaceBase::realm() const
-{ 
-    return m_realm; 
-}
-
-ProtectionSpaceAuthenticationScheme ProtectionSpaceBase::authenticationScheme() const
-{ 
-    return m_authenticationScheme; 
-}
-
 bool ProtectionSpaceBase::receivesCredentialSecurely() const
 {
     return (m_serverType == ProtectionSpaceServerHTTPS ||

Modified: trunk/Source/WebCore/platform/network/ProtectionSpaceBase.h (287076 => 287077)


--- trunk/Source/WebCore/platform/network/ProtectionSpaceBase.h	2021-12-15 16:59:20 UTC (rev 287076)
+++ trunk/Source/WebCore/platform/network/ProtectionSpaceBase.h	2021-12-15 17:10:36 UTC (rev 287077)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2020 Apple Inc.  All rights reserved.
+ * Copyright (C) 2007-2021 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -64,12 +64,12 @@
 public:
     bool isHashTableDeletedValue() const { return m_isHashTableDeletedValue; }
     
-    WEBCORE_EXPORT const String& host() const;
-    WEBCORE_EXPORT int port() const;
-    WEBCORE_EXPORT ProtectionSpaceServerType serverType() const;
+    const String& host() const { return m_host; }
+    int port() const { return m_port; }
+    ProtectionSpaceServerType serverType() const { return m_serverType; }
     WEBCORE_EXPORT bool isProxy() const;
-    WEBCORE_EXPORT const String& realm() const;
-    WEBCORE_EXPORT ProtectionSpaceAuthenticationScheme authenticationScheme() const;
+    const String& realm() const { return m_realm; }
+    ProtectionSpaceAuthenticationScheme authenticationScheme() const { return m_authenticationScheme; }
     
     WEBCORE_EXPORT bool receivesCredentialSecurely() const;
     WEBCORE_EXPORT bool isPasswordBased() const;
@@ -79,7 +79,7 @@
     WEBCORE_EXPORT static bool compare(const ProtectionSpace&, const ProtectionSpace&);
 
 protected:
-    WEBCORE_EXPORT ProtectionSpaceBase();
+    ProtectionSpaceBase() = default;
     WEBCORE_EXPORT ProtectionSpaceBase(const String& host, int port, ProtectionSpaceServerType, const String& realm, ProtectionSpaceAuthenticationScheme);
 
     // Hash table deleted values, which are only constructed and never copied or destroyed.
@@ -88,12 +88,15 @@
     static bool platformCompare(const ProtectionSpace&, const ProtectionSpace&) { return true; }
 
 private:
-    String m_host;
-    int m_port;
-    ProtectionSpaceServerType m_serverType;
-    String m_realm;
-    ProtectionSpaceAuthenticationScheme m_authenticationScheme;
-    bool m_isHashTableDeletedValue;
+    // Need to enforce empty, non-null strings due to the pickiness of the String == String operator
+    // combined with the semantics of the String(NSString*) constructor
+    String m_host { emptyString() };
+    String m_realm { emptyString() };
+
+    int m_port { 0 };
+    ProtectionSpaceServerType m_serverType { ProtectionSpaceServerHTTP };
+    ProtectionSpaceAuthenticationScheme m_authenticationScheme { ProtectionSpaceAuthenticationSchemeDefault };
+    bool m_isHashTableDeletedValue { false };
 };
 
 inline bool operator==(const ProtectionSpace& a, const ProtectionSpace& b) { return ProtectionSpaceBase::compare(a, b); }

Modified: trunk/Source/WebCore/platform/network/ProtectionSpaceHash.h (287076 => 287077)


--- trunk/Source/WebCore/platform/network/ProtectionSpaceHash.h	2021-12-15 16:59:20 UTC (rev 287076)
+++ trunk/Source/WebCore/platform/network/ProtectionSpaceHash.h	2021-12-15 17:10:36 UTC (rev 287077)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2009-2021 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -23,11 +23,11 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
  */
 
-#ifndef ProtectionSpaceHash_h
-#define ProtectionSpaceHash_h
+#pragma once
 
 #include "ProtectionSpace.h"
 #include <wtf/HashTraits.h>
+#include <wtf/Hasher.h>
 
 namespace WebCore {
 
@@ -34,23 +34,18 @@
 struct ProtectionSpaceHash {
     static unsigned hash(const ProtectionSpace& protectionSpace)
     { 
-        unsigned hashCodes[5] = {
-            protectionSpace.host().impl() ? protectionSpace.host().impl()->hash() : 0, 
-            static_cast<unsigned>(protectionSpace.port()),
-            static_cast<unsigned>(protectionSpace.serverType()),
-            static_cast<unsigned>(protectionSpace.authenticationScheme()),
-            protectionSpace.realm().impl() ? protectionSpace.realm().impl()->hash() : 0
-        };
-
-        unsigned codeCount = sizeof(hashCodes);
-        // Ignore realm for proxies.
-        if (protectionSpace.isProxy())
-            codeCount -= sizeof(hashCodes[0]);
-        return StringHasher::hashMemory(hashCodes, codeCount);
+        Hasher hasher;
+        add(hasher, protectionSpace.host());
+        add(hasher, protectionSpace.port());
+        add(hasher, protectionSpace.serverType());
+        add(hasher, protectionSpace.authenticationScheme());
+        if (!protectionSpace.isProxy())
+            add(hasher, protectionSpace.realm());
+        return hasher.hash();
     }
     
     static bool equal(const ProtectionSpace& a, const ProtectionSpace& b) { return a == b; }
-    static const bool safeToCompareToEmptyOrDeleted = false;
+    static constexpr bool safeToCompareToEmptyOrDeleted = false;
 };
 
 } // namespace WebCore
@@ -57,10 +52,9 @@
 
 namespace WTF {
 
-template<> struct HashTraits<WebCore::ProtectionSpace> : SimpleClassHashTraits<WebCore::ProtectionSpace> { };
+template<> struct HashTraits<WebCore::ProtectionSpace> : SimpleClassHashTraits<WebCore::ProtectionSpace> {
+    static constexpr bool emptyValueIsZero = false;
+};
 template<> struct DefaultHash<WebCore::ProtectionSpace> : WebCore::ProtectionSpaceHash { };
 
 } // namespace WTF
-
-
-#endif // ProtectionSpaceHash_h
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to