Bug 196099 - Fix key path extraction code in IndexedDB to check own property
Summary: Fix key path extraction code in IndexedDB to check own property
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: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-21 11:55 PDT by Sihui Liu
Modified: 2019-03-21 17:11 PDT (History)
7 users (show)

See Also:


Attachments
Patch (27.72 KB, patch)
2019-03-21 12:06 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (2.46 MB, application/zip)
2019-03-21 13:08 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews115 for mac-highsierra (2.75 MB, application/zip)
2019-03-21 14:15 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews202 for win-future (12.94 MB, application/zip)
2019-03-21 14:54 PDT, EWS Watchlist
no flags Details
Patch (26.84 KB, patch)
2019-03-21 15:02 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-highsierra (2.45 MB, application/zip)
2019-03-21 16:05 PDT, EWS Watchlist
no flags Details
Patch for landing (29.13 KB, patch)
2019-03-21 16:23 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2019-03-21 11:55:17 PDT
imported/w3c/web-platform-tests/IndexedDB/clone-before-keypath-eval.html, imported/w3c/web-platform-tests/IndexedDB/keygenerator-inject.html, imported/w3c/web-platform-tests/IndexedDB/wasm-module-value.html
Comment 1 Sihui Liu 2019-03-21 12:06:27 PDT
Created attachment 365585 [details]
Patch
Comment 2 EWS Watchlist 2019-03-21 13:08:50 PDT
Comment on attachment 365585 [details]
Patch

Attachment 365585 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11603001

New failing tests:
storage/indexeddb/structured-clone-private.html
storage/indexeddb/structured-clone.html
Comment 3 EWS Watchlist 2019-03-21 13:08:51 PDT
Created attachment 365601 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 4 EWS Watchlist 2019-03-21 14:15:22 PDT
Comment on attachment 365585 [details]
Patch

Attachment 365585 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11603603

New failing tests:
storage/indexeddb/structured-clone-private.html
storage/indexeddb/structured-clone.html
Comment 5 EWS Watchlist 2019-03-21 14:15:24 PDT
Created attachment 365617 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 EWS Watchlist 2019-03-21 14:53:52 PDT
Comment on attachment 365585 [details]
Patch

Attachment 365585 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11604279

New failing tests:
storage/indexeddb/structured-clone-private.html
Comment 7 EWS Watchlist 2019-03-21 14:54:04 PDT
Created attachment 365620 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 8 Sihui Liu 2019-03-21 15:02:24 PDT
Created attachment 365621 [details]
Patch
Comment 9 Ryosuke Niwa 2019-03-21 15:29:29 PDT
Comment on attachment 365621 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Fix three IDB WPT tests

Let's update the bug title as I renamed. This doesn't tell us what we're doing at all.
You can mention that you're fixing a few other bugs as well.

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:70
> +    if (obj->inherits<JSArray>(vm) && (keyPathElement == "length")) {

I think we need to use isArray in ArrayConstructor.h instead.
No parenthesis is needed around keyPathElement ==.

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:71
> +        result = jsNumber(asArray(object)->length());

And then run [[GET]] here.

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:75
> +        result = obj->get(&exec, identifier);

I don't think we should be running getter here.
To something like jsBlob.wrapped()->size()

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:79
> +        result = obj->get(&exec, identifier);

Ditto.
Comment 10 EWS Watchlist 2019-03-21 16:05:04 PDT
Comment on attachment 365621 [details]
Patch

Attachment 365621 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11605387

New failing tests:
imported/w3c/web-platform-tests/IndexedDB/wasm-module-value.html
Comment 11 EWS Watchlist 2019-03-21 16:05:06 PDT
Created attachment 365634 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 12 Sihui Liu 2019-03-21 16:23:35 PDT
Created attachment 365637 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2019-03-21 17:03:37 PDT
Comment on attachment 365637 [details]
Patch for landing

Clearing flags on attachment: 365637

Committed r243348: <https://trac.webkit.org/changeset/243348>
Comment 14 WebKit Commit Bot 2019-03-21 17:03:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Ryosuke Niwa 2019-03-21 17:05:27 PDT
Comment on attachment 365637 [details]
Patch for landing

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

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:76
> +            result = jsNumber(jsCast<JSBlob*>(obj)->wrapped().size());

I think it would have been better to have a local variable like this to share the code between two code paths:
Blob& blob = jsCast<JSBlob*>(obj)->wrapped()

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:85
> +        if (keyPathElement == "name") {

Ditto.
Comment 16 Radar WebKit Bug Importer 2019-03-21 17:11:24 PDT
<rdar://problem/49132044>