Bug 205753 - The "ITP Database Backend" flag is not working correctly
Summary: The "ITP Database Backend" flag is not working correctly
Status: RESOLVED DUPLICATE of bug 205844
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-03 15:37 PST by Kate Cheney
Modified: 2020-01-13 12:59 PST (History)
4 users (show)

See Also:

Attachments
Patch (5.93 KB, patch)
2020-01-03 16:49 PST, Kate Cheney 		dino: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kate Cheney 2020-01-03 15:37:36 PST 
Since being turned on-by-default, the ITP Database flag has stopped working correctly.
Comment 1 Radar WebKit Bug Importer 2020-01-03 15:38:58 PST 
<rdar://problem/58309011>
Comment 2 Kate Cheney 2020-01-03 16:49:34 PST 
Created attachment 386730 [details]
Patch
Comment 3 Dean Jackson 2020-01-05 14:40:34 PST 
Comment on attachment 386730 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:402
> +    if (!isITPDatabaseEnabledValue)
> +        parameters.shouldEnableITPDatabase = m_defaultPageGroup->preferences().isITPDatabaseEnabled();
> +    else
> +        parameters.shouldEnableITPDatabase = isITPDatabaseEnabledValue.boolValue;

I think this conditional should be reversed. i.e. if the value from user prefs exists, use it, otherwise go to the page group defaults.
Comment 4 Kate Cheney 2020-01-06 08:41:17 PST 
(In reply to Dean Jackson from comment #3)
> Comment on attachment 386730 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=386730&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:402
> > +    if (!isITPDatabaseEnabledValue)
> > +        parameters.shouldEnableITPDatabase = m_defaultPageGroup->preferences().isITPDatabaseEnabled();
> > +    else
> > +        parameters.shouldEnableITPDatabase = isITPDatabaseEnabledValue.boolValue;
> 
> I think this conditional should be reversed. i.e. if the value from user
> prefs exists, use it, otherwise go to the page group defaults.

I agree, that order is more readable. I'll change it before landing. 

These api test failures also look concerning, I'll investigate.
Comment 5 Kate Cheney 2020-01-13 12:58:50 PST 
this was fixed (along with the failing api tests) in https://bugs.webkit.org/show_bug.cgi?id=205844
Comment 6 Kate Cheney 2020-01-13 12:59:19 PST 

*** This bug has been marked as a duplicate of bug 205844 ***