Bug 147646 – [Win] Layout Test http/tests/security/contentSecurityPolicy/source-list-parsing-10.html is failing

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Brent Fulgham]

The following layout test is failing on Windows:

http/tests/security/contentSecurityPolicy/source-list-parsing-10.html

Probable cause:

Possibly related to my CSP_NEXT changes. I'm reviewing now.

[Brent Fulgham]

Skipped failing test in r187879 <http://trac.webkit.org/changeset/187879>.

[Radar WebKit Bug Importer]

<rdar://problem/26522796>

[Per Arne Vollan]

Created attachment 282751 [details]
Patch

[Per Arne Vollan]

(In reply to comment #3)
> Created attachment 282751 [details]
> Patch

Another option is to add Windows specific test results, since Windows has enabled CSP_NEXT.

[Brent Fulgham]

Comment on attachment 282751 [details]
Patch

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

Could you please generate a Windows result with CSP_NEXT active instead of turning off the feature?

> ChangeLog:8
> +        Disable CSP_NEXT. We can then use the common expected test results for this test. 

We should leave it activated, and provide a test result. We want CSP_NEXT available on Windows, just like it is on Mac/iOS.

> Source/cmake/OptionsWin.cmake:13
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSP_NEXT PUBLIC OFF)

I don't like this change. We should keep CSP_NEXT activated and just provide a test result so we can confirm it functions properly.

[Per Arne Vollan]

(In reply to comment #5)
> Comment on attachment 282751 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282751&action=review
> 
> Could you please generate a Windows result with CSP_NEXT active instead of
> turning off the feature?
> 
> > ChangeLog:8
> > +        Disable CSP_NEXT. We can then use the common expected test results for this test. 
> 
> We should leave it activated, and provide a test result. We want CSP_NEXT
> available on Windows, just like it is on Mac/iOS.
> 
> > Source/cmake/OptionsWin.cmake:13
> > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSP_NEXT PUBLIC OFF)
> 
> I don't like this change. We should keep CSP_NEXT activated and just provide
> a test result so we can confirm it functions properly.

Thanks for reviewing!

I'll add Windows test results instead :)

I have probably got this all wrong, but it seems to me that CSP_NEXT is disabled on Mac? The Mac test expectations also seem to indicate this.

[Brent Fulgham]

(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 282751 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=282751&action=review
> > 
> > Could you please generate a Windows result with CSP_NEXT active instead of
> > turning off the feature?
> > 
> > > ChangeLog:8
> > > +        Disable CSP_NEXT. We can then use the common expected test results for this test. 
> > 
> > We should leave it activated, and provide a test result. We want CSP_NEXT
> > available on Windows, just like it is on Mac/iOS.
> > 
> > > Source/cmake/OptionsWin.cmake:13
> > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSP_NEXT PUBLIC OFF)
> > 
> > I don't like this change. We should keep CSP_NEXT activated and just provide
> > a test result so we can confirm it functions properly.
> 
> Thanks for reviewing!
> 
> I'll add Windows test results instead :)
> 
> I have probably got this all wrong, but it seems to me that CSP_NEXT is
> disabled on Mac? The Mac test expectations also seem to indicate this.

No! I'm wrong. I just looked at the Mac build configuration and CSP_NEXT is definitely turned off.

I was confused because Dan has been moving things out of CSP_NEXT as we've been adding support for CSP 2.0, and I thought for some reason that CSP_NEXT was turned on as well.

Go ahead and deactivate CSP_NEXT on Windows.

[Per Arne Vollan]

Thanks, Brent!

[Per Arne Vollan]

Committed r202848: <https://trac.webkit.org/changeset/202848>