Windows layout tests fail when changing ContentSniffingPolicy/ContentEncodingSniffingPolicy enum values

• Component: WebCore Misc.
• Status: NEW ---

[David Kilzer (:ddkilzer)]

Windows layout tests fail when changing ContentSniffingPolicy/ContentEncodingSniffingPolicy enum values as seen in Bug 213093.

However, the build doesn't break, so it seems the Apple Windows port is likely using hard-coded numeric values instead of enum values somewhere.

Here are the two enums I had to revert (in Bug 213093) to fix Windows layout tests in Source/WebCore/loader/ResourceLoaderOptions.h:

enum class ContentSniffingPolicy : bool {
    SniffContent,
    DoNotSniffContent
};

enum class ContentEncodingSniffingPolicy : bool {
    Sniff,
    DoNotSniff
};

See:  <https://bugs.webkit.org/attachment.cgi?oldid=401700&action=interdiff&newid=401704&headers=1>

Failing EWS test run:  <https://ews-build.webkit.org/#/builders/10/builds/21380>
Failing EWS test results:  <https://ews-build.webkit.org/results/Windows-EWS/r401700-21380/results.html>

[David Kilzer (:ddkilzer)]

My usual trick of looking for a C-style cast or a static_cast<> back to one of the enum values didn't turn up anything:

$ grep -r 'SniffingPolicy)' Source/WebCore Source/WebKitLegacy
$ grep -r 'SniffingPolicy>' Source/WebCore Source/WebKitLegacy

[Darin Adler]

I suggest looking for an if statement:

    if (policy) {
        xxx
    }

[David Kilzer (:ddkilzer)]

(In reply to Darin Adler from comment #2)
> I suggest looking for an if statement:
> 
>     if (policy) {
>         xxx
>     }

Couldn't find anything.

Another guess is that an incremental build didn't rebuild some source file (with the new values) because it didn't include ResourceLoaderOptions.h explicitly.

[David Kilzer (:ddkilzer)]

Created attachment 401756 [details]
Patch v1

Try including ResourceLoaderOptions.h everywhere that *SniffingPolicy enums are used.

[Darin Adler]

Comment on attachment 401756 [details]
Patch v1

If this works and passes tests, that will tell us it was a recompilation issues, but I don’t think we should land it this way. We will have proved that those files need to be touched to work around a bug in the dependency tracking of the Windows build tools, and it’s good to do that so we don’t need a full recompile on Windows, but let’s not add includes. Just touch each of the files some other way.

[David Kilzer (:ddkilzer)]

(In reply to Darin Adler from comment #5)
> Comment on attachment 401756 [details]
> Patch v1
> 
> If this works and passes tests, that will tell us it was a recompilation
> issues, but I don’t think we should land it this way. We will have proved
> that those files need to be touched to work around a bug in the dependency
> tracking of the Windows build tools, and it’s good to do that so we don’t
> need a full recompile on Windows, but let’s not add includes. Just touch
> each of the files some other way.

Well, that didn't work.  The failures have to do with HTTP/0.9, so that should narrow down the area of code affected.

I don't have time to look into this further right now.

[Darin Adler]

(In reply to David Kilzer (:ddkilzer) from comment #6)
> The failures have to do with HTTP/0.9, so that
> should narrow down the area of code affected.
> 
> I don't have time to look into this further right now.

We’ll figure it out eventually! No rush, I think.