Web Inspector: Cookies view should use model objects instead of raw payload data

https://bugs.webkit.org/show_bug.cgi?id=189533
<rdar://problem/44364183>

Reviewed by Joseph Pecoraro and Devin Rousso.

Source/WebInspectorUI:

* UserInterface/Models/Cookie.js:
(WI.Cookie):
(WI.Cookie.fromPayload):
(WI.Cookie.parseSetCookieResponseHeader):
(WI.Cookie.prototype.get type):
(WI.Cookie.prototype.get name):
(WI.Cookie.prototype.get value):
(WI.Cookie.prototype.get header):
(WI.Cookie.prototype.get expires):
(WI.Cookie.prototype.get maxAge):
(WI.Cookie.prototype.get path):
(WI.Cookie.prototype.get domain):
(WI.Cookie.prototype.get secure):
(WI.Cookie.prototype.get httpOnly):
(WI.Cookie.prototype.get sameSite):
(WI.Cookie.prototype.get size):
(WI.Cookie.prototype.get url):
(WI.Cookie.prototype.expirationDate):
Cleanup Cookie object; add pubic getters for data, `url` property,
static `fromPayload` method, and calculate `_size` if missing.

* UserInterface/Views/CookieStorageContentView.js:
(WI.CookieStorageContentView.prototype.tableDidRemoveRows):
(WI.CookieStorageContentView.prototype._reloadCookies):
Create Cookie objects from the payload instead of using raw payload data.

LayoutTests:

* inspector/unit-tests/cookie-expected.txt:
* inspector/unit-tests/cookie.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index b50c9eb..8642cbf 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2018-12-14  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Cookies view should use model objects instead of raw payload data
+        https://bugs.webkit.org/show_bug.cgi?id=189533
+        <rdar://problem/44364183>
+
+        Reviewed by Joseph Pecoraro and Devin Rousso.
+
+        * inspector/unit-tests/cookie-expected.txt:
+        * inspector/unit-tests/cookie.html:
+
 2018-12-14  Simon Fraser  <simon.fraser@apple.com>
 
         REGRESSION (r233268): contents of an animated element inside overflow:hidden disappear

diff --git a/LayoutTests/inspector/unit-tests/cookie-expected.txt b/LayoutTests/inspector/unit-tests/cookie-expected.txt
index e4dd1ca..681f7d1 100644 (file)
--- a/LayoutTests/inspector/unit-tests/cookie-expected.txt
+++ b/LayoutTests/inspector/unit-tests/cookie-expected.txt
@@ -44,7 +44,7 @@ PASS: cookie.value should be 'gamma'.
 PASS: Empty header should produce null.
 HEADER: Set-Cookie: name=value
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -57,7 +57,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; path=/foo
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -70,7 +70,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; domain=example.com
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -83,7 +83,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; secure
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -96,7 +96,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; Secure
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -109,7 +109,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; HttpOnly
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -122,7 +122,7 @@ PASS: cookie.httpOnly should be 'true'.
 
 HEADER: Set-Cookie: name=value; expires=Fri 06-Oct-2017 03:20:27 GMT; Max-Age=3600
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -135,7 +135,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=value; expires=Fri 06-Oct-2017 03:43:47 GMT; Max-Age=5000; path=/foo; domain=example.com; secure; HttpOnly
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -149,7 +149,7 @@ PASS: cookie.httpOnly should be 'true'.
 HEADER: Set-Cookie: name=value; Unknown; path=/one/two
 WARN: Unknown Cookie attribute: Unknown
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -163,7 +163,7 @@ PASS: cookie.httpOnly should be 'false'.
 HEADER: Set-Cookie: name=value; Unknown=Ignored; path=/one/two
 WARN: Unknown Cookie attribute: Unknown=Ignored
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'value'.
@@ -176,7 +176,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name=somewhat longer value than normal with spaces, and commas; domain=other.example.com
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be 'somewhat longer value than normal with spaces, and commas'.
@@ -189,7 +189,7 @@ PASS: cookie.httpOnly should be 'false'.
 
 HEADER: Set-Cookie: name==value=;Domain=.example.com;Expires=Wed, 04-Apr-2018 03:34:02 GMT
 PASS: Value should be a WI.Cookie.
-PASS: cookie.rawHeader should be the original header text.
+PASS: cookie.header should be the original header text.
 PASS: cookie.type should be WI.Cookie.Type.Response.
 PASS: cookie.name should be 'name'.
 PASS: cookie.value should be '=value='.
@@ -201,3 +201,8 @@ PASS: cookie.secure should be 'false'.
 PASS: cookie.httpOnly should be 'false'.
 
 
+-- Running test case: WI.Cookie.url
+PASS: Cookie url should include domain.
+PASS: Cookie url should include domain and path.
+PASS: Secure cookie url should begin with 'https'.
+

diff --git a/LayoutTests/inspector/unit-tests/cookie.html b/LayoutTests/inspector/unit-tests/cookie.html
index f01e5cc..d0d0423 100644 (file)
--- a/LayoutTests/inspector/unit-tests/cookie.html
+++ b/LayoutTests/inspector/unit-tests/cookie.html
@@ -55,7 +55,7 @@ function test()
                 InspectorTest.log(`HEADER: Set-Cookie: ${header}`);
                 let cookie = WI.Cookie.parseSetCookieResponseHeader(header);
                 InspectorTest.expectThat(cookie instanceof WI.Cookie, `Value should be a WI.Cookie.`);
-                InspectorTest.expectEqual(cookie.rawHeader, header, `cookie.rawHeader should be the original header text.`);
+                InspectorTest.expectEqual(cookie.header, header, `cookie.header should be the original header text.`);
                 InspectorTest.expectEqual(cookie.type, WI.Cookie.Type.Response, `cookie.type should be WI.Cookie.Type.Response.`);
                 InspectorTest.expectEqual(cookie.name, expected.name, `cookie.name should be '${expected.name}'.`);
                 InspectorTest.expectEqual(cookie.value, expected.value, `cookie.value should be '${expected.value}'.`);
@@ -210,6 +210,23 @@ function test()
         }
     });
 
+    suite.addTestCase({
+        name: "WI.Cookie.url",
+        description: "Cookie url property.",
+        test() {
+            let cookieWithDomain = WI.Cookie.parseSetCookieResponseHeader(`name=value; domain=example.com`);
+            InspectorTest.expectEqual(cookieWithDomain.url, "http://example.com", "Cookie url should include domain.");
+
+            let cookieWithDomainAndPath = WI.Cookie.parseSetCookieResponseHeader(`name=value; domain=example.com; path=/foo`);
+            InspectorTest.expectEqual(cookieWithDomainAndPath.url, "http://example.com/foo", "Cookie url should include domain and path.");
+
+            let secureCookie = WI.Cookie.parseSetCookieResponseHeader(`name=value; domain=example.com; secure`);
+            InspectorTest.expectEqual(secureCookie.url, "https://example.com", "Secure cookie url should begin with 'https'.");
+
+            return true;
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>

diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
index 84b773c..0a2ae0d 100644 (file)
--- a/Source/WebInspectorUI/ChangeLog
+++ b/Source/WebInspectorUI/ChangeLog
@@ -1,3 +1,37 @@
+2018-12-14  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Cookies view should use model objects instead of raw payload data
+        https://bugs.webkit.org/show_bug.cgi?id=189533
+        <rdar://problem/44364183>
+
+        Reviewed by Joseph Pecoraro and Devin Rousso.
+
+        * UserInterface/Models/Cookie.js:
+        (WI.Cookie):
+        (WI.Cookie.fromPayload):
+        (WI.Cookie.parseSetCookieResponseHeader):
+        (WI.Cookie.prototype.get type):
+        (WI.Cookie.prototype.get name):
+        (WI.Cookie.prototype.get value):
+        (WI.Cookie.prototype.get header):
+        (WI.Cookie.prototype.get expires):
+        (WI.Cookie.prototype.get maxAge):
+        (WI.Cookie.prototype.get path):
+        (WI.Cookie.prototype.get domain):
+        (WI.Cookie.prototype.get secure):
+        (WI.Cookie.prototype.get httpOnly):
+        (WI.Cookie.prototype.get sameSite):
+        (WI.Cookie.prototype.get size):
+        (WI.Cookie.prototype.get url):
+        (WI.Cookie.prototype.expirationDate):
+        Cleanup Cookie object; add pubic getters for data, `url` property,
+        static `fromPayload` method, and calculate `_size` if missing.
+
+        * UserInterface/Views/CookieStorageContentView.js:
+        (WI.CookieStorageContentView.prototype.tableDidRemoveRows):
+        (WI.CookieStorageContentView.prototype._reloadCookies):
+        Create Cookie objects from the payload instead of using raw payload data.
+
 2018-12-13  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: remove DOM.BackendNodeId and associated commands/events

diff --git a/Source/WebInspectorUI/UserInterface/Models/Cookie.js b/Source/WebInspectorUI/UserInterface/Models/Cookie.js
index 4672942..04bb22e 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Models/Cookie.js
+++ b/Source/WebInspectorUI/UserInterface/Models/Cookie.js
@@ -25,12 +25,12 @@
 
 WI.Cookie = class Cookie
 {
-    constructor(type, name, value, raw, expires, maxAge, path, domain, secure, httpOnly, sameSite)
+    constructor(type, name, value, {header, expires, maxAge, path, domain, secure, httpOnly, sameSite, size} = {})
     {
         console.assert(Object.values(WI.Cookie.Type).includes(type));
         console.assert(typeof name === "string");
         console.assert(typeof value === "string");
-        console.assert(!raw || typeof raw === "string");
+        console.assert(!header || typeof header === "string");
         console.assert(!expires || expires instanceof Date);
         console.assert(!maxAge || typeof maxAge === "number");
         console.assert(!path || typeof path === "string");
@@ -38,37 +38,35 @@ WI.Cookie = class Cookie
         console.assert(!secure || typeof secure === "boolean");
         console.assert(!httpOnly || typeof httpOnly === "boolean");
         console.assert(!sameSite || Object.values(WI.Cookie.SameSiteType).includes(sameSite));
+        console.assert(!size || typeof size === "number");
 
-        this.type = type;
-        this.name = name || "";
-        this.value = value || "";
-
-        if (this.type === WI.Cookie.Type.Response) {
-            this.rawHeader = raw || "";
-            this.expires = expires || null;
-            this.maxAge = maxAge || null;
-            this.path = path || null;
-            this.domain = domain || null;
-            this.secure = secure || false;
-            this.httpOnly = httpOnly || false;
-            this.sameSite = sameSite || WI.Cookie.SameSiteType.None;
+        this._type = type;
+        this._name = name;
+        this._value = value;
+        this._size = size || this._name.length + this._value.length;
+
+        if (this._type === WI.Cookie.Type.Response) {
+            this._header = header || "";
+            this._expires = expires || null;
+            this._maxAge = maxAge || null;
+            this._path = path || null;
+            this._domain = domain || null;
+            this._secure = secure || false;
+            this._httpOnly = httpOnly || false;
+            this._sameSite = sameSite || WI.Cookie.SameSiteType.None;
         }
     }
 
-    // Public
+    // Static
 
-    expirationDate(requestSentDate)
+    static fromPayload(payload)
     {
-        if (this.maxAge) {
-            let startDate = requestSentDate || new Date;
-            return new Date(startDate.getTime() + (this.maxAge * 1000));
-        }
+        let {name, value, ...options} = payload;
+        options.expires = options.expires ? new Date(options.expires) : null;
 
-        return this.expires;
+        return new WI.Cookie(WI.Cookie.Type.Response, name, value, options);
     }
 
-    // Static
-
     // RFC 6265 defines the HTTP Cookie and Set-Cookie header fields:
     // https://www.ietf.org/rfc/rfc6265.txt
 
@@ -212,7 +210,40 @@ WI.Cookie = class Cookie
             }
         }
 
-        return new WI.Cookie(WI.Cookie.Type.Response, name, value, header, expires, maxAge, path, domain, secure, httpOnly, sameSite);
+        return new WI.Cookie(WI.Cookie.Type.Response, name, value, {header, expires, maxAge, path, domain, secure, httpOnly, sameSite});
+    }
+
+    // Public
+
+    get type() { return this._type; }
+    get name() { return this._name; }
+    get value() { return this._value; }
+    get header() { return this._header; }
+    get expires() { return this._expires; }
+    get maxAge() { return this._maxAge; }
+    get path() { return this._path; }
+    get domain() { return this._domain; }
+    get secure() { return this._secure; }
+    get httpOnly() { return this._httpOnly; }
+    get sameSite() { return this._sameSite; }
+    get size() { return this._size; }
+
+    get url()
+    {
+        let url = this._secure ? "https://" : "http://";
+        url += this._domain || "";
+        url += this._path || "";
+        return url;
+    }
+
+    expirationDate(requestSentDate)
+    {
+        if (this._maxAge) {
+            let startDate = requestSentDate || new Date;
+            return new Date(startDate.getTime() + (this._maxAge * 1000));
+        }
+
+        return this._expires;
     }
 };
 

diff --git a/Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js b/Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js
index d0c2b43..3aa2d9e 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js
+++ b/Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js
@@ -132,10 +132,7 @@ WI.CookieStorageContentView = class CookieStorageContentView extends WI.ContentV
 
             this._cookies.splice(rowIndex, 1);
 
-            // FIXME: <https://bugs.webkit.org/b/189533> add a WI.Cookie.url property
-            // once we switch over to using model objects instead of raw payload data.
-            let cookieURL = (cookie.secure ? "https://" : "http://") + cookie.domain + cookie.path;
-            PageAgent.deleteCookie(cookie.name, cookieURL);
+            PageAgent.deleteCookie(cookie.name, cookie.url);
         }
     }
 
@@ -312,7 +309,7 @@ WI.CookieStorageContentView = class CookieStorageContentView extends WI.ContentV
     _reloadCookies()
     {
         PageAgent.getCookies().then((payload) => {
-            this._cookies = this._filterCookies(payload.cookies);
+            this._cookies = this._filterCookies(payload.cookies.map(WI.Cookie.fromPayload));
             this._updateSort();
             this._table.reloadData();
         }).catch((error) => {

diff --git a/Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js b/Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js
index d1a7e09..3ca8415 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js
+++ b/Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js
@@ -356,7 +356,7 @@ WI.ResourceHeadersContentView = class ResourceHeadersContentView extends WI.Cont
                 let responseCookies = this._resource.responseCookies;
                 console.assert(responseCookies.length > 0);
                 for (let cookie of responseCookies)
-                    this._responseHeadersSection.appendKeyValuePair(key, cookie.rawHeader, "header");
+                    this._responseHeadersSection.appendKeyValuePair(key, cookie.header, "header");
                 continue;
             }