[webkit-reviews] review denied: [Bug 170730] REGRESSION(r215211): [GTK] Several webgl related tests are failing : [Attachment 306900] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has denied Miguel Gomez
<magomez at igalia.com>'s request for review:
Bug 170730: REGRESSION(r215211): [GTK] Several webgl related tests are failing
https://bugs.webkit.org/show_bug.cgi?id=170730

Attachment 306900: Patch

https://bugs.webkit.org/attachment.cgi?id=306900&action=review




--- Comment #10 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 306900
  --> https://bugs.webkit.org/attachment.cgi?id=306900
Patch

I do not think this is the right fix for this bug.

-- A quick fix is the following:
1. Change back the ImageDecoder:: isSizeAvailable() 

<< bool isSizeAvailable() { return encodedDataStatus() >=
EncodedDataStatus::SizeAvailable; }
>> bool isSizeAvailable() { return m_sizeAvailable; }

2. Change back the condition in all the decoders encodedDataStatus() functions:

EncodedDataStatus JPEGImageDecoder::encodedDataStatus() const
{
<<    if (ImageDecoder::encodedDataStatus() < EncodedDataStatus::SizeAvailable)
>>    if (ImageDecoder::isSizeAvailable())

-- A better but more involving fix is the following:

1. Replace the ImageDecoder members m_failed, m_sizeAvailable,
m_isAllDataReceived by a single member named m_encodedDataStatus. Initialize it
with EncodedDataStatus::TypeAvailable.

2. Make ImageDecoder::setData() fixes the m_encodedDataStatus if its value is 
EncodedDataStatus::TypeAvailable or allDataReceived is true.

	virtual void setData(SharedBuffer& data, bool allDataReceived)
	{
	    if (m_encodedDataStatus == EncodedDataStatus::Error)
		return;

	    m_data = &data;
	    if (m_encodedDataStatus == EncodedDataStatus::TypeAvailable)
		decode();

	    if (m_encodedDataStatus == EncodedDataStatus::Error)
		return;

	    if (allDataReceived) {
		ASSERT(m_encodedDataStatus == EncodedDataStatus::
SizeAvailable);
		m_encodedDataStatus == EncodedDataStatus::Complete;
	   }
	}

3. Add a pure virtual function named decode() in ImageDecoder. And implement it
in all the decoders. For example GIFImageDecoder will have the following
implementation: 

void decode() override { decode(0, GIFSizeQuery); }

4. Delete all the implementations of encodedDataStatus() and make
ImageDecoder:: encodedDataStatus() not virtual and make it returns the member
m_encodedDataStatus.

5. Change ImageDecoder methods setFailed() and fail(), setSize(), 
encodedDataStatus() and isSizeAvailable() to use directly the member
m_encodedDataStatus.

EncodedDataStatus encodedDataStatus() const { return m_encodedDataStatus; }
bool failed() const { return m_encodedDataStatus == EncodedDataStatus::Error; }
bool isSizeAvailable() { return m_encodedDataStatus >=
EncodedDataStatus::SizeAvailable; }