HTTPHeaderMap should not derive from HashMap
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jun 2014 20:17:20 +0000 (20:17 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jun 2014 20:17:20 +0000 (20:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133392

Reviewed by Darin Adler.

Source/WebCore:
Use a HashMap member variable instead.

* WebCore.exp.in:
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::responseReceived):
* platform/network/HTTPHeaderMap.cpp:
(WebCore::HTTPHeaderMap::copyData):
(WebCore::HTTPHeaderMap::adopt):
(WebCore::HTTPHeaderMap::get):
(WebCore::HTTPHeaderMap::set):
(WebCore::HTTPHeaderMap::add):
(WebCore::HTTPHeaderMap::contains):
(WebCore::HTTPHeaderMap::find):
(WebCore::HTTPHeaderMap::remove):
(WebCore::HTTPHeaderMap::keys):
* platform/network/HTTPHeaderMap.h:
(WebCore::HTTPHeaderMap::isEmpty):
(WebCore::HTTPHeaderMap::size):
(WebCore::HTTPHeaderMap::clear):
(WebCore::HTTPHeaderMap::begin):
(WebCore::HTTPHeaderMap::end):
(WebCore::HTTPHeaderMap::operator==):
(WebCore::HTTPHeaderMap::operator!=):
* platform/network/ResourceRequestBase.cpp:
(WebCore::ResourceRequestBase::adopt):
* platform/network/ResourceRequestBase.h:
* platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::adopt):
* platform/network/ResourceResponseBase.h:
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::createRequest):

Source/WebKit2:
* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<HTTPHeaderMap>::encode):
(IPC::ArgumentCoder<HTTPHeaderMap>::decode):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/platform/network/HTTPHeaderMap.cpp
Source/WebCore/platform/network/HTTPHeaderMap.h
Source/WebCore/platform/network/ResourceRequestBase.cpp
Source/WebCore/platform/network/ResourceRequestBase.h
Source/WebCore/platform/network/ResourceResponseBase.cpp
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp
Source/WebCore/platform/network/soup/ResourceRequest.h
Source/WebCore/xml/XMLHttpRequest.cpp
Source/WebKit/win/WebURLResponse.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/mac/DiskCacheMonitor.mm
Source/WebKit2/Shared/WebCoreArgumentCoders.cpp

index 050d28e..1617212 100644 (file)
@@ -1,3 +1,42 @@
+2014-05-29  Anders Carlsson  <andersca@apple.com>
+
+        HTTPHeaderMap should not derive from HashMap
+        https://bugs.webkit.org/show_bug.cgi?id=133392
+
+        Reviewed by Darin Adler.
+
+        Use a HashMap member variable instead.
+
+        * WebCore.exp.in:
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::responseReceived):
+        * platform/network/HTTPHeaderMap.cpp:
+        (WebCore::HTTPHeaderMap::copyData):
+        (WebCore::HTTPHeaderMap::adopt):
+        (WebCore::HTTPHeaderMap::get):
+        (WebCore::HTTPHeaderMap::set):
+        (WebCore::HTTPHeaderMap::add):
+        (WebCore::HTTPHeaderMap::contains):
+        (WebCore::HTTPHeaderMap::find):
+        (WebCore::HTTPHeaderMap::remove):
+        (WebCore::HTTPHeaderMap::keys):
+        * platform/network/HTTPHeaderMap.h:
+        (WebCore::HTTPHeaderMap::isEmpty):
+        (WebCore::HTTPHeaderMap::size):
+        (WebCore::HTTPHeaderMap::clear):
+        (WebCore::HTTPHeaderMap::begin):
+        (WebCore::HTTPHeaderMap::end):
+        (WebCore::HTTPHeaderMap::operator==):
+        (WebCore::HTTPHeaderMap::operator!=):
+        * platform/network/ResourceRequestBase.cpp:
+        (WebCore::ResourceRequestBase::adopt):
+        * platform/network/ResourceRequestBase.h:
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::adopt):
+        * platform/network/ResourceResponseBase.h:
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::createRequest):
+
 2014-06-07  Zan Dobersek  <zdobersek@igalia.com>
 
         Use C++11 lambdas to construct FileThread::Task objects
index ae4751e..af7315e 100644 (file)
@@ -289,6 +289,9 @@ __ZN7WebCore13GraphicsLayer7setSizeERKNS_9FloatSizeE
 __ZN7WebCore13GraphicsLayer8addChildEPS0_
 __ZN7WebCore13GraphicsLayerC2ERNS_19GraphicsLayerClientE
 __ZN7WebCore13GraphicsLayerD2Ev
+__ZN7WebCore13HTTPHeaderMap3addERKN3WTF12AtomicStringERKNS1_6StringE
+__ZN7WebCore13HTTPHeaderMap3setERKN3WTF12AtomicStringERKNS1_6StringE
+__ZN7WebCore13HTTPHeaderMap6removeEPKc
 __ZN7WebCore13HTTPHeaderMapC1Ev
 __ZN7WebCore13HTTPHeaderMapD1Ev
 __ZN7WebCore13HitTestResultC1ERKNS_11LayoutPointE
index 6cabb96..54b075d 100644 (file)
@@ -604,9 +604,7 @@ void DocumentLoader::responseReceived(CachedResource* resource, const ResourceRe
     if (willLoadFallback)
         return;
 
-    DEPRECATED_DEFINE_STATIC_LOCAL(AtomicString, xFrameOptionHeader, ("x-frame-options", AtomicString::ConstructFromLiteral));
-
-    auto it = response.httpHeaderFields().find(xFrameOptionHeader);
+    auto it = response.httpHeaderFields().find("x-frame-options");
     if (it != response.httpHeaderFields().end()) {
         String content = it->value;
         ASSERT(m_mainResource);
index c6d5c7d..4ca944b 100644 (file)
@@ -43,35 +43,38 @@ HTTPHeaderMap::~HTTPHeaderMap()
 {
 }
 
-PassOwnPtr<CrossThreadHTTPHeaderMapData> HTTPHeaderMap::copyData() const
+std::unique_ptr<CrossThreadHTTPHeaderMapData> HTTPHeaderMap::copyData() const
 {
-    OwnPtr<CrossThreadHTTPHeaderMapData> data = adoptPtr(new CrossThreadHTTPHeaderMapData());
-    data->reserveInitialCapacity(size());
+    auto data = std::make_unique<CrossThreadHTTPHeaderMapData>();
+    data->reserveInitialCapacity(m_headers.size());
 
     for (const auto& header : *this)
         data->uncheckedAppend(std::make_pair(header.key.string().isolatedCopy(), header.value.isolatedCopy()));
 
-    return data.release();
+    return data;
 }
 
-void HTTPHeaderMap::adopt(PassOwnPtr<CrossThreadHTTPHeaderMapData> data)
+void HTTPHeaderMap::adopt(std::unique_ptr<CrossThreadHTTPHeaderMapData> data)
 {
-    clear();
-    size_t dataSize = data->size();
-    for (size_t index = 0; index < dataSize; ++index) {
-        std::pair<String, String>& header = (*data)[index];
-        set(header.first, header.second);
-    }
+    m_headers.clear();
+
+    for (auto& header : *data)
+        m_headers.add(std::move(header.first), std::move(header.second));
 }
 
 String HTTPHeaderMap::get(const AtomicString& name) const
 {
-    return HashMap<AtomicString, String, CaseFoldingHash>::get(name);
+    return m_headers.get(name);
+}
+
+HTTPHeaderMap::AddResult HTTPHeaderMap::set(const AtomicString& name, const String& value)
+{
+    return m_headers.set(name, value);
 }
 
 HTTPHeaderMap::AddResult HTTPHeaderMap::add(const AtomicString& name, const String& value)
 {
-    return HashMap<AtomicString, String, CaseFoldingHash>::add(name, value);
+    return m_headers.add(name, value);
 }
 
 // Adapter that allows the HashMap to take C strings as keys.
@@ -94,20 +97,46 @@ struct CaseFoldingCStringTranslator {
 
 String HTTPHeaderMap::get(const char* name) const
 {
-    const_iterator i = find<CaseFoldingCStringTranslator>(name);
-    if (i == end())
+    auto it = find(name);
+    if (it == end())
         return String();
-    return i->value;
+    return it->value;
 }
     
 bool HTTPHeaderMap::contains(const char* name) const
 {
-    return find<CaseFoldingCStringTranslator>(name) != end();
+    return find(name) != end();
+}
+
+HTTPHeaderMap::iterator HTTPHeaderMap::find(const char* name)
+{
+    return m_headers.find<CaseFoldingCStringTranslator>(name);
+}
+
+HTTPHeaderMap::const_iterator HTTPHeaderMap::find(const char* name) const
+{
+    return m_headers.find<CaseFoldingCStringTranslator>(name);
 }
 
 HTTPHeaderMap::AddResult HTTPHeaderMap::add(const char* name, const String& value)
 {
-    return HashMap<AtomicString, String, CaseFoldingHash>::add<CaseFoldingCStringTranslator>(name, value);
+    return m_headers.add<CaseFoldingCStringTranslator>(name, value);
 }
 
+void HTTPHeaderMap::remove(const char* name)
+{
+    remove(find(name));
+}
+
+void HTTPHeaderMap::remove(iterator it)
+{
+    m_headers.remove(it);
+}
+
+WTF::IteratorRange<HTTPHeaderMap::const_iterator::Keys> HTTPHeaderMap::keys() const
+{
+    return m_headers.keys();
+}
+
+
 } // namespace WebCore
index 11615ed..cd9d848 100644 (file)
@@ -29,7 +29,6 @@
 
 #include <utility>
 #include <wtf/HashMap.h>
-#include <wtf/PassOwnPtr.h>
 #include <wtf/Vector.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/AtomicStringHash.h>
 
 namespace WebCore {
 
-    typedef Vector<std::pair<String, String>> CrossThreadHTTPHeaderMapData;
+typedef Vector<std::pair<String, String>> CrossThreadHTTPHeaderMapData;
 
-    // FIXME: Not every header fits into a map. Notably, multiple Set-Cookie header fields are needed to set multiple cookies.
-    class HTTPHeaderMap : public HashMap<AtomicString, String, CaseFoldingHash> {
-    public:
-        HTTPHeaderMap();
-        ~HTTPHeaderMap();
+// FIXME: Not every header fits into a map. Notably, multiple Set-Cookie header fields are needed to set multiple cookies.
 
-        // Gets a copy of the data suitable for passing to another thread.
-        PassOwnPtr<CrossThreadHTTPHeaderMapData> copyData() const;
+class HTTPHeaderMap {
+    typedef HashMap<AtomicString, String, CaseFoldingHash> HashMapType;
+public:
+    typedef HashMapType::const_iterator const_iterator;
+    typedef HashMapType::iterator iterator;
+    typedef HashMapType::AddResult AddResult;
 
-        void adopt(PassOwnPtr<CrossThreadHTTPHeaderMapData>);
-        
-        String get(const AtomicString& name) const;
+    HTTPHeaderMap();
+    ~HTTPHeaderMap();
 
-        AddResult add(const AtomicString& name, const String& value);
+    // Gets a copy of the data suitable for passing to another thread.
+    std::unique_ptr<CrossThreadHTTPHeaderMapData> copyData() const;
+    void adopt(std::unique_ptr<CrossThreadHTTPHeaderMapData>);
 
-        // Alternate accessors that are faster than converting the char* to AtomicString first.
-        bool contains(const char*) const;
-        String get(const char*) const;
-        AddResult add(const char* name, const String& value);
-        
-    };
+    bool isEmpty() const { return m_headers.isEmpty(); }
+    int size() const { return m_headers.size(); }
+
+    void clear() { m_headers.clear(); }
+
+    String get(const AtomicString& name) const;
+
+    AddResult set(const AtomicString& name, const String& value);
+    AddResult add(const AtomicString& name, const String& value);
+
+    // Alternate accessors that are faster than converting the char* to AtomicString first.
+    bool contains(const char*) const;
+    String get(const char*) const;
+    const_iterator find(const char*) const;
+    iterator find(const char*);
+    AddResult add(const char* name, const String& value);
+
+    void remove(const char*);
+    void remove(iterator);
+
+    const_iterator begin() const { return m_headers.begin(); }
+    const_iterator end() const { return m_headers.end(); }
+
+    WTF::IteratorRange<const_iterator::Keys> keys() const;
+
+    friend bool operator==(const HTTPHeaderMap& a, const HTTPHeaderMap& b)
+    {
+        return a.m_headers == b.m_headers;
+    }
+
+    friend bool operator!=(const HTTPHeaderMap& a, const HTTPHeaderMap& b)
+    {
+        return !(a == b);
+    }
+
+private:
+    HashMap<AtomicString, String, CaseFoldingHash> m_headers;
+};
 
 } // namespace WebCore
 
index 701b7c1..f0db647 100644 (file)
@@ -27,6 +27,7 @@
 #include "ResourceRequestBase.h"
 
 #include "ResourceRequest.h"
+#include <wtf/PassOwnPtr.h>
 
 namespace WebCore {
 
@@ -58,7 +59,7 @@ PassOwnPtr<ResourceRequest> ResourceRequestBase::adopt(PassOwnPtr<CrossThreadRes
     request->setPriority(data->m_priority);
 
     request->updateResourceRequest();
-    request->m_httpHeaderFields.adopt(data->m_httpHeaders.release());
+    request->m_httpHeaderFields.adopt(std::move(data->m_httpHeaders));
 
     size_t encodingCount = data->m_responseContentDispositionEncodingFallbackArray.size();
     if (encodingCount > 0) {
index ab72ada..6c48398 100644 (file)
@@ -33,8 +33,6 @@
 #include "URL.h"
 #include "ResourceLoadPriority.h"
 
-#include <wtf/OwnPtr.h>
-
 namespace WebCore {
 
     enum ResourceRequestCachePolicy {
@@ -248,7 +246,7 @@ namespace WebCore {
         URL m_firstPartyForCookies;
 
         String m_httpMethod;
-        OwnPtr<CrossThreadHTTPHeaderMapData> m_httpHeaders;
+        std::unique_ptr<CrossThreadHTTPHeaderMapData> m_httpHeaders;
         Vector<String> m_responseContentDispositionEncodingFallbackArray;
         RefPtr<FormData> m_httpBody;
         bool m_allowCookies;
index 36ffc01..cb9af24 100644 (file)
@@ -105,7 +105,7 @@ PassOwnPtr<ResourceResponse> ResourceResponseBase::adopt(PassOwnPtr<CrossThreadR
     response->setHTTPStatusText(data->m_httpStatusText);
 
     response->lazyInit(CommonAndUncommonFields);
-    response->m_httpHeaderFields.adopt(data->m_httpHeaders.release());
+    response->m_httpHeaderFields.adopt(std::move(data->m_httpHeaders));
     response->setResourceLoadTiming(data->m_resourceLoadTiming);
     response->doPlatformAdopt(data);
     return response.release();
index c1f3e43..8bdadab 100644 (file)
@@ -195,7 +195,7 @@ public:
     String m_suggestedFilename;
     int m_httpStatusCode;
     String m_httpStatusText;
-    OwnPtr<CrossThreadHTTPHeaderMapData> m_httpHeaders;
+    std::unique_ptr<CrossThreadHTTPHeaderMapData> m_httpHeaders;
     ResourceLoadTiming m_resourceLoadTiming;
 };
 
index 741b816..1f7b4c2 100644 (file)
@@ -27,6 +27,7 @@
 #include "ResourceRequestCFNet.h"
 
 #include "ResourceRequest.h"
+#include <wtf/PassOwnPtr.h>
 
 #if ENABLE(PUBLIC_SUFFIX_LIST)
 #include "PublicSuffix.h"
index c9aab14..44c8345 100644 (file)
@@ -30,6 +30,7 @@
 #include "GUniquePtrSoup.h"
 #include "ResourceRequestBase.h"
 #include <libsoup/soup.h>
+#include <wtf/PassOwnPtr.h>
 
 namespace WebCore {
 
index 39260b8..4392ac6 100644 (file)
@@ -758,7 +758,7 @@ void XMLHttpRequest::createRequest(ExceptionCode& ec)
         request.setHTTPBody(m_requestEntityBody.release());
     }
 
-    if (m_requestHeaders.size() > 0)
+    if (!m_requestHeaders.isEmpty())
         request.addHTTPHeaderFields(m_requestHeaders);
 
     ThreadableLoaderOptions options;
index 37a68a7..701a5f6 100644 (file)
@@ -360,7 +360,11 @@ HRESULT STDMETHODCALLTYPE WebURLResponse::allHeaderFields(
 {
     ASSERT(m_response.isHTTP());
 
-    *headerFields = COMPropertyBag<String, AtomicString, CaseFoldingHash>::createInstance(m_response.httpHeaderFields());
+    HashMap<String, String, CaseFoldingHash> fields;
+    for (const auto& keyValuePair : m_response.httpHeaderFields())
+        fields.add(keyValuePair.key, keyValuePair.value);
+
+    *headerFields = COMPropertyBag<String, String, CaseFoldingHash>::adopt(fields);
     return S_OK;
 }
 
index 2075243..cf63d01 100644 (file)
@@ -1,3 +1,14 @@
+2014-05-29  Anders Carlsson  <andersca@apple.com>
+
+        HTTPHeaderMap should not derive from HashMap
+        https://bugs.webkit.org/show_bug.cgi?id=133392
+
+        Reviewed by Darin Adler.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<HTTPHeaderMap>::encode):
+        (IPC::ArgumentCoder<HTTPHeaderMap>::decode):
+
 2014-06-07  Alexey Proskuryakov  <ap@apple.com>
 
         [iOS] Fix a path used for sandbox profiles
index cbd954c..7565c90 100644 (file)
@@ -30,6 +30,7 @@
 #import "NetworkProcessConnectionMessages.h"
 #import "NetworkResourceLoader.h"
 #import "WebCoreArgumentCoders.h"
+#import <wtf/PassOwnPtr.h>
 
 #ifdef __has_include
 #if __has_include(<CFNetwork/CFURLCachePriv.h>)
index d1401aa..c2769e0 100644 (file)
@@ -395,12 +395,35 @@ bool ArgumentCoder<PluginInfo>::decode(ArgumentDecoder& decoder, PluginInfo& plu
 
 void ArgumentCoder<HTTPHeaderMap>::encode(ArgumentEncoder& encoder, const HTTPHeaderMap& headerMap)
 {
-    encoder << static_cast<const HashMap<AtomicString, String, CaseFoldingHash>&>(headerMap);
+    encoder << static_cast<uint64_t>(headerMap.size());
+    for (auto& keyValuePair : headerMap) {
+        encoder << keyValuePair.key;
+        encoder << keyValuePair.value;
+    }
 }
 
 bool ArgumentCoder<HTTPHeaderMap>::decode(ArgumentDecoder& decoder, HTTPHeaderMap& headerMap)
 {
-    return decoder.decode(static_cast<HashMap<AtomicString, String, CaseFoldingHash>&>(headerMap));
+    uint64_t size;
+    if (!decoder.decode(size))
+        return false;
+
+    for (size_t i = 0; i < size; ++i) {
+        AtomicString name;
+        if (!decoder.decode(name))
+            return false;
+
+        String value;
+        if (!decoder.decode(value))
+            return false;
+
+        if (!headerMap.add(name, value).isNewEntry) {
+            decoder.markInvalid();
+            return false;
+        }
+    }
+
+    return true;
 }