Bug 211896

Summary: Regression(r254856) Family Health iOS app is broken due to lack for WebSQL support
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: New BugsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, benjamin, cmarcelo, ews-watchlist, ggaren, mjs, sihui_liu, 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=210889
https://bugs.webkit.org/show_bug.cgi?id=211906
Bug Depends on:    
Bug Blocks: 204907    
Attachments:
Description Flags
Patch none

Description Chris Dumez 2020-05-14 08:07:42 PDT
Family Health iOS app is broken due to lack for WebSQL support.
Comment 1 Chris Dumez 2020-05-14 08:08:21 PDT
<rdar://problem/63025045>
Comment 2 Chris Dumez 2020-05-14 08:34:33 PDT
Created attachment 399361 [details]
Patch
Comment 3 Chris Dumez 2020-05-14 08:36:08 PDT
Comment on attachment 399361 [details]
Patch

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

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;

Note that personally, I think we should use:
bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;

without any application checks. Please let me know if you agree / disagree.
Comment 4 Maciej Stachowiak 2020-05-14 08:42:08 PDT
Comment on attachment 399361 [details]
Patch

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

>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
>> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> 
> Note that personally, I think we should use:
> bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> 
> without any application checks. Please let me know if you agree / disagree.

Lack of application check creates a risk we'll have to keep rolling the OS version forward into the future, which makes it harder to ever delete WebSQL. So I think limiting it to specific apps is probably better. Though perhaps if there's enough apps, we could factor it out into a function so we don't end up with an unwieldy or expression.
Comment 5 Maciej Stachowiak 2020-05-14 08:46:12 PDT
Comment on attachment 399361 [details]
Patch

Do we have a way to test this type of change, via API tests for instance?

Please add a test if possible.

Otherwise, r=me
Comment 6 Chris Dumez 2020-05-14 08:47:17 PDT
(In reply to Maciej Stachowiak from comment #5)
> Comment on attachment 399361 [details]
> Patch
> 
> Do we have a way to test this type of change, via API tests for instance?
> 
> Please add a test if possible.
> 
> Otherwise, r=me

As far as I know, we still have test coverage for WebSQL being enabled.
Comment 7 Chris Dumez 2020-05-14 08:52:53 PDT
(In reply to Maciej Stachowiak from comment #4)
> Comment on attachment 399361 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399361&action=review
> 
> >> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
> >> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> > 
> > Note that personally, I think we should use:
> > bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> > 
> > without any application checks. Please let me know if you agree / disagree.
> 
> Lack of application check creates a risk we'll have to keep rolling the OS
> version forward into the future, which makes it harder to ever delete
> WebSQL. So I think limiting it to specific apps is probably better. Though
> perhaps if there's enough apps, we could factor it out into a function so we
> don't end up with an unwieldy or expression.

Why would we keep rolling the IOS version forward? We drop support for WebSQL in a specific SDK, and then apps have to update if they want to build with the new SDK.

Checking only for specific apps is risky because we may not notice quickly all the apps that are broken without WebSQL or the breakage may be more subtle for certain apps. Also, it is quite a waste of engineering time to investigate app breakage and bisect it to this particular WebSQL change. I wasted a day on this and don't wish this on anybody else.
Comment 8 EWS 2020-05-14 09:11:30 PDT
Committed r261695: <https://trac.webkit.org/changeset/261695>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399361 [details].