Bug 48494 – fix setSize() order in WebP image decoding

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

[Pascal Massimino]

Hi,

in WEBPImageDecoder.cpp, setSize() was only called
when onlySize was true, causing refresh problems.
This patch makes sure setSize() is always called
appropriately.

detective work: pkasting@

Pascal

[Pascal Massimino]

Created attachment 72130 [details]
fix setSize() calls in WEBPImageDecoder.cpp

[Adam Barth]

Comment on attachment 72130 [details]
fix setSize() calls in WEBPImageDecoder.cpp

Looks good, but we need a ChangeLog and (ideally) a test.

[Pascal Massimino]

Created attachment 72139 [details]
fix setSize() calls in WEBPImageDecoder.cpp

added the missing ChangeLog

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 72139 [details]
fix setSize() calls in WEBPImageDecoder.cpp

You need to fill out all those fields in the ChangeLog (except the Reviewed by NOBODY line).  Also, we can't land a patch with tabs.

[Pascal Massimino]

Adam,

oops... added the missing ChangeLog...

(In reply to comment #2)
> (From update of attachment 72130 [details])
> Looks good, but we need a ChangeLog and (ideally) a test.

to exercise the bug (lack of refresh) there's some manual intervention required:
 - load a big picture
 - switch to another tab
 - switch back to the picture tab
 - scroll up/down
=> some parts were blank.

I'm not sure how a test can reproduce that...

Pascal

[Pascal Massimino]

Created attachment 72144 [details]
fix setSize() calls in WEBPImageDecoder.cpp

... and now, without the tabs

[Adam Barth]

Comment on attachment 72144 [details]
fix setSize() calls in WEBPImageDecoder.cpp

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

> WebCore/ChangeLog:3
> +        Reviewed by NOBODY (OOPS!).

This line is ok because the tools fill in the reviewer automatically for you.

> WebCore/ChangeLog:6
> +        fix setSize() call flow: it was only called the first
> +        time (when onlySize is true)

Also, you should link to the bug from the ChangeLog so folks can find it from the SVN revision history.

> WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

There's an SVN pre-commit hook that rejects OOPS in files.  This line will prevent the patch from being landed.  You should replace this line with an explanation of why there is no test.

[Pascal Massimino]

Created attachment 72145 [details]
fix setSize() calls in WEBPImageDecoder.cpp

fix the changeLog: add bug URL, explain the missing test.

[Adam Barth]

Comment on attachment 72145 [details]
fix setSize() calls in WEBPImageDecoder.cpp

Thanks.  Your TPS reports are now in order!

[WebKit Commit Bot]

Comment on attachment 72145 [details]
fix setSize() calls in WEBPImageDecoder.cpp

Clearing flags on attachment: 72145

Committed r70758: <http://trac.webkit.org/changeset/70758>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.