Attachment 340428 Details for Bug 185654 – Patch

[patch] Patch

bug-185654-20180515124455.patch (text/plain), 11.68 KB, created by Daniel Bates on 2018-05-15 12:44:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-05-15 12:44:56 PDT

Size: 11.68 KB

patch

obsolete

>Subversion Revision: 231403
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 79adbb4771321ecbb21a96d75c25299641dc026c..3b53ec6a709300ebaba1c1009eff11a80ac511c8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-05-15  Daniel Bates  <dabates@apple.com>
>+
>+        Cleanup platform Cookie
>+        https://bugs.webkit.org/show_bug.cgi?id=185654
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Remove unnecessary constructor and use =default for default constructor.
>+        Simplify Cookie::decode() by having it decode directly into a stack-allocated
>+        cookie. Remove some unnecessary local variables. Fix style nits.
>+
>+        * platform/Cookie.h:
>+        (WebCore::Cookie::Cookie):
>+        (WebCore::Cookie::isNull const):
>+        (WebCore::Cookie::encode const):
>+        (WebCore::Cookie::decode):
>+        * platform/network/cf/CookieJarCFNet.cpp:
>+        (WebCore::getRawCookies):
>+        * platform/network/cocoa/CookieCocoa.mm:
>+        (WebCore::cookieCreated):
>+        (WebCore::Cookie::Cookie):
>+        (WebCore::Cookie::operator== const):
>+        (WebCore::Cookie::hash const):
>+        * platform/network/soup/CookieJarSoup.cpp:
>+        (WebCore::getRawCookies):
>+
> 2018-05-09  Daniel Bates  <dabates@apple.com>
> 
>         REGRESSION (r231479): com.apple.WebCore crash in WebCore::DocumentLoader::stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied
>diff --git a/Source/WebCore/platform/Cookie.h b/Source/WebCore/platform/Cookie.h
>index 1aa664ff88454184508368e5341c663b88fda0ba..53e30e107ca785d2291ec2f79f5733182e16374a 100644
>--- a/Source/WebCore/platform/Cookie.h
>+++ b/Source/WebCore/platform/Cookie.h
>@@ -37,25 +37,9 @@
> namespace WebCore {
> 
> struct Cookie {
>-    Cookie() { }
>-    
>+    Cookie() = default;
>     Cookie(WTF::HashTableDeletedValueType)
>         : name(WTF::HashTableDeletedValue)
>-    { }
>-
>-    Cookie(const String& name, const String& value, const String& domain, const String& path, double created, double expires, bool httpOnly, bool secure, bool session, const String& comment, const URL& commentURL, const Vector<uint16_t> ports)
>-        : name(name)
>-        , value(value)
>-        , domain(domain)
>-        , path(path)
>-        , created(created)
>-        , expires(expires)
>-        , httpOnly(httpOnly)
>-        , secure(secure)
>-        , session(session)
>-        , comment(comment)
>-        , commentURL(commentURL)
>-        , ports(ports)
>     {
>     }
> 
>@@ -75,17 +59,8 @@ struct Cookie {
> 
>     bool isNull() const
>     {
>-        return name.isNull()
>-        && value.isNull()
>-        && domain.isNull()
>-        && path.isNull()
>-        && created == 0
>-        && expires == 0
>-        && !httpOnly
>-        && !secure
>-        && !session
>-        && comment.isNull()
>-        && commentURL.isNull();
>+        return name.isNull() && value.isNull() && domain.isNull() && path.isNull() && !created
>+            && !expires && !httpOnly && !secure && !session && comment.isNull() && commentURL.isNull();
>     }
>     
>     String name;
>@@ -119,72 +94,49 @@ struct CookieHash {
> template<class Encoder>
> void Cookie::encode(Encoder& encoder) const
> {
>-    encoder << name << value << domain << path << created << expires << httpOnly << secure << session << comment << commentURL << ports;
>+    encoder << name;
>+    encoder << value;
>+    encoder << domain;
>+    encoder << path;
>+    encoder << created;
>+    encoder << expires;
>+    encoder << httpOnly;
>+    encoder << secure;
>+    encoder << session;
>+    encoder << comment;
>+    encoder << commentURL;
>+    encoder << ports;
> }
> 
> template<class Decoder>
> std::optional<Cookie> Cookie::decode(Decoder& decoder)
> {
>-    std::optional<String> name;
>-    decoder >> name;
>-    if (!name)
>+    Cookie cookie;
>+    if (!decoder.decode(cookie.name))
>         return std::nullopt;
>-    
>-    std::optional<String> value;
>-    decoder >> value;
>-    if (!value)
>+    if (!decoder.decode(cookie.value))
>         return std::nullopt;
>-
>-    std::optional<String> domain;
>-    decoder >> domain;
>-    if (!domain)
>+    if (!decoder.decode(cookie.domain))
>         return std::nullopt;
>-
>-    std::optional<String> path;
>-    decoder >> path;
>-    if (!path)
>+    if (!decoder.decode(cookie.path))
>         return std::nullopt;
>-
>-    std::optional<double> created;
>-    decoder >> created;
>-    if (!created)
>+    if (!decoder.decode(cookie.created))
>         return std::nullopt;
>-
>-    std::optional<double> expires;
>-    decoder >> expires;
>-    if (!expires)
>+    if (!decoder.decode(cookie.expires))
>         return std::nullopt;
>-
>-    std::optional<bool> httpOnly;
>-    decoder >> httpOnly;
>-    if (!httpOnly)
>+    if (!decoder.decode(cookie.httpOnly))
>         return std::nullopt;
>-
>-    std::optional<bool> secure;
>-    decoder >> secure;
>-    if (!secure)
>+    if (!decoder.decode(cookie.secure))
>         return std::nullopt;
>-
>-    std::optional<bool> session;
>-    decoder >> session;
>-    if (!session)
>+    if (!decoder.decode(cookie.session))
>         return std::nullopt;
>-
>-    std::optional<String> comment;
>-    decoder >> comment;
>-    if (!comment)
>+    if (!decoder.decode(cookie.comment))
>         return std::nullopt;
>-
>-    URL commentURL;
>-    if (!decoder.decode(commentURL))
>+    if (!decoder.decode(cookie.commentURL))
>         return std::nullopt;
>-
>-    std::optional<Vector<uint16_t>> ports;
>-    decoder >> ports;
>-    if (!ports)
>+    if (!decoder.decode(cookie.ports))
>         return std::nullopt;
>-
>-    return {{ WTFMove(*name), WTFMove(*value), WTFMove(*domain), WTFMove(*path), WTFMove(*created), WTFMove(*expires), WTFMove(*httpOnly), WTFMove(*secure), WTFMove(*session), WTFMove(*comment), WTFMove(commentURL), WTFMove(*ports) }};
>+    return cookie;
> }
> 
> }
>diff --git a/Source/WebCore/platform/network/cf/CookieJarCFNet.cpp b/Source/WebCore/platform/network/cf/CookieJarCFNet.cpp
>index 00bf0bd31a4487eff6cece66d5db3e2f7bdc53fe..1e0c74a27ac068f75a9f3310ff74f967090354c3 100644
>--- a/Source/WebCore/platform/network/cf/CookieJarCFNet.cpp
>+++ b/Source/WebCore/platform/network/cf/CookieJarCFNet.cpp
>@@ -257,24 +257,18 @@ bool getRawCookies(const NetworkStorageSession& session, const URL& firstParty,
>     rawCookies.reserveCapacity(count);
> 
>     for (CFIndex i = 0; i < count; i++) {
>-        CFHTTPCookieRef cookie = checked_cf_cast<CFHTTPCookieRef>(CFArrayGetValueAtIndex(cookiesCF.get(), i));
>-        String name = cookieName(cookie).get();
>-        String value = cookieValue(cookie).get();
>-        String domain = cookieDomain(cookie).get();
>-        String path = cookiePath(cookie).get();
>-
>-        double created = cookieCreatedTime(cookie);
>-        double expires = cookieExpirationTime(cookie);
>-
>-        bool httpOnly = CFHTTPCookieIsHTTPOnly(cookie);
>-        bool secure = CFHTTPCookieIsSecure(cookie);
>-        bool session = false; // FIXME: Need API for if a cookie is a session cookie.
>-
>-        String comment;
>-        URL commentURL;
>-        Vector<uint16_t> ports;
>-
>-        rawCookies.uncheckedAppend(Cookie(name, value, domain, path, created, expires, httpOnly, secure, session, comment, commentURL, ports));
>+        CFHTTPCookieRef cfCookie = checked_cf_cast<CFHTTPCookieRef>(CFArrayGetValueAtIndex(cookiesCF.get(), i));
>+        Cookie cookie;
>+        cookie.name = cookieName(cfCookie).get();
>+        cookie.value = cookieValue(cfCookie).get();
>+        cookie.domain = cookieDomain(cfCookie).get();
>+        cookie.path = cookiePath(cfCookie).get();
>+        cookie.created = cookieCreatedTime(cfCookie);
>+        cookie.expires = cookieExpirationTime(cfCookie);
>+        cookie.httpOnly = CFHTTPCookieIsHTTPOnly(cfCookie);
>+        cookie.secure = CFHTTPCookieIsSecure(cfCookie);
>+        cookie.session = false; // FIXME: Need API for if a cookie is a session cookie.
>+        rawCookies.uncheckedAppend(WTFMove(cookie));
>     }
> 
>     return true;
>diff --git a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>index 575579dbbbdc3a104f9f461a14cb8380fa9c6abd..9bc2caba0967babbf3021daa1bd82972330c65d6 100644
>--- a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2015 Apple, Inc.  All rights reserved.
>+ * Copyright (C) 2015-2018 Apple, Inc.  All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -68,12 +68,22 @@ static double cookieCreated(NSHTTPCookie *cookie)
>     if ([value isKindOfClass:[NSString class]])
>         return toCanonicalFormat(((NSString *)value).doubleValue);
> 
>-    return 0.0;
>+    return 0;
> }
> 
> Cookie::Cookie(NSHTTPCookie *cookie)
>-    : Cookie(cookie.name, cookie.value, cookie.domain, cookie.path, cookieCreated(cookie), [cookie.expiresDate timeIntervalSince1970] * 1000.0,
>-    cookie.HTTPOnly, cookie.secure, cookie.sessionOnly, cookie.comment, cookie.commentURL, portVectorFromList(cookie.portList))
>+    : name { cookie.name }
>+    , value { cookie.value }
>+    , domain { cookie.domain }
>+    , path { cookie.path }
>+    , created { cookieCreated(cookie) }
>+    , expires { [cookie.expiresDate timeIntervalSince1970] * 1000.0 }
>+    , httpOnly { static_cast<bool>(cookie.HTTPOnly) }
>+    , secure { static_cast<bool>(cookie.secure) }
>+    , session { static_cast<bool>(cookie.sessionOnly) }
>+    , comment { cookie.comment }
>+    , commentURL { cookie.commentURL }
>+    , ports { portVectorFromList(cookie.portList) }
> {
> }
> 
>@@ -132,17 +142,14 @@ bool Cookie::operator==(const Cookie& other) const
>     bool otherNull = other.isNull();
>     if (thisNull || otherNull)
>         return thisNull == otherNull;
>-    
>-    NSHTTPCookie *nsCookie(*this);
>-    return [nsCookie isEqual:other];
>+    return [static_cast<NSHTTPCookie *>(*this) isEqual:other];
> }
>     
> unsigned Cookie::hash() const
> {
>     ASSERT(!name.isHashTableDeletedValue());
>     ASSERT(!isNull());
>-    NSHTTPCookie *nsCookie(*this);
>-    return nsCookie.hash;
>+    return static_cast<NSHTTPCookie *>(*this).hash;
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/platform/network/soup/CookieJarSoup.cpp b/Source/WebCore/platform/network/soup/CookieJarSoup.cpp
>index deadfa4ffef10edc335e5b2f6768a1bb663d44fb..9c0912e6b49ab45302d5c8f97711700c1bd9e459 100644
>--- a/Source/WebCore/platform/network/soup/CookieJarSoup.cpp
>+++ b/Source/WebCore/platform/network/soup/CookieJarSoup.cpp
>@@ -159,11 +159,19 @@ bool getRawCookies(const NetworkStorageSession& session, const URL& firstParty,
>         return false;
> 
>     for (GSList* iter = cookies.get(); iter; iter = g_slist_next(iter)) {
>-        SoupCookie* cookie = static_cast<SoupCookie*>(iter->data);
>-        rawCookies.append(Cookie(String::fromUTF8(cookie->name), String::fromUTF8(cookie->value), String::fromUTF8(cookie->domain),
>-            String::fromUTF8(cookie->path), 0, cookie->expires ? static_cast<double>(soup_date_to_time_t(cookie->expires)) * 1000 : 0,
>-            cookie->http_only, cookie->secure, !cookie->expires, String(), URL(), Vector<uint16_t>{ }));
>-        soup_cookie_free(cookie);
>+        SoupCookie* soupCookie = static_cast<SoupCookie*>(iter->data);
>+        Cookie cookie;
>+        cookie.name = String::fromUTF8(soupCookie->name);
>+        cookie.value = String::fromUTF8(soupCookie->value);
>+        cookie.domain = String::fromUTF8(soupCookie->domain);
>+        cookie.path = String::fromUTF8(soupCookie->path);
>+        cookie.created = 0;
>+        cookie.expires = soupCookie->expires ? static_cast<double>(soup_date_to_time_t(soupCookie->expires)) * 1000 : 0;
>+        cookie.httpOnly = soupCookie->http_only;
>+        cookie.secure = soupCookie->secure;
>+        cookie.session = !soupCookie->expires;
>+        rawCookies.append(WTFMove(cookie));
>+        soup_cookie_free(soupCookie);
>     }
> 
>     return true;

Flags:

pvollan: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185654: 340425 | 340426 | 340428 | 340476