Bug 206681 - REGRESSION: [iOS] http/wpt/cache-storage/quota-third-party.https.html is flaky failing.
Summary: REGRESSION: [iOS] http/wpt/cache-storage/quota-third-party.https.html is flak...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
: 206857 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-01-23 11:00 PST by Jason Lawrence
Modified: 2020-01-28 14:35 PST (History)
7 users (show)

See Also:


Attachments
Update Test Expectations (1.35 KB, patch)
2020-01-27 15:18 PST, Jason Lawrence
no flags Details | Formatted Diff | Diff
Patch (4.05 KB, patch)
2020-01-28 09:02 PST, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch for landing (2.76 KB, patch)
2020-01-28 10:08 PST, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Lawrence 2020-01-23 11:00:42 PST
http/wpt/cache-storage/quota-third-party.https.html

Description: 
This test is flaky failing on iOS.

History:
https://results.webkit.org/?limit=5000&suite=layout-tests&test=http%2Fwpt%2Fcache-storage%2Fquota-third-party.https.html

Diff:
--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/http/wpt/cache-storage/quota-third-party.https-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/http/wpt/cache-storage/quota-third-party.https-actual.txt
@@ -1,8 +1,8 @@
-CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
-CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
+CONSOLE MESSAGE: Cache API operation failed: Failed writing data to the file system
+CONSOLE MESSAGE: Cache API operation failed: Internal error
   
 
 PASS same origin iframe has regular quota 
-PASS cross origin iframe has reduced quota 
+FAIL cross origin iframe has reduced quota assert_equals: expected "FAIL" but got "UNEXPECTED"
 PASS cross origin iframe has reduced quota after resetting quota
Comment 1 Radar WebKit Bug Importer 2020-01-23 11:38:16 PST
<rdar://problem/58842429>
Comment 2 Ryan Haddad 2020-01-23 13:33:33 PST
This appears to be a recent regression, starting on or around r254397.
Comment 3 Jason Lawrence 2020-01-27 15:18:55 PST
Created attachment 388925 [details]
Update Test Expectations
Comment 4 Truitt Savell 2020-01-27 15:20:54 PST
Comment on attachment 388925 [details]
Update Test Expectations

Clearing flags on attachment: 388925

Committed r255185: <https://trac.webkit.org/changeset/255185>
Comment 5 Kate Cheney 2020-01-28 08:13:12 PST
*** Bug 206857 has been marked as a duplicate of this bug. ***
Comment 6 Kate Cheney 2020-01-28 09:02:32 PST
Created attachment 389012 [details]
Patch
Comment 7 youenn fablet 2020-01-28 09:30:01 PST
Comment on attachment 389012 [details]
Patch

r=me but I think we should consider turning off the behavior that removes "websitedata store" for iframes.

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

> LayoutTests/ChangeLog:9
> +        the origin's cache was deleted before the test finished.

I wonder whether we should turn off this behavior by default for tests and enable it for specific tests that actually require that behavior.
My fear is that other tests might end up have similar issues and we will have difficulties understanding why, or fixing them if WPT.

> LayoutTests/http/wpt/cache-storage/quota-third-party.https.html:13
> +    testRunner.setStatisticsHasHadUserInteraction("https://127.0.0.1:9443", true, start());

Should it be start instead of start()?
Or can we not pass any function at all and not have any start function?
Comment 8 Kate Cheney 2020-01-28 10:04:45 PST
(In reply to youenn fablet from comment #7)
> Comment on attachment 389012 [details]
> Patch
> 
> r=me but I think we should consider turning off the behavior that removes
> "websitedata store" for iframes.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389012&action=review
> 
> > LayoutTests/ChangeLog:9
> > +        the origin's cache was deleted before the test finished.
> 
> I wonder whether we should turn off this behavior by default for tests and
> enable it for specific tests that actually require that behavior.
> My fear is that other tests might end up have similar issues and we will
> have difficulties understanding why, or fixing them if WPT.
> 

I agree. Processing ITP domains at random times has caused flaky tests in the past, so I'm wondering if there is an even more general solution that might fix more flakiness than just caches. I'll look into it!

> > LayoutTests/http/wpt/cache-storage/quota-third-party.https.html:13
> > +    testRunner.setStatisticsHasHadUserInteraction("https://127.0.0.1:9443", true, start());
> 
> Should it be start instead of start()?
> Or can we not pass any function at all and not have any start function?

You're right, the start function is not needed. Patch for landing will remove it.
Comment 9 Kate Cheney 2020-01-28 10:08:20 PST
Created attachment 389024 [details]
Patch for landing
Comment 10 Kate Cheney 2020-01-28 10:17:58 PST
(In reply to katherine_cheney from comment #8)
> (In reply to youenn fablet from comment #7)
> > Comment on attachment 389012 [details]
> > Patch
> > 
> > r=me but I think we should consider turning off the behavior that removes
> > "websitedata store" for iframes.
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=389012&action=review
> > 
> > > LayoutTests/ChangeLog:9
> > > +        the origin's cache was deleted before the test finished.
> > 
> > I wonder whether we should turn off this behavior by default for tests and
> > enable it for specific tests that actually require that behavior.
> > My fear is that other tests might end up have similar issues and we will
> > have difficulties understanding why, or fixing them if WPT.
> > 
> 
> I agree. Processing ITP domains at random times has caused flaky tests in
> the past, so I'm wondering if there is an even more general solution that
> might fix more flakiness than just caches. I'll look into it!
> 

On the other hand, though, if we are trying to test actual behavior and persistence of website data (i.e. caches, cookies, etc), maybe it could lead to ineffective testing to keep ITP's handling of these off by default in tests, because it won't mimic real situations. ITP's behavior probably shouldn't just be on for ITP tests, because it affects so many other areas of WebKit.

> > > LayoutTests/http/wpt/cache-storage/quota-third-party.https.html:13
> > > +    testRunner.setStatisticsHasHadUserInteraction("https://127.0.0.1:9443", true, start());
> > 
> > Should it be start instead of start()?
> > Or can we not pass any function at all and not have any start function?
> 
> You're right, the start function is not needed. Patch for landing will
> remove it.
Comment 11 WebKit Commit Bot 2020-01-28 11:06:01 PST
The commit-queue encountered the following flaky tests while processing attachment 389024 [details]:

editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org)
The commit-queue is continuing to process your patch.
Comment 12 WebKit Commit Bot 2020-01-28 11:06:40 PST
Comment on attachment 389024 [details]
Patch for landing

Clearing flags on attachment: 389024

Committed r255263: <https://trac.webkit.org/changeset/255263>
Comment 13 WebKit Commit Bot 2020-01-28 11:06:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 youenn fablet 2020-01-28 11:24:36 PST
> > I agree. Processing ITP domains at random times has caused flaky tests in
> > the past, so I'm wondering if there is an even more general solution that
> > might fix more flakiness than just caches. I'll look into it!

Did these issues show up some bugs in implementation or in tests mainly?

> On the other hand, though, if we are trying to test actual behavior and
> persistence of website data (i.e. caches, cookies, etc), maybe it could lead
> to ineffective testing to keep ITP's handling of these off by default in
> tests, because it won't mimic real situations. ITP's behavior probably
> shouldn't just be on for ITP tests, because it affects so many other areas
> of WebKit.

Layout tests are almost run without any history. For instance, WebsiteDataStore data is removed before each test.
This is very far from a typical situation where ITP would kick in and do useful processing.
I might be wrong but I do not think we get a lot of value there from ITP testing functionality. Maybe we get some threading issues from these tests but wound't be the IPT specific testing be sufficient for that as well.
Comment 15 Kate Cheney 2020-01-28 14:35:30 PST
(In reply to youenn fablet from comment #14)
> > > I agree. Processing ITP domains at random times has caused flaky tests in
> > > the past, so I'm wondering if there is an even more general solution that
> > > might fix more flakiness than just caches. I'll look into it!
> 
> Did these issues show up some bugs in implementation or in tests mainly?
> 

Only tests (see https://bugs.webkit.org/show_bug.cgi?id=197285 for example). Processing is pretty much always good in implementation.

> > On the other hand, though, if we are trying to test actual behavior and
> > persistence of website data (i.e. caches, cookies, etc), maybe it could lead
> > to ineffective testing to keep ITP's handling of these off by default in
> > tests, because it won't mimic real situations. ITP's behavior probably
> > shouldn't just be on for ITP tests, because it affects so many other areas
> > of WebKit.
> 
> Layout tests are almost run without any history. For instance,
> WebsiteDataStore data is removed before each test.
> This is very far from a typical situation where ITP would kick in and do
> useful processing.
> I might be wrong but I do not think we get a lot of value there from ITP
> testing functionality. Maybe we get some threading issues from these tests
> but wound't be the IPT specific testing be sufficient for that as well.

I see what you're saying, and I agree it could help prevent hours spent looking into a flaky test like I had to do. I'll file a new radar for this. Thanks for your feedback Youenn!