Bug 180562 – Turn on ENABLE_APPLICATION_MANIFEST on iOS

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[David Quesada]

.

[Radar WebKit Bug Importer]

<rdar://problem/35924737>

[David Quesada]

Created attachment 328777 [details]
Patch

Patch for EWS

[David Quesada]

Created attachment 328793 [details]
Patch for EWS, v2

Fixed a compile error, enabled relevant LayoutTests on ios.

[David Quesada]

Created attachment 328817 [details]
Patch v3

Fix the other build errors in CachedResourceRequest caused by files being unified-source-bundled differently and removing an implicit #include when the feature is enabled.

[Sam Weinig]

What makes this feature iOS only? In general, we would like everything to be enable on all supported platforms.

[Brady Eidson]

Comment on attachment 328817 [details]
Patch v3

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

> LayoutTests/platform/ios/TestExpectations:24
> +http/tests/security/contentSecurityPolicy/manifest-src-allowed.html [ Pass ]
> +http/tests/security/contentSecurityPolicy/manifest-src-blocked.html [ Pass ]
> +applicationmanifest/ [ Pass ]

Why explicit PASSes? Where are they otherwise skipped?

> Source/WebCore/loader/cache/CachedResourceRequest.cpp:39
> +#include "ServiceWorkerRegistrationData.h"

This can't be a necessary part of your patch

[David Quesada]

(In reply to Sam Weinig from comment #5)
> What makes this feature iOS only? In general, we would like everything to be
> enable on all supported platforms.

(In reply to Brady Eidson from comment #6)
> Comment on attachment 328817 [details]
> Patch v3
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328817&action=review
> 
> > LayoutTests/platform/ios/TestExpectations:24
> > +http/tests/security/contentSecurityPolicy/manifest-src-allowed.html [ Pass ]
> > +http/tests/security/contentSecurityPolicy/manifest-src-blocked.html [ Pass ]
> > +applicationmanifest/ [ Pass ]
> 
> Why explicit PASSes? Where are they otherwise skipped?

They're skipped in LayoutTests/TestExpectations, since the feature was disabled while I was landing all the pieces.

> 
> > Source/WebCore/loader/cache/CachedResourceRequest.cpp:39
> > +#include "ServiceWorkerRegistrationData.h"
> 
> This can't be a necessary part of your patch

Do you mean I can't include this line in this patch, or you don't see why this line is needed? Before enabling the flag, CachedResourceRequest.cpp implicitly depended on having this header included by another file in the unified sources. When the feature is enabled and additional files are included in the unified sources, CachedResourceRequest.cpp is moved to a different combined source and is the first file included, so it doesn't get this header. I should have included this in an earlier patch, but I mistakenly thought it was just a build error in the tree at the time, so I worked around it.

[David Quesada]

(In reply to Sam Weinig from comment #5)
> What makes this feature iOS only? In general, we would like everything to be
> enable on all supported platforms.

It really isn't iOS-specific, I'll enable it on Mac too. The support for testing it is only implemented for Cocoa, so it's not quite ready to be enabled on other platforms yet.

[David Quesada]

Created attachment 328859 [details]
Patch v4

Not iOS-only.

[David Quesada]

Created attachment 328869 [details]
Patch v5

Only enable the layout tests for mac-wk2 and ios-wk2 platforms.

[Geoffrey Garen]

Comment on attachment 328869 [details]
Patch v5

R=me

[WebKit Commit Bot]

Comment on attachment 328869 [details]
Patch v5

Clearing flags on attachment: 328869

Committed r225747: <https://trac.webkit.org/changeset/225747>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.