Remove ImageSource::bytesDecodedToDetermineProperties() and fallout

Remove ImageSource::bytesDecodedToDetermineProperties() and fallout

Since the WebKit split, it always returns zero. Removing it enables us
also to remove the not always up-to-date m_decodedSize and avoid a
quite common assertion failure on GIF animations. No functional
changes expected.

BUG=288879
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159433

[slavi, 2013-10-04 21:35:16]

On 2013/10/04 09:07:41, davve wrote:

+pkasting, hclam
Looks good to me but @pkasting and @hclam would be better reviewers for that.

[Alpha Left Google, 2013-10-04 21:37:08]

pkasting was in the discussion, he can better review this than i do.

[Peter Kasting, 2013-10-04 22:04:14]

LGTM

[commit-bot: I haz the power, 2013-10-05 06:42:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/1

[davve, 2013-10-05 06:45:09]

Could someone help me land this, please? (I'm not a committer and a bit unsure
about procedure.)

[commit-bot: I haz the power, 2013-10-05 07:05:24]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-05 07:35:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/20001

[slavi, 2013-10-07 17:17:13]

On 2013/10/05 06:45:09, davve wrote:
> Could someone help me land this, please? (I'm not a committer and a bit unsure
> about procedure.)

Try rebasing the patch, reuploading and clicking on Commit again. The tree was
red last friday.

[commit-bot: I haz the power, 2013-10-07 19:54:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/25001

[davve, 2013-10-07 19:55:56]

On 2013/10/07 17:17:13, slavi wrote:
> On 2013/10/05 06:45:09, davve wrote:
> > Could someone help me land this, please? (I'm not a committer and a bit
unsure
> > about procedure.)
> 
> Try rebasing the patch, reuploading and clicking on Commit again. The tree was
> red last friday.

Ok. Attempting commit.

[commit-bot: I haz the power, 2013-10-07 20:11:59]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[davve, 2013-10-08 04:36:54]

'presubmit' failed because I'm missing review on my updates in patch set #2.

[slavi, 2013-10-08 17:30:58]

On 2013/10/08 04:36:54, davve wrote:
> 'presubmit' failed because I'm missing review on my updates in patch set #2.

+jamesr who's OWNER of WebKit/Source/web for review

[slavi, 2013-10-08 17:31:23]

+jamesr for real

[jamesr, 2013-10-08 20:44:05]

lgtm

[commit-bot: I haz the power, 2013-10-08 21:44:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/25001

[commit-bot: I haz the power, 2013-10-08 22:25:12]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_layout.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[davve, 2013-10-09 06:54:36]

Gulp, not sure how but I missed the one obvious test:
Source/core/platform/graphics/BitmapImageTest.cpp. It seems to have one of few
users of Image::decodedSize(). Do you prefer if I put back an implementation of
decodedSize() that loops through the decoder and adds up the decoded frames, or
do I adjust the test somehow? Not sure what the policy is on having production
code just for testing.

From a quick scan, it looks like I would have to add methods to BitmapImage,
ImageSource, DeferredImageDecoder and ImageDecoder.

[davve, 2013-10-09 11:12:56]

On 2013/10/09 06:54:36, davve wrote:
> Do you prefer if I put back an implementation of
> decodedSize() that loops through the decoder and adds up the decoded frames,
or
> do I adjust the test somehow? 

Since the test always decodes all frames, I can make do with the cached frame
data. I updated the test with a decodedSize() of its own, added a disclaimer and
made a small addition to FrameData::clear() to clear the m_frameBytes member to
make the test still pass.

PTAL.

[Peter Kasting, 2013-10-10 18:53:40]

LGTM

https://codereview.chromium.org/25976002/diff/54001/Source/core/platform/grap...
File Source/core/platform/graphics/BitmapImageTest.cpp (right):

https://codereview.chromium.org/25976002/diff/54001/Source/core/platform/grap...
Source/core/platform/graphics/BitmapImageTest.cpp:90: // In the context of this
test, the following loop will give the correct result. But only because the test
Nit: result. But -> result, but

https://codereview.chromium.org/25976002/diff/54001/Source/core/platform/grap...
Source/core/platform/graphics/BitmapImageTest.cpp:92: // is up-to-date. Because
of how multi frame images (like GIF) works, requesting one frame to be decoded
may
Nit: works -> work

[commit-bot: I haz the power, 2013-10-10 19:31:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/63001

[commit-bot: I haz the power, 2013-10-10 19:58:22]

Retried try job too often on linux_blink_rel for step(s) webkit_unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-10-11 06:44:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/25976002/46001

[commit-bot: I haz the power, 2013-10-11 08:50:18]

Change committed as 159433