Attachment 371749 Details for Bug 198635 – Patch for landing

[patch] Patch for landing

bug-198635-20190610085759.patch (text/plain), 7.77 KB, created by Sihui Liu on 2019-06-10 08:57:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-06-10 08:57:59 PDT

Size: 7.77 KB

patch

obsolete

>Subversion Revision: 246225
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1d11ad9236605671450624636dce5d9431cd08e9..dd901d1e392763d80c9718635f185b3866b99d06 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-06-10  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Test: WebKit.WKHTTPCookieStoreWithoutProcessPoolDuplicates
>+
>+        * platform/Cookie.h:
>+        (WebCore::Cookie::isKeyEqual const):
>+        (WTF::HashTraits<WebCore::Cookie>::isEmptyValue):
>+
> 2019-06-07  Truitt Savell  <tsavell@apple.com>
> 
>         Unreviewed, rolling out r246138.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4ca0f11eac727295b5d234531f74738f3a08bebc..a8ab9244d38dccc83e6a72be20dc6663aa645d13 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-06-10  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        When there is no process pool, we store cookies set in memory with HashSet m_pendingCookies of WebsiteDataStore.
>+
>+        HashSet does not contain duplicate Cookies that are completely identical, but it may contain Cookies that have 
>+        all the other properties identical other than value. This is not correct because Cookies with same name, domain
>+        and path should be treated as the same cookie. When a cookie is set via API, we should either insert the 
>+        cookie into m_pendingCookies if the cookie does not exist, or update the cookie value if it already exists.
>+
>+        Note that we still use HashSet with CookieHash for m_pendingCookies because in cookie deletion, we only delete 
>+        cookie when there is a complete match. If some cookie from m_pendingCookies has all other properties the same as
>+        the cookie specified in the deletion function, but the value is different, it will not be removed.
>+
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::addPendingCookie):
>+
> 2019-06-05  Alex Christensen  <achristensen@webkit.org>
> 
>         Introduce new SPI for context menus on iOS
>diff --git a/Source/WebCore/platform/Cookie.h b/Source/WebCore/platform/Cookie.h
>index 1e05fcb362724c27fdabd8e2fb509490388c8dbe..443333646f6746dd1fd5de171a05a9cf9036de9d 100644
>--- a/Source/WebCore/platform/Cookie.h
>+++ b/Source/WebCore/platform/Cookie.h
>@@ -76,6 +76,13 @@ struct Cookie {
>             && commentURL.isNull();
>     }
>     
>+    bool isKeyEqual(const Cookie& otherCookie) const
>+    {
>+        return name == otherCookie.name
>+            && domain == otherCookie.domain
>+            && path == otherCookie.path;
>+    }
>+
>     String name;
>     String value;
>     String domain;
>@@ -169,6 +176,9 @@ namespace WTF {
>         static WebCore::Cookie emptyValue() { return { }; }
>         static void constructDeletedValue(WebCore::Cookie& slot) { slot = WebCore::Cookie(WTF::HashTableDeletedValue); }
>         static bool isDeletedValue(const WebCore::Cookie& slot) { return slot.name.isHashTableDeletedValue(); }
>+
>+        static const bool hasIsEmptyValueFunction = true;
>+        static bool isEmptyValue(const WebCore::Cookie& slot) { return slot.isNull(); }
>     };
>     template<> struct EnumTraits<WebCore::Cookie::SameSitePolicy> {
>     using values = EnumValues<
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index 3ba29b31e0f3e89ba154587c1521a8aea7869a4f..c28a640f7773d40e2089094db68ca13ba76e3f52 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -1941,6 +1941,9 @@ Vector<WebCore::Cookie> WebsiteDataStore::pendingCookies() const
> 
> void WebsiteDataStore::addPendingCookie(const WebCore::Cookie& cookie)
> {
>+    m_pendingCookies.removeIf([&cookie](auto& pendingCookie) {
>+        return pendingCookie.isKeyEqual(cookie);
>+    });
>     m_pendingCookies.add(cookie);
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index b47011d7b8a5873a696cffee1d8594a99ecac0e0..0dc7c14f24f3faa65fb0003859abe7fb3fc9e9dd 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-10  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (areCookiesEqual):
>+        (TEST):
>+
> 2019-06-07  Daniel Bates  <dabates@apple.com>
> 
>         [lldb-webkit] Pretty-print all kinds of Documents
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 0f1e79ee16029dcb1ca8651e48efbe697b04020c..6e3b8534df82cdd4c2f7194b018cb4ad654b9f86 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -627,3 +627,58 @@ TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolEphemeralSession)
>     [webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
>     TestWebKitAPI::Util::run(&finished);
> }
>+
>+static bool areCookiesEqual(NSHTTPCookie *first, NSHTTPCookie *second)
>+{
>+    return [first.name isEqual:second.name] && [first.domain isEqual:second.domain] && [first.path isEqual:second.path] && [first.value isEqual:second.value];
>+}
>+
>+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolDuplicates)
>+{
>+    RetainPtr<WKHTTPCookieStore> httpCookieStore = [WKWebsiteDataStore defaultDataStore].httpCookieStore;
>+    RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
>+        NSHTTPCookiePath: @"/",
>+        NSHTTPCookieName: @"SessionCookieName",
>+        NSHTTPCookieValue: @"CookieValue",
>+        NSHTTPCookieDomain: @"127.0.0.1",
>+    }];
>+
>+    auto properties = adoptNS([sessionCookie.get().properties mutableCopy]);
>+    properties.get()[NSHTTPCookieDomain] = @"localhost";
>+    RetainPtr<NSHTTPCookie> sessionCookieDifferentDomain = [NSHTTPCookie cookieWithProperties:properties.get()];
>+    properties.get()[NSHTTPCookieValue] = @"OtherCookieValue";
>+    RetainPtr<NSHTTPCookie> sessionCookieDifferentValue = [NSHTTPCookie cookieWithProperties:properties.get()];
>+    finished = false;
>+    
>+    [httpCookieStore.get() setCookie:sessionCookie.get() completionHandler:^{
>+        finished = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+    finished = false;
>+
>+    [httpCookieStore.get() setCookie:sessionCookieDifferentDomain.get() completionHandler:^{
>+        finished = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+    finished = false;
>+
>+    [httpCookieStore.get() setCookie:sessionCookieDifferentValue.get() completionHandler:^{
>+        finished = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+    finished = false;
>+
>+    [httpCookieStore.get() getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>+        EXPECT_EQ(2u, cookies.count);
>+        bool sessionCookieExists = false, otherSessionCookieExists = false;
>+        for (NSHTTPCookie* cookie in cookies) {
>+            if (areCookiesEqual(cookie, sessionCookie.get()))
>+                sessionCookieExists = true;
>+            else if (areCookiesEqual(cookie, sessionCookieDifferentValue.get()))
>+                otherSessionCookieExists = true;
>+        }
>+        EXPECT_TRUE(sessionCookieExists && otherSessionCookieExists);
>+        finished = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 198635: 371543 | 371554 | 371606 | 371641 | 371643 | 371749