Bug 210344 – http/tests/in-app-browser-privacy/app-bound-domain.html is a constant failure on iOS

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Kate Cheney]

The in-app-browser-privacy tests try to enable in-app browser privacy as an internal debug flag, but it is not an internal debug feature.

[Kate Cheney]

<rdar://problem/61583925>

[Kate Cheney]

Created attachment 396101 [details]
Patch

[Alex Christensen]

Comment on attachment 396101 [details]
Patch

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

> Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:127
> +#if PLATFORM(IOS_FAMILY)

Why is this not needed on Mac?

[Kate Cheney]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 396101 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396101&action=review
> 
> > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:127
> > +#if PLATFORM(IOS_FAMILY)
> 
> Why is this not needed on Mac?

This is an iOS feature only, so this option should only be set if the tests are running iOS (they are skipped on other platforms).

[Alex Christensen]

Comment on attachment 396101 [details]
Patch

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

> Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:131
> +        [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];

Two thoughts:
1. TestController::platformAddTestOptions is a place where we are supposed to be writing to options, and this reads from options.  I think TestController::platformResetPreferencesToConsistentValues or TestController::platformResetStateToConsistentValues  and maybe TestController::updatePlatformSpecificTestOptionsForTest would be a better place to reset and set these values in NSUserDefaults.
2. Instead of setting the bool to NO, let's remove object for key.

[Kate Cheney]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 396101 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396101&action=review
> 
> > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:131
> > +        [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
> 
> Two thoughts:
> 1. TestController::platformAddTestOptions is a place where we are supposed
> to be writing to options, and this reads from options.  I think
> TestController::platformResetPreferencesToConsistentValues or
> TestController::platformResetStateToConsistentValues  and maybe
> TestController::updatePlatformSpecificTestOptionsForTest would be a better
> place to reset and set these values in NSUserDefaults.
> 2. Instead of setting the bool to NO, let's remove object for key.

I don't think this will work because updatePlatformSpecificTestOptionsForTest() is called before updateTestOptionsFromTestHeader() in TestController::testOptionsForTest(), so it doesn't have the updated value. Then I think platformResetStateToConsistentValues is called at the end of the test, so I can remove the key there but not set it prior to the test.

[Kate Cheney]

Created attachment 396313 [details]
Patch

[Kate Cheney]

(In reply to katherine_cheney from comment #6)
> (In reply to Alex Christensen from comment #5)
> > Comment on attachment 396101 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=396101&action=review
> > 
> > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:131
> > > +        [[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"WebKitDebugIsInAppBrowserPrivacyEnabled"];
> > 
> > Two thoughts:
> > 1. TestController::platformAddTestOptions is a place where we are supposed
> > to be writing to options, and this reads from options.  I think
> > TestController::platformResetPreferencesToConsistentValues or
> > TestController::platformResetStateToConsistentValues  and maybe
> > TestController::updatePlatformSpecificTestOptionsForTest would be a better
> > place to reset and set these values in NSUserDefaults.
> > 2. Instead of setting the bool to NO, let's remove object for key.
> 
> I don't think this will work because
> updatePlatformSpecificTestOptionsForTest() is called before
> updateTestOptionsFromTestHeader() in TestController::testOptionsForTest(),
> so it doesn't have the updated value. Then I think
> platformResetStateToConsistentValues is called at the end of the test, so I
> can remove the key there but not set it prior to the test.

Maybe I'm missing something? Because it seems like all internal/experimental features are set before the header comments are read, and those seem to work fine. But this test still fails if I call it before updateTestOptionsFromTestHeader().

[Brent Fulgham]

r=me

[EWS]

Committed r260040: <https://trac.webkit.org/changeset/260040>

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