Re-enable DrMemory test for ICOImageDecoderTests.parseAndDecodeByteByByte.

third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
index f8d39b99ad264cef00633c49a40f6643b3af1b64..42471343d98d7eebcada154baefdf5e9e56ab09d 100644
--- a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
@@ -136,6 +136,10 @@ size_t ICOImageDecoder::decodeFrameCount()
     if (failed())
         return m_frameBufferCache.size();
 
+    // Check if there is enough data available to parse directory entries.
+    if (m_decodedOffset < sizeOfDirectory + m_dirEntries.size() * sizeOfDirectory)

[Peter Kasting, 2016/06/22 23:35:28]

Is the problem here as follows?:

* Someone calls decodeFrameCount() after we've received enough data for the
initial directory, but not for the subsequent directory entries
* So |m_dirEntries| has been resized, but the individual entries are not
initialized
* This means their offsets and sizes are zero, and thus the loop below claims
we've received all of them
* As we start receiving more data, the frame count then recognizes that we don't
have all the necessary data, and decreases

If so, I don't think this is the right fix.  There are a lot of functions in
this file that expect that if m_dirEntries contains an entry, that entry's
details are set.

I think the best way to fix this is:

* Change processDirectory() to take an outparam, numDirectoryEntries, and set it
to the desired number of directory entries.  Remove the m_dirEntries.resize()
call from that function.
* Change processDirectoryEntries() to take this value as an argument, check
against it in the top conditional (instead of using m_dirEntries.size()), and if
the conditional succeeds, do the m_dirEntries.resize() call there.
* Change decodeDirectory() to something like this:

    size_t numDirectoryEntries = m_dirEntries.size();
    if ((m_decodedOffset < sizeOfDirectory) &&
!processDirectory(&numDirectoryEntries))
        ...

    return (m_decodedOffset >= (sizeOfDirectory + (numDirectoryEntries *
sizeOfDirEntry))) || processDirectoryEntries(numDirectoryEntries);

This ensures that m_dirEntries stays empty until we can properly set its
members.  This should avoid the need to add a null constructor for
IconDirectoryEntry as well, as we'll never allocate one of those unless we are
going to immediately set all its members or else setFailed().

[aleksandar.stojiljkovic, 2016/06/23 10:11:29]

Yes, that's the problem.
Used your approach with the change that i introduced member variable to hold
parsed m_dirEntriesCount before resizing m_dirEntries.
This way, we avoid multiple calls to readUint16 in processDirectory when
m_data->size( is between sizeOfDirectory (6) and (sizeOfDirectory +
m_dirEntriesCount * sizeOfDirEntry).
Thanks.

+        return 0;
+
     // Length of sequence of completely received frames.
     for (size_t i = 0; i < m_dirEntries.size(); ++i) {
         const IconDirectoryEntry& dirEntry = m_dirEntries[i];