Bug 181240 – Implement Cache API partitioning based on ClientOrigin

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

Currently, we are only using the client origin but we should also use the top origin.

[youenn fablet]

rdar://problem/36164145

[youenn fablet]

Created attachment 330381 [details]
Patch

[youenn fablet]

Created attachment 330396 [details]
Patch

[EWS Watchlist]

Attachment 330396 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:219:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 330398 [details]
Patch

[EWS Watchlist]

Attachment 330398 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:187:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:223:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 330398 [details]
Patch

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

> Source/WebCore/page/ClientOrigin.h:75
> +    std::optional<SecurityOriginData> topOrigin, clientOrigin;

We usually have each declaration on its own line.

[youenn fablet]

Created attachment 330451 [details]
Patch for landing

[EWS Watchlist]

Attachment 330451 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:187:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:223:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 330451 [details]
Patch for landing

Clearing flags on attachment: 330451

Committed r226401: <https://trac.webkit.org/changeset/226401>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Matt Lewis]

This caused the test http/tests/cache-storage/cache-clearing-origin.https.html to time out consistently on all WK2 platforms. 

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fcache-storage%2Fcache-clearing-origin.https.html

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r226448%20(2057)/results.html

Diff:

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/http/tests/cache-storage/cache-clearing-origin.https-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/http/tests/cache-storage/cache-clearing-origin.https-actual.txt
@@ -1,5 +1,6 @@
+CONSOLE MESSAGE: line 2451: Unhandled Promise Rejection: Error: assert_equals: client origin of cache 1 expected "https://localhost:8443" but got "https://127.0.0.1:8443"
+#PID UNRESPONSIVE - com.apple.WebKit.WebContent.Development (pid 52826)
+FAIL: Timed out waiting for notifyDone to be called
 
-
-PASS Create a cache storage from localhost and clear it 
-PASS Clearing disk cache of a given origin 
-
+#EOF
+#EOF

[youenn fablet]

Will look at it over the weekend. Could you add an expectation in the meantime?

[Matt Lewis]

This is causing the trees to be very red. Rather than setting an expectation, I would rather roll back the change until it can be landed without issue.

[Matt Lewis]

Reverted r226401 for reason:

This caused timeouts on multiple platforms.

Committed r226453: <https://trac.webkit.org/changeset/226453>

[youenn fablet]

(In reply to Matt Lewis from comment #15)
> Reverted r226401 for reason:
> 
> This caused timeouts on multiple platforms.
> 
> Committed r226453: <https://trac.webkit.org/changeset/226453>

Reason was the modified test was not always deterministic, leading to the timeout.
Sorting the cache representation in the test fixes the timeout.
I will reland with the updated test.

[youenn fablet]

Created attachment 330596 [details]
Updated test

[EWS Watchlist]

Attachment 330596 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:187:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/Modules/cache/DOMCacheStorage.cpp:223:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 330596 [details]
Updated test

Clearing flags on attachment: 330596

Committed r226481: <https://trac.webkit.org/changeset/226481>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.