[Webkit-unassigned] [Bug 22191] logic error in CachedImage.cpp

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22191


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25081|review?                     |review-
               Flag|                            |




------- Comment #2 from darin at apple.com  2008-11-12 08:59 PDT -------
(From update of attachment 25081)
You say that the mistake in this function is obvious. But it's not obvious to
me. It seems to me that the clear() function needs to be called before
m_errorOccurred, because we want it to actually discard any decoded data in the
partially created image.

In fact, I'm not sure why the check for m_errorOccurred in the
destroyDecodedData is helpful. It seems that we do want to destroy the decoded
data even if an error occurred.

You say that this causes a crash in Chromium, but you don't supply any details.

I'm not convinced this is the correct fix.

> +	Fixes logic of error() to first set
> +	m_errorOccurred, then call clear(),
> +	which depends on m_errorOccurred having the
> +	correct value.

Please don't use tabs in ChangeLog -- our repository is configured so it
doesn't allow tabs.

I think it's likely that if this is really a bug there's some way to reproduce
a crash in ports other than Chromium and therefore we can create a regression
test.

If you remain convinced that this patch is 1) correct and 2) not testable in
other ports, then feel free to submit the patch again, but I don't think it's
obvious that this is a correct change.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.