Bug 195011 - [WPE][GTK] Deprecate WebSQL APIs
Summary: [WPE][GTK] Deprecate WebSQL APIs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-25 12:06 PST by Michael Catanzaro
Modified: 2019-06-12 09:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch (16.19 KB, patch)
2019-02-25 12:07 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (17.92 KB, patch)
2019-06-12 07:32 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2019-02-25 12:06:17 PST
WebSQL is disabled at runtime since 2.24 and we don't have any way in our API to enable it. I don't think we want to add a way to do so, not unless some application actually needs it, which seems quite unlikely. So deprecate stuff.

Note: this is for 2.24
Comment 1 Michael Catanzaro 2019-02-25 12:07:30 PST
Created attachment 362916 [details]
Patch
Comment 2 EWS Watchlist 2019-02-25 12:09:51 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 EWS Watchlist 2019-02-25 12:10:07 PST
Attachment 362916 [details] did not pass style-queue:


WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitWebsiteDataManager.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitWebsiteDataManager.h"
ERROR: Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:135:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Michael Catanzaro 2019-05-06 19:03:21 PDT
Ping for review.
Comment 5 Adrian Perez 2019-05-07 01:31:00 PDT
According to https://caniuse.com/#feat=sql-storage the feature is
available still in Safari... I wonder if it might cause trouble due
to websites deciding to use WebSQL after doing UA sniffing and finding
that “looks like Safari” 🤔

On the other hand, Firefox does not have WebSQL, and I see that there
has been no work on the spec since 2010 so it seems like it should be
safe (ish?) to completely disable it.

Other than the commentary above, patch LGTM.
Comment 6 Michael Catanzaro 2019-05-07 07:22:05 PDT
I think caniuse.com is outdated.

This patch doesn't disable WebSQL anyway. It only updates our API docs to reflect that WebSQL is already disabled since 2.24 and cannot be enabled. Currently our API docs are wrong.
Comment 7 Michael Catanzaro 2019-06-06 06:24:58 PDT
Ping reviewers. This is broken in 2.24.
Comment 8 Carlos Garcia Campos 2019-06-12 05:01:55 PDT
Comment on attachment 362916 [details]
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:323
> +     * Deprecated: 2.24. WebSQL is no longer supported. Use IndexedDB instead.

2.26 now

> Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h:43
> + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).

WebSQL databases. Deprecated 2.26

> Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h:43
> - * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases.
> + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:156
> +ALLOW_DEPRECATED_DECLARATIONS_BEGIN
>      GUniquePtr<char> webSQLDirectory(g_build_filename(Test::dataDirectory(), "websql", nullptr));
>      g_assert_cmpstr(webSQLDirectory.get(), ==, webkit_website_data_manager_get_websql_directory(test->m_manager));
>      test->runJavaScriptAndWaitUntilFinished("db = openDatabase(\"TestDatabase\", \"1.0\", \"TestDatabase\", 1);", nullptr);
>      g_assert_true(g_file_test(webSQLDirectory.get(), G_FILE_TEST_IS_DIR));
> +ALLOW_DEPRECATED_DECLARATIONS_END

Let's remove the tests, since it does nothing in the end.
Comment 9 Carlos Garcia Campos 2019-06-12 05:03:31 PDT
hmm, since it's not only deprecated but disabled, we might consider to add a g_warning() to let the user know that WebSQL is not going to work anyway. And document that as well. Just saying it's deprecated sounds like it still works.
Comment 10 Michael Catanzaro 2019-06-12 07:25:43 PDT
(In reply to Carlos Garcia Campos from comment #8)
> Comment on attachment 362916 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362916&action=review
> 
> > Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:323
> > +     * Deprecated: 2.24. WebSQL is no longer supported. Use IndexedDB instead.
> 
> 2.26 now

No, it's really for 2.24, late or not. Remember WebSQL is not available at all since 2.24.

> > Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h:43
> > + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).
> 
> WebSQL databases. Deprecated 2.26
> 
> > Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h:43
> > - * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases.
> > + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).
> 
> Ditto.

OK (2.24)

> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:156
> > +ALLOW_DEPRECATED_DECLARATIONS_BEGIN
> >      GUniquePtr<char> webSQLDirectory(g_build_filename(Test::dataDirectory(), "websql", nullptr));
> >      g_assert_cmpstr(webSQLDirectory.get(), ==, webkit_website_data_manager_get_websql_directory(test->m_manager));
> >      test->runJavaScriptAndWaitUntilFinished("db = openDatabase(\"TestDatabase\", \"1.0\", \"TestDatabase\", 1);", nullptr);
> >      g_assert_true(g_file_test(webSQLDirectory.get(), G_FILE_TEST_IS_DIR));
> > +ALLOW_DEPRECATED_DECLARATIONS_END
> 
> Let's remove the tests, since it does nothing in the end.

OK
Comment 11 Michael Catanzaro 2019-06-12 07:32:45 PDT
Created attachment 371954 [details]
Patch
Comment 12 Michael Catanzaro 2019-06-12 07:33:32 PDT
(cq? to see if you're OK with the 2.24 deprecations. I know it's odd to use these retroactively, but it's weird to say it's deprecated in 2.26 as that might trick people into thinking it's available in 2.24.)
Comment 13 Michael Catanzaro 2019-06-12 07:34:08 PDT
(In reply to Michael Catanzaro from comment #12)
> (cq? to see if you're OK with the 2.24 deprecations. I know it's odd to use
> these retroactively, but it's weird to say it's deprecated in 2.26 as that
> might trick people into thinking it's available in 2.24.)

BTW I do intend this for 2.24 branch as well.
Comment 14 WebKit Commit Bot 2019-06-12 09:07:48 PDT
Comment on attachment 371954 [details]
Patch

Clearing flags on attachment: 371954

Committed r246353: <https://trac.webkit.org/changeset/246353>
Comment 15 WebKit Commit Bot 2019-06-12 09:07:50 PDT
All reviewed patches have been landed.  Closing bug.