Bug 175201 – [Cache API] Add Cache and CacheStorage IDL definitions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

Add IDLS for Cache and CacheStorage as per https://www.w3.org/TR/service-workers-1/#idl-index

[youenn fablet]

Created attachment 317267 [details]
Patch

[youenn fablet]

Created attachment 317269 [details]
Patch

[Build Bot]

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


ERROR: Source/WebCore/Modules/cache/Cache.h:55:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 72 files


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

[Build Bot]

Comment on attachment 317269 [details]
Patch

Attachment 317269 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4254827

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-add.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-delete.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage-keys.https.html
js/dom/global-constructors-attributes-dedicated-worker.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/stub-4.6.2-cache.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-matchAll.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-keys.https.html
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage.https.html
imported/w3c/web-platform-tests/service-workers/stub-4.6.3-cache-storage.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-put.https.html

[Build Bot]

Created attachment 317284 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 317269 [details]
Patch

Attachment 317269 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4254844

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-add.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-put.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-add.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-matchAll.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-delete.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage-keys.https.html
js/dom/global-constructors-attributes-dedicated-worker.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-keys.https.html
imported/w3c/web-platform-tests/service-workers/stub-4.6.2-cache.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-matchAll.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-put.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-keys.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage.https.html
imported/w3c/web-platform-tests/service-workers/stub-4.6.3-cache-storage.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-delete.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-storage-match.https.html

[Build Bot]

Created attachment 317288 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[youenn fablet]

Created attachment 317295 [details]
Patch

[Build Bot]

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


ERROR: Source/WebCore/Modules/cache/Cache.h:55:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 75 files


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

[Build Bot]

Comment on attachment 317295 [details]
Patch

Attachment 317295 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4255622

New failing tests:
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 317306 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 317295 [details]
Patch

Attachment 317295 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4255643

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-storage-match.https.html

[Build Bot]

Created attachment 317307 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 317295 [details]
Patch

Attachment 317295 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4255651

New failing tests:
js/dom/global-constructors-attributes-dedicated-worker.html

[Build Bot]

Created attachment 317309 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[youenn fablet]

Created attachment 317311 [details]
Patch

[Build Bot]

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


ERROR: Source/WebCore/Modules/cache/Cache.h:55:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 76 files


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

[youenn fablet]

Created attachment 317320 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #18)
> Created attachment 317320 [details]
> Patch

Marking cache-storage/window tests as flaky as unhandled rejected promises seem to cause issues.

[Build Bot]

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


ERROR: Source/WebCore/Modules/cache/Cache.h:55:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 76 files


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

[Sam Weinig]

Comment on attachment 317320 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        * runtime/CommonIdentifiers.h:

Why is this needed?

> Source/WebCore/ChangeLog:11
> +        It is off for DRT but on for WTR.

Why is it off for DRT?

[youenn fablet]

> > Source/JavaScriptCore/ChangeLog:8
> > +        * runtime/CommonIdentifiers.h:
> 
> Why is this needed?

These symbols are used by binding generated code (runtime enabled feature).

> > Source/WebCore/ChangeLog:11
> > +        It is off for DRT but on for WTR.
> 
> Why is it off for DRT?

Main target for this feature is WK2.

WK1 is lower priority and implementing it in WK1 will not be free.
Most of the implementation will either happen in the NetworkProcess or in the StorageProcess, relying on WK2 classes like NetworkCacheStorage et al.

[Brady Eidson]

Comment on attachment 317320 [details]
Patch

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

> Source/WebCore/Modules/cache/Cache.h:52
> +    void match(RequestInfo&&, std::optional<CacheQueryOptions>&&, Ref<DeferredPromise>&&);
> +    void matchAll(std::optional<RequestInfo>&&, std::optional<CacheQueryOptions>&&, MatchAllPromise&&);
> +    void add(RequestInfo&&, DOMPromiseDeferred<void>&&);
> +
> +    void addAll(Vector<RequestInfo>&&, DOMPromiseDeferred<void>&&);
> +    void put(RequestInfo&&, Ref<FetchResponse>&&, DOMPromiseDeferred<void>&&);
> +    void remove(RequestInfo&&, std::optional<CacheQueryOptions>&&, DOMPromiseDeferred<IDLBoolean>&&);
> +    void keys(std::optional<RequestInfo>&&, std::optional<CacheQueryOptions>&&, KeysPromise&&);

Why does one of these take a Ref<DefferedPromise>&& but the others take a DOMPromiseDeferred<>&&?

In my (very recent) experience with IDLs and promise related code, I think the code generator exclusively spat out Ref<DefferedPromise>&&

[youenn fablet]

> Why does one of these take a Ref<DefferedPromise>&& but the others take a
> DOMPromiseDeferred<>&&?
> 
> In my (very recent) experience with IDLs and promise related code, I think
> the code generator exclusively spat out Ref<DefferedPromise>&&

DeferredPromise is a generic Promise that can be resolved with any value, Promise<any> typically.

DOMPromiseDeferred<T> is a wrapper around DeferredPromise and can only resolve with a given value of type T, Promise<Cache> for instance.

[WebKit Commit Bot]

Comment on attachment 317320 [details]
Patch

Clearing flags on attachment: 317320

Committed r220311: <http://trac.webkit.org/changeset/220311>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33738001>

[Brady Eidson]

This caused layout test failures on the bots.

e.g. https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r220318%20(3404)/results.html

Tried to rollout by webkitbot isn't responding.

[youenn fablet]

(In reply to Brady Eidson from comment #28)
> This caused layout test failures on the bots.
> 
> e.g.
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/
> r220318%20(3404)/results.html
> 
> Tried to rollout by webkitbot isn't responding.

The service worker tests might need a rebase after bug 175215 landed.
I'll fix their expectations.

[youenn fablet]

Reopening to attach new patch.

[youenn fablet]

Created attachment 317343 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #31)
> Created attachment 317343 [details]
> Patch

Fixing expectations.

[WebKit Commit Bot]

Comment on attachment 317343 [details]
Patch

Clearing flags on attachment: 317343

Committed r220319: <http://trac.webkit.org/changeset/220319>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brady Eidson]

(In reply to youenn fablet from comment #32)
> 
> Fixing expectations.

You and me both, brother.

[Matt Lewis]

Talked to youenn about failing test js/dom/global-constructors-attributes.html on El Capitan WK2. Needed rebaseline:
https://trac.webkit.org/changeset/220374/webkit