Bug 190271 - Add a switch for Web SQL
Summary: Add a switch for Web SQL
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: 2018-10-03 18:58 PDT by Sihui Liu
Modified: 2018-10-16 17:00 PDT (History)
11 users (show)

See Also:


Attachments
Patch (10.25 KB, patch)
2018-10-03 19:03 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (9.70 KB, patch)
2018-10-15 17:00 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.64 MB, application/zip)
2018-10-15 18:54 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.92 MB, application/zip)
2018-10-15 19:39 PDT, EWS Watchlist
no flags Details
Patch (10.73 KB, patch)
2018-10-16 09:28 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 2018-10-03 18:58:36 PDT
This could help check whether websites are broken without Web SQL.
Comment 1 Sihui Liu 2018-10-03 19:03:49 PDT
Created attachment 351575 [details]
Patch
Comment 2 Chris Dumez 2018-10-04 09:06:47 PDT
Comment on attachment 351575 [details]
Patch

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

> Source/WebKit/Shared/WebPreferences.yaml:1364
> +  webcoreBinding: RuntimeEnabledFeatures

Don't we need a category? What's the default category if you do not specify one?

> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:332
> +WK_EXPORT bool WKPreferencesGetWebSQLEnabled(WKPreferencesRef preferencesRef);

Do we really need this SPI?
Comment 3 Sihui Liu 2018-10-15 17:00:53 PDT
Created attachment 352406 [details]
Patch
Comment 4 Sihui Liu 2018-10-15 17:04:18 PDT
(In reply to Chris Dumez from comment #2)
> Comment on attachment 351575 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=351575&action=review
> 
> > Source/WebKit/Shared/WebPreferences.yaml:1364
> > +  webcoreBinding: RuntimeEnabledFeatures
> 
> Don't we need a category? What's the default category if you do not specify
> one?
> 
It is said default category means displaying in nowhere. I changed it to experimental.

> > Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:332
> > +WK_EXPORT bool WKPreferencesGetWebSQLEnabled(WKPreferencesRef preferencesRef);
> 
> Do we really need this SPI?
Yes. We are going to toggle this setting, making "Disable Web SQL" an experimental feature.

Uploaded a new patch with a more appropriate variable name: disableWebSQLEnabled.
Comment 5 EWS Watchlist 2018-10-15 18:54:50 PDT
Comment on attachment 352406 [details]
Patch

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

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2018-10-15 18:54:52 PDT
Created attachment 352421 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-10-15 19:39:28 PDT
Comment on attachment 352406 [details]
Patch

Attachment 352406 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9605026

Number of test failures exceeded the failure limit.
Comment 8 EWS Watchlist 2018-10-15 19:39:30 PDT
Created attachment 352424 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 9 Ryosuke Niwa 2018-10-15 21:29:10 PDT
Appears to be failing to build on MacOS?

[50093/50123] storage/websql/empty-statement.html failed unexpectedly (test timed out, text diff)
[50094/50123] storage/websql/execute-sql-args.html failed unexpectedly (test timed out, text diff)
[50095/50123] storage/websql/execute-sql-rowsAffected.html failed unexpectedly (text diff)
[50096/50123] storage/websql/executesql-accepts-only-one-statement.html failed unexpectedly (test timed out, text diff)
[50097/50123] storage/websql/hash-change-with-xhr.html failed unexpectedly (test timed out, text diff)
[50098/50123] storage/websql/multiple-transactions-on-different-handles.html failed unexpectedly (test timed out, text diff)
[50099/50123] storage/websql/multiple-transactions.html failed unexpectedly (test timed out, text diff)
[50100/50123] storage/websql/null-callbacks.html failed unexpectedly (text diff)
[50101/50123] storage/websql/open-database-creation-callback-isolated-world.html failed unexpectedly (test timed out, text diff)

This might be because WTR tries to enable all experimental features...
Comment 10 Ryosuke Niwa 2018-10-15 21:33:30 PDT
Comment on attachment 352406 [details]
Patch

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

> Source/WebKit/Shared/WebPreferences.yaml:1297
> +DisabledWebSQLEnabled:

This is a very strangely named preference. Can we just call it WebSQLDisabled instead?

> Source/WebKit/Shared/WebPreferences.yaml:1303
> +  category: experimental

Does it really make sense to expose it as an experimental feature??
Comment 11 Ryosuke Niwa 2018-10-15 21:33:56 PDT
Comment on attachment 352406 [details]
Patch

r- because tests are failing in WK2.
Comment 12 Sihui Liu 2018-10-16 08:46:17 PDT
(In reply to Ryosuke Niwa from comment #10)
> Comment on attachment 352406 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=352406&action=review
> 
> > Source/WebKit/Shared/WebPreferences.yaml:1297
> > +DisabledWebSQLEnabled:
> 
> This is a very strangely named preference. Can we just call it
> WebSQLDisabled instead?
> 
Okay.

> > Source/WebKit/Shared/WebPreferences.yaml:1303
> > +  category: experimental
> 
> Does it really make sense to expose it as an experimental feature??
Making it experimental so we could expose "Disable WebSQL" to the web developer, letting them check if their pages broken under the feature. Is there another way to achieve this?
Comment 13 Sihui Liu 2018-10-16 09:28:40 PDT
Created attachment 352461 [details]
Patch
Comment 14 WebKit Commit Bot 2018-10-16 16:59:46 PDT
Comment on attachment 352461 [details]
Patch

Clearing flags on attachment: 352461

Committed r237210: <https://trac.webkit.org/changeset/237210>
Comment 15 WebKit Commit Bot 2018-10-16 16:59:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Radar WebKit Bug Importer 2018-10-16 17:00:56 PDT
<rdar://problem/45323549>