Bug 175201

Summary: [Cache API] Add Cache and CacheStorage IDL definitions
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, buildbot, cdumez, commit-queue, jlewis3, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=174541
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews103 for mac-elcapitan
none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews115 for mac-elcapitan
none
Patch
none
Patch
none
Patch none

Description youenn fablet 2017-08-04 12:04:03 PDT
Add IDLS for Cache and CacheStorage as per https://www.w3.org/TR/service-workers-1/#idl-index
Comment 1 youenn fablet 2017-08-04 12:34:12 PDT
Created attachment 317267 [details]
Patch
Comment 2 youenn fablet 2017-08-04 12:44:25 PDT
Created attachment 317269 [details]
Patch
Comment 3 Build Bot 2017-08-04 12:47:14 PDT
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.
Comment 4 Build Bot 2017-08-04 14:08:48 PDT
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
Comment 5 Build Bot 2017-08-04 14:08:49 PDT
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
Comment 6 Build Bot 2017-08-04 14:29:48 PDT
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
Comment 7 Build Bot 2017-08-04 14:29:49 PDT
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
Comment 8 youenn fablet 2017-08-04 14:51:37 PDT
Created attachment 317295 [details]
Patch
Comment 9 Build Bot 2017-08-04 14:55:19 PDT
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.
Comment 10 Build Bot 2017-08-04 16:15:10 PDT
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
Comment 11 Build Bot 2017-08-04 16:15:11 PDT
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
Comment 12 Build Bot 2017-08-04 16:23:21 PDT
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
Comment 13 Build Bot 2017-08-04 16:23:23 PDT
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
Comment 14 Build Bot 2017-08-04 16:41:53 PDT
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
Comment 15 Build Bot 2017-08-04 16:41:54 PDT
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
Comment 16 youenn fablet 2017-08-04 16:47:38 PDT
Created attachment 317311 [details]
Patch
Comment 17 Build Bot 2017-08-04 16:51:05 PDT
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.
Comment 18 youenn fablet 2017-08-04 17:52:23 PDT
Created attachment 317320 [details]
Patch
Comment 19 youenn fablet 2017-08-04 17:53:18 PDT
(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.
Comment 20 Build Bot 2017-08-04 17:55:45 PDT
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.
Comment 21 Sam Weinig 2017-08-04 18:01:19 PDT
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?
Comment 22 youenn fablet 2017-08-04 18:17:19 PDT
> > 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.
Comment 23 Brady Eidson 2017-08-04 20:43:46 PDT
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>&&
Comment 24 youenn fablet 2017-08-04 21:35:53 PDT
> 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.
Comment 25 WebKit Commit Bot 2017-08-04 22:06:18 PDT
Comment on attachment 317320 [details]
Patch

Clearing flags on attachment: 317320

Committed r220311: <http://trac.webkit.org/changeset/220311>
Comment 26 WebKit Commit Bot 2017-08-04 22:06:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 27 Radar WebKit Bug Importer 2017-08-04 22:06:44 PDT
<rdar://problem/33738001>
Comment 28 Brady Eidson 2017-08-05 09:25:05 PDT
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.
Comment 29 youenn fablet 2017-08-05 09:29:02 PDT
(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.
Comment 30 youenn fablet 2017-08-05 11:14:05 PDT
Reopening to attach new patch.
Comment 31 youenn fablet 2017-08-05 11:14:06 PDT
Created attachment 317343 [details]
Patch
Comment 32 youenn fablet 2017-08-05 11:14:26 PDT
(In reply to youenn fablet from comment #31)
> Created attachment 317343 [details]
> Patch

Fixing expectations.
Comment 33 WebKit Commit Bot 2017-08-05 11:59:21 PDT
Comment on attachment 317343 [details]
Patch

Clearing flags on attachment: 317343

Committed r220319: <http://trac.webkit.org/changeset/220319>
Comment 34 WebKit Commit Bot 2017-08-05 11:59:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 35 Brady Eidson 2017-08-05 20:07:52 PDT
(In reply to youenn fablet from comment #32)
> 
> Fixing expectations.

You and me both, brother.
Comment 36 Matt Lewis 2017-08-07 17:55:05 PDT
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