Bug 171058 – [GTK] After upgrading glib and glib-networking, resources with zero bytes are always identified as text/plain

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Alberto Lopez Perez]

On r215556 <https://trac.webkit.org/r215556> we upgraded glib from 2.44.1 to 2.52.1 and glib-networking from 2.41.4 to 2.50.0

This causes failures on the following tests:


Regressions: Unexpected text-only failures (13)
  fast/preloader/document-write-noscript.html [ Failure ]
  fast/preloader/document-write.html [ Failure ]
  fast/preloader/first-base-tag-scanned-wins.html [ Failure ]
  fast/preloader/first-base-tag-wins.html [ Failure ]
  fast/preloader/image-srcset.html [ Failure ]
  fast/preloader/image.html [ Failure ]
  fast/preloader/input.html [ Failure ]
  fast/preloader/link.html [ Failure ]
  fast/preloader/noscript.html [ Failure ]
  fast/preloader/script.html [ Failure ]
  fast/preloader/style.html [ Failure ]
  fast/preloader/understands-base-tag.html [ Failure ]
  tables/mozilla/core/col_widths_fix_autoFixPer.html [ Failure ]


This are the diffs for all this tests: http://sprunge.us/GZPB?diff

As you can see, the resource gives always now "MIME type text/plain" instead of the MIME type of the extension of the file.

Upon further investigation it turns out, all this files are empty!

$ ls -1s LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/base-image3.png LayoutTests/fast/preloader/resources/base-image2.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/base-image1.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/link1.css LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/style1.css LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image2.png
0 LayoutTests/fast/preloader/resources/base-image3.png
0 LayoutTests/fast/preloader/resources/image1.png
0 LayoutTests/fast/preloader/resources/image1.png
0 LayoutTests/fast/preloader/resources/link1.css
0 LayoutTests/fast/preloader/resources/noscript-image2.png
0 LayoutTests/fast/preloader/resources/noscript-image2.png
0 LayoutTests/fast/preloader/resources/script1.js
0 LayoutTests/fast/preloader/resources/script1.js
0 LayoutTests/fast/preloader/resources/style1.css


$ file LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/base-image3.png LayoutTests/fast/preloader/resources/base-image2.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/base-image1.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/link1.css LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/style1.css LayoutTests/fast/preloader/resources/base-image1.png
LayoutTests/fast/preloader/resources/script1.js:          empty
LayoutTests/fast/preloader/resources/noscript-image2.png: empty
LayoutTests/fast/preloader/resources/base-image3.png:     empty
LayoutTests/fast/preloader/resources/base-image2.png:     empty
LayoutTests/fast/preloader/resources/image1.png:          empty
LayoutTests/fast/preloader/resources/base-image1.png:     empty
LayoutTests/fast/preloader/resources/image1.png:          empty
LayoutTests/fast/preloader/resources/link1.css:           empty
LayoutTests/fast/preloader/resources/noscript-image2.png: empty
LayoutTests/fast/preloader/resources/script1.js:          empty
LayoutTests/fast/preloader/resources/style1.css:          empty
LayoutTests/fast/preloader/resources/base-image1.png:     empty


And for the test tables/mozilla/core/col_widths_fix_autoFixPer.html it turns out also this test is just an empty html

$ ls -s1 LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html
0 LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html


$ file LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html
LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html: empty


That explains the diff on this test also.

[Carlos Alberto Lopez Perez]

Expectations updated at https://trac.webkit.org/changeset/215556

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #1)
> Expectations updated at https://trac.webkit.org/changeset/215556

 ^^^ I mean at: http://trac.webkit.org/changeset/215561

[Carlos Garcia Campos]

I think this was broken by this commit 

https://git.gnome.org/browse/glib/commit/?id=202a9c3497e0c0b5789e533509dd8671617ae20c

Before this commit the empty file check was done after the g_content_type_guess and only when guessing from path returned an uncertain result. Now the check is done before unconditionally, so there's no basename guess for empty files.

[Carlos Garcia Campos]

Reading the comments in the GNOME bug it seems the change in behavior was accidental, but then they decided it was an improvement. I kind of agree that a  zero size file with .png extension is not a png file (or any other extension). I bet this only affects unit tests, so we can work around the issue inside webkit, either patching glib to only check zero-sized files after guessing the content type form the filename if the result is uncertain, or using real files (small enough, but real) in the tests.

[Carlos Alberto Lopez Perez]

(In reply to Carlos Garcia Campos from comment #4)
> Reading the comments in the GNOME bug it seems the change in behavior was
> accidental, but then they decided it was an improvement. I kind of agree
> that a  zero size file with .png extension is not a png file (or any other
> extension). I bet this only affects unit tests, so we can work around the
> issue inside webkit, either patching glib to only check zero-sized files
> after guessing the content type form the filename if the result is
> uncertain, or using real files (small enough, but real) in the tests.

I wonder why this files are empty on our Layout tests.

Is that intentional? Or it is a mistake nobody noticed so far?
Looking at the git history of this file nothing seems to suggest that the files are empty on purpose. It looks like a mistake.

Maybe we should replace the empty files with files valid for their supposed mime-type (for example, for a png file we can use a 1x1 transparent png).

At the same time we can add a new test that loads several zero-byte resource files. This test should produce plain/text on glib based ports and other thing on the rest of ports.

[Carlos Garcia Campos]

I guess they are empty because the actual contents doesn't really matter. So, making them valid for their mime type would be harmless, and would fix these tests for the glib based ports.

[Michael Catanzaro]

I would vote for changing the tests to not use empty files. This is a silly reason to carry a downstream patch forever and it's not advantageous for our tests to exhibit special behavior that never occurs in releases.

[Claudio Saavedra]

Created attachment 309888 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 309888 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 309888 [details]
Patch

Clearing flags on attachment: 309888

Committed r216762: <http://trac.webkit.org/changeset/216762>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.