Bug 33258 – [Regression] PNG-in-ICO decoding in public image decoders broken

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Peter Kasting]

Created attachment 45968 [details]
down.ico

http://trac.webkit.org/changeset/52831 broke PNG-in-ICO decoding.  Testcase attached.  Will fix soon.

[Yong Li]

sorry it's my bug.

see line 121 in PNGImageDecoder.cpp:

            if ((sizeOnly && decoder->isSizeAvailable()) || m_hasFinishedDecoding)
                break;

just chaning "break" to "return" should fix it.

-Yong

[Yong Li]

Created attachment 45973 [details]
the candidate patch

[WebKit Review Bot]

Attachment 45973 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Traceback (most recent call last):
  File "WebKitTools/Scripts/check-webkit-style", line 98, in <module>
    main()
  File "WebKitTools/Scripts/check-webkit-style", line 62, in main
    defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
AttributeError: 'module' object has no attribute 'ArgumentDefaults'

[Adam Barth]

Sorry for the spam.  Stylebot is sick.

[Eric Seidel (no email)]

Looks like this caused crashes:
http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/8928
http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r52862%20(8928)/results.html
svg/W3C-SVG-1.1/linking-a-01-b.svg

I think we'll need to roll this out. :(

[Eric Seidel (no email)]

http://trac.webkit.org/changeset/52862 was the original commit.

[Eric Seidel (no email)]

Do we not have a way to test image decoders?  I'm surprised this landed w/o a test case?

[Eric Seidel (no email)]

Yong doesn't seem to be around on IRC, so I'm rolling this out in hopes it will fix the crashes on the bot.

[Eric Seidel (no email)]

Rolled out in http://trac.webkit.org/changeset/52871.  Peter Kasting notes that we may need to roll out r52831 as well, otherwise this breaks chromium.  This change should have been unrelated.  If it's found to be unrelated, I'll roll it back in.

[Eric Seidel (no email)]

I'm rolling this back in after further discussions with Peter and Yong.  I'm testing it on Leopard Debug first to make sure.

[Eric Seidel (no email)]

The leopard build is broken at the moment due to http://trac.webkit.org/changeset/52872, so I can't land the re-land yet.

[Eric Seidel (no email)]

Committed r52880: <http://trac.webkit.org/changeset/52880>