Bug 184315 - Add a test for scoped cookies used to load AppCache resources
Summary: Add a test for scoped cookies used to load AppCache resources
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-04 14:29 PDT by youenn fablet
Modified: 2018-04-05 23:48 PDT (History)
4 users (show)

See Also:

Attachments
Patch (5.55 KB, patch)
2018-04-04 14:30 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (6.15 KB, patch)
2018-04-05 21:10 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Fixing flakiness (6.29 KB, patch)
2018-04-05 21:56 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2018-04-04 14:29:31 PDT 
Add a test for scoped cookies used to load AppCache resources
Comment 1 youenn fablet 2018-04-04 14:30:44 PDT 
Created attachment 337227 [details]
Patch
Comment 2 Ryosuke Niwa 2018-04-04 15:45:20 PDT 
Comment on attachment 337227 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=337227&action=review

> LayoutTests/http/tests/appcache/document-cookie-http-only.php:-2
> -setcookie("foo", "bar", 0, "/", null, null, true);

Don't we want to keep the test for regular cookies?
Comment 3 youenn fablet 2018-04-04 16:02:47 PDT 
(In reply to Ryosuke Niwa from comment #2)
> Comment on attachment 337227 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=337227&action=review
> 
> > LayoutTests/http/tests/appcache/document-cookie-http-only.php:-2
> > -setcookie("foo", "bar", 0, "/", null, null, true);
> 
> Don't we want to keep the test for regular cookies?

The test is moving from using one HttpOnly cookie to two HttpOnly cookies.
So we are not losing coverage here.

We have other tests that are covering non HttpOnly cookies.

It would be easy to modify this test to cache another script protected by a regular non-scoped cookie. Is that your suggestion?
Comment 4 Ryosuke Niwa 2018-04-04 18:57:55 PDT 
(In reply to youenn fablet from comment #3)
> (In reply to Ryosuke Niwa from comment #2)
> > Comment on attachment 337227 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=337227&action=review
> > 
> > > LayoutTests/http/tests/appcache/document-cookie-http-only.php:-2
> > > -setcookie("foo", "bar", 0, "/", null, null, true);
> > 
> > Don't we want to keep the test for regular cookies?
> 
> The test is moving from using one HttpOnly cookie to two HttpOnly cookies.
> So we are not losing coverage here.
> 
> We have other tests that are covering non HttpOnly cookies.
> 
> It would be easy to modify this test to cache another script protected by a
> regular non-scoped cookie. Is that your suggestion?

Yes, that's my suggestion.
Comment 5 youenn fablet 2018-04-05 21:10:08 PDT 
Created attachment 337342 [details]
Patch
Comment 6 youenn fablet 2018-04-05 21:56:40 PDT 
Created attachment 337345 [details]
Fixing flakiness
Comment 7 WebKit Commit Bot 2018-04-05 23:47:36 PDT 
Comment on attachment 337345 [details]
Fixing flakiness

Clearing flags on attachment: 337345

Committed r230328: <https://trac.webkit.org/changeset/230328>
Comment 8 WebKit Commit Bot 2018-04-05 23:47:37 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-04-05 23:48:23 PDT 
<rdar://problem/39231738>