Title: [287336] branches/safari-612-branch/Source/WebCore
Revision
287336
Author
[email protected]
Date
2021-12-21 16:14:30 -0800 (Tue, 21 Dec 2021)

Log Message

Cherry-pick r287077. rdar://problem/85150486

    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.

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

Modified Paths

Diff

Modified: branches/safari-612-branch/Source/WebCore/ChangeLog (287335 => 287336)


--- branches/safari-612-branch/Source/WebCore/ChangeLog	2021-12-22 00:14:24 UTC (rev 287335)
+++ branches/safari-612-branch/Source/WebCore/ChangeLog	2021-12-22 00:14:30 UTC (rev 287336)
@@ -1,5 +1,88 @@
 2021-12-21  Alan Coon  <[email protected]>
 
+        Cherry-pick r287077. rdar://problem/85150486
+
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287077 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-12-15  Chris Dumez  <[email protected]>
+
+            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-21  Alan Coon  <[email protected]>
+
         Cherry-pick r287067. rdar://problem/86276497
 
     Make sure to start a realtime outgoing source in case it is taken to another sender

Modified: branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.cpp (287335 => 287336)


--- branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.cpp	2021-12-22 00:14:24 UTC (rev 287335)
+++ branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.cpp	2021-12-22 00:14:30 UTC (rev 287336)
@@ -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: branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.h (287335 => 287336)


--- branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.h	2021-12-22 00:14:24 UTC (rev 287335)
+++ branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceBase.h	2021-12-22 00:14:30 UTC (rev 287336)
@@ -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: branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceHash.h (287335 => 287336)


--- branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceHash.h	2021-12-22 00:14:24 UTC (rev 287335)
+++ branches/safari-612-branch/Source/WebCore/platform/network/ProtectionSpaceHash.h	2021-12-22 00:14:30 UTC (rev 287336)
@@ -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
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to