Revert of Re-land: Blink image-decoders: (lazy) deferred image decoding support for ICO

Revert of Re-land: Blink image-decoders: (lazy) deferred image decoding support for ICO (patchset #2 id:20001 of https://codereview.chromium.org/2065423003/ )

Reason for revert:
This again caused errors on the DrMemorybots.

First failing build with this change:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5095

Sample error:
ICOImageDecoderTests.parseAndDecodeByteByByte:
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83): error: Expected: (frameCount) <= (decoder->frameCount()), actual: 2 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83): error: Expected: (frameCount) <= (decoder->frameCount()), actual: 3 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83): error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83): error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0

Original issue's description:
> Re-land: Blink image-decoders: (lazy) deferred image decoding support for ICO
> 
> BUG=513306
> 
> original patch review-url: https://codereview.chromium.org/2044093002
> 
> Committed: https://crrev.com/f32e119c6c7ec9e98298fcd43a285b735f021952
> Cr-Commit-Position: refs/heads/master@{#400383}

TBR=pkasting@chromium.org,scroggo@chromium.org,fmalita@chromium.org,aleksandar.stojiljkovic@intel.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=513306

[benwells, 2016-06-20 10:28:24]

The CQ bit was checked by benwells@chromium.org

[benwells, 2016-06-20 10:28:25]

Created Revert of Re-land: Blink image-decoders: (lazy) deferred image decoding
support for ICO

[commit-bot: I haz the power, 2016-06-20 10:28:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2080673002/1

[commit-bot: I haz the power, 2016-06-20 11:44:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-20 11:44:11]

Failed to apply patch for
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp' with
conflicts.
  U third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp

Patch:      
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
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
100644
--- a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
@@ -85,14 +85,6 @@
     return m_frameSize.isEmpty() ? ImageDecoder::setSize(width, height) :
((IntSize(width, height) == m_frameSize) || setFailed());
 }
 
-bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
-{
-    if (index >= m_dirEntries.size())
-        return false;
-    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
-    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <= m_data->size();
-}
-
 bool ICOImageDecoder::setFailed()
 {
     m_bmpReaders.clear();
@@ -130,19 +122,6 @@
 size_t ICOImageDecoder::decodeFrameCount()
 {
     decodeSize();
-
-    // If decodeSize() fails, return the existing number of frames.  This way
-    // if we get halfway through the image before decoding fails, we won't
-    // suddenly start reporting that the image has zero frames.
-    if (failed())
-        return m_frameBufferCache.size();
-
-    // Length of sequence of completely received frames.
-    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
-        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
-        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) > m_data->size())
-            return i;
-    }
     return m_dirEntries.size();
 }
 
@@ -196,12 +175,13 @@
 
     if (imageType == BMP) {
         if (!m_bmpReaders[index]) {
+            // We need to have already sized m_frameBufferCache before this,
and
+            // we must not resize it again later (see caution in frameCount()).
+            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
             m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
dirEntry.m_imageOffset, 0, true));
             m_bmpReaders[index]->setData(m_data.get());
+            m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
         }
-        // Update the pointer to the buffer as it could change after
-        // m_frameBufferCache.resize().
-        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
         m_frameSize = dirEntry.m_size;
         bool result = m_bmpReaders[index]->decodeBMP(false);
         m_frameSize = IntSize();
@@ -300,7 +280,6 @@
         entry.m_bitCount = readUint16(6);
         entry.m_hotSpot = IntPoint();
     }
-    entry.m_byteSize = readUint32(8);
     entry.m_imageOffset = readUint32(12);
 
     // Some icons don't have a bit depth, only a color count.  Convert the

[aleksandar.stojiljkovic, 2016-06-20 11:53:33]

On 2016/06/20 11:44:11, commit-bot: I haz the power wrote:
> Failed to apply patch for
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
> While running git apply --index -3 -p1;
>   error: patch failed:
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
>   Falling back to three-way merge...
>   Applied patch to
> 'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp'
with
> conflicts.
>   U third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> 
> Patch:      
> 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
>
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
> 100644
> ---
a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> +++
b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> @@ -85,14 +85,6 @@
>      return m_frameSize.isEmpty() ? ImageDecoder::setSize(width, height) :
> ((IntSize(width, height) == m_frameSize) || setFailed());
>  }
>  
> -bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
> -{
> -    if (index >= m_dirEntries.size())
> -        return false;
> -    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
> -    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <= m_data->size();
> -}
> -
>  bool ICOImageDecoder::setFailed()
>  {
>      m_bmpReaders.clear();
> @@ -130,19 +122,6 @@
>  size_t ICOImageDecoder::decodeFrameCount()
>  {
>      decodeSize();
> -
> -    // If decodeSize() fails, return the existing number of frames.  This way
> -    // if we get halfway through the image before decoding fails, we won't
> -    // suddenly start reporting that the image has zero frames.
> -    if (failed())
> -        return m_frameBufferCache.size();
> -
> -    // Length of sequence of completely received frames.
> -    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
> -        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
> -        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) > m_data->size())
> -            return i;
> -    }
>      return m_dirEntries.size();
>  }
>  
> @@ -196,12 +175,13 @@
>  
>      if (imageType == BMP) {
>          if (!m_bmpReaders[index]) {
> +            // We need to have already sized m_frameBufferCache before this,
> and
> +            // we must not resize it again later (see caution in
frameCount()).
> +            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
>              m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
> dirEntry.m_imageOffset, 0, true));
>              m_bmpReaders[index]->setData(m_data.get());
> +            m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
>          }
> -        // Update the pointer to the buffer as it could change after
> -        // m_frameBufferCache.resize().
> -        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
>          m_frameSize = dirEntry.m_size;
>          bool result = m_bmpReaders[index]->decodeBMP(false);
>          m_frameSize = IntSize();
> @@ -300,7 +280,6 @@
>          entry.m_bitCount = readUint16(6);
>          entry.m_hotSpot = IntPoint();
>      }
> -    entry.m_byteSize = readUint32(8);
>      entry.m_imageOffset = readUint32(12);
>  
>      // Some icons don't have a bit depth, only a color count.  Convert the

Apologize for this again. Could I help somehow about "failed to apply patch"
issue here?

[scroggo_chromium, 2016-06-20 12:24:47]

On 2016/06/20 11:53:33, aleksandar.stojiljkovic wrote:
> On 2016/06/20 11:44:11, commit-bot: I haz the power wrote:
> > Failed to apply patch for
> > third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
> > While running git apply --index -3 -p1;
> >   error: patch failed:
> >
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
> >   Falling back to three-way merge...
> >   Applied patch to
> > 'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp'
> with
> > conflicts.
> >   U
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > 
> > Patch:      
> > 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
> >
>
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
> > 100644
> > ---
> a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > +++
> b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > @@ -85,14 +85,6 @@
> >      return m_frameSize.isEmpty() ? ImageDecoder::setSize(width, height) :
> > ((IntSize(width, height) == m_frameSize) || setFailed());
> >  }
> >  
> > -bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
> > -{
> > -    if (index >= m_dirEntries.size())
> > -        return false;
> > -    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
> > -    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <=
m_data->size();
> > -}
> > -
> >  bool ICOImageDecoder::setFailed()
> >  {
> >      m_bmpReaders.clear();
> > @@ -130,19 +122,6 @@
> >  size_t ICOImageDecoder::decodeFrameCount()
> >  {
> >      decodeSize();
> > -
> > -    // If decodeSize() fails, return the existing number of frames.  This
way
> > -    // if we get halfway through the image before decoding fails, we won't
> > -    // suddenly start reporting that the image has zero frames.
> > -    if (failed())
> > -        return m_frameBufferCache.size();
> > -
> > -    // Length of sequence of completely received frames.
> > -    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
> > -        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
> > -        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) >
m_data->size())
> > -            return i;
> > -    }
> >      return m_dirEntries.size();
> >  }
> >  
> > @@ -196,12 +175,13 @@
> >  
> >      if (imageType == BMP) {
> >          if (!m_bmpReaders[index]) {
> > +            // We need to have already sized m_frameBufferCache before
this,
> > and
> > +            // we must not resize it again later (see caution in
> frameCount()).
> > +            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
> >              m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
> > dirEntry.m_imageOffset, 0, true));
> >              m_bmpReaders[index]->setData(m_data.get());
> > +            m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> >          }
> > -        // Update the pointer to the buffer as it could change after
> > -        // m_frameBufferCache.resize().
> > -        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> >          m_frameSize = dirEntry.m_size;
> >          bool result = m_bmpReaders[index]->decodeBMP(false);
> >          m_frameSize = IntSize();
> > @@ -300,7 +280,6 @@
> >          entry.m_bitCount = readUint16(6);
> >          entry.m_hotSpot = IntPoint();
> >      }
> > -    entry.m_byteSize = readUint32(8);
> >      entry.m_imageOffset = readUint32(12);
> >  
> >      // Some icons don't have a bit depth, only a color count.  Convert the
> 
> Apologize for this again. Could I help somehow about "failed to apply patch"
> issue here?

That means you need to rebase. It looks like you created this CL by clicking the
revert button, but you can still run

git cl patch 2080673002

to checkout this branch. Then fix any problems and re-upload.

[benwells, 2016-06-20 12:44:54]

On 2016/06/20 12:24:47, scroggo_chromium wrote:
> On 2016/06/20 11:53:33, aleksandar.stojiljkovic wrote:
> > On 2016/06/20 11:44:11, commit-bot: I haz the power wrote:
> > > Failed to apply patch for
> > > third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
> > > While running git apply --index -3 -p1;
> > >   error: patch failed:
> > >
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
> > >   Falling back to three-way merge...
> > >   Applied patch to
> > >
'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp'
> > with
> > > conflicts.
> > >   U
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > 
> > > Patch:      
> > > 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
> > >
> >
>
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
> > > 100644
> > > ---
> > a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > +++
> > b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > @@ -85,14 +85,6 @@
> > >      return m_frameSize.isEmpty() ? ImageDecoder::setSize(width, height) :
> > > ((IntSize(width, height) == m_frameSize) || setFailed());
> > >  }
> > >  
> > > -bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
> > > -{
> > > -    if (index >= m_dirEntries.size())
> > > -        return false;
> > > -    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
> > > -    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <=
> m_data->size();
> > > -}
> > > -
> > >  bool ICOImageDecoder::setFailed()
> > >  {
> > >      m_bmpReaders.clear();
> > > @@ -130,19 +122,6 @@
> > >  size_t ICOImageDecoder::decodeFrameCount()
> > >  {
> > >      decodeSize();
> > > -
> > > -    // If decodeSize() fails, return the existing number of frames.  This
> way
> > > -    // if we get halfway through the image before decoding fails, we
won't
> > > -    // suddenly start reporting that the image has zero frames.
> > > -    if (failed())
> > > -        return m_frameBufferCache.size();
> > > -
> > > -    // Length of sequence of completely received frames.
> > > -    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
> > > -        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
> > > -        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) >
> m_data->size())
> > > -            return i;
> > > -    }
> > >      return m_dirEntries.size();
> > >  }
> > >  
> > > @@ -196,12 +175,13 @@
> > >  
> > >      if (imageType == BMP) {
> > >          if (!m_bmpReaders[index]) {
> > > +            // We need to have already sized m_frameBufferCache before
> this,
> > > and
> > > +            // we must not resize it again later (see caution in
> > frameCount()).
> > > +            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
> > >              m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
> > > dirEntry.m_imageOffset, 0, true));
> > >              m_bmpReaders[index]->setData(m_data.get());
> > > +            m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > >          }
> > > -        // Update the pointer to the buffer as it could change after
> > > -        // m_frameBufferCache.resize().
> > > -        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > >          m_frameSize = dirEntry.m_size;
> > >          bool result = m_bmpReaders[index]->decodeBMP(false);
> > >          m_frameSize = IntSize();
> > > @@ -300,7 +280,6 @@
> > >          entry.m_bitCount = readUint16(6);
> > >          entry.m_hotSpot = IntPoint();
> > >      }
> > > -    entry.m_byteSize = readUint32(8);
> > >      entry.m_imageOffset = readUint32(12);
> > >  
> > >      // Some icons don't have a bit depth, only a color count.  Convert
the
> > 
> > Apologize for this again. Could I help somehow about "failed to apply patch"
> > issue here?
> 
> That means you need to rebase. It looks like you created this CL by clicking
the
> revert button, but you can still run
> 
> git cl patch 2080673002
> 
> to checkout this branch. Then fix any problems and re-upload.

I've logged issue crbug.com/621488 to fix this. In the meantime I've put up a CL
to disable the test under DrMemory.

[benwells, 2016-06-20 12:47:35]

On 2016/06/20 12:44:54, benwells wrote:
> On 2016/06/20 12:24:47, scroggo_chromium wrote:
> > On 2016/06/20 11:53:33, aleksandar.stojiljkovic wrote:
> > > On 2016/06/20 11:44:11, commit-bot: I haz the power wrote:
> > > > Failed to apply patch for
> > > >
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
> > > > While running git apply --index -3 -p1;
> > > >   error: patch failed:
> > > >
> >
third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
> > > >   Falling back to three-way merge...
> > > >   Applied patch to
> > > >
> 'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp'
> > > with
> > > > conflicts.
> > > >   U
> > third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > 
> > > > Patch:      
> > > >
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
> > > >
> > >
> >
>
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
> > > > 100644
> > > > ---
> > >
a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > +++
> > >
b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > @@ -85,14 +85,6 @@
> > > >      return m_frameSize.isEmpty() ? ImageDecoder::setSize(width, height)
:
> > > > ((IntSize(width, height) == m_frameSize) || setFailed());
> > > >  }
> > > >  
> > > > -bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
> > > > -{
> > > > -    if (index >= m_dirEntries.size())
> > > > -        return false;
> > > > -    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
> > > > -    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <=
> > m_data->size();
> > > > -}
> > > > -
> > > >  bool ICOImageDecoder::setFailed()
> > > >  {
> > > >      m_bmpReaders.clear();
> > > > @@ -130,19 +122,6 @@
> > > >  size_t ICOImageDecoder::decodeFrameCount()
> > > >  {
> > > >      decodeSize();
> > > > -
> > > > -    // If decodeSize() fails, return the existing number of frames. 
This
> > way
> > > > -    // if we get halfway through the image before decoding fails, we
> won't
> > > > -    // suddenly start reporting that the image has zero frames.
> > > > -    if (failed())
> > > > -        return m_frameBufferCache.size();
> > > > -
> > > > -    // Length of sequence of completely received frames.
> > > > -    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
> > > > -        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
> > > > -        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) >
> > m_data->size())
> > > > -            return i;
> > > > -    }
> > > >      return m_dirEntries.size();
> > > >  }
> > > >  
> > > > @@ -196,12 +175,13 @@
> > > >  
> > > >      if (imageType == BMP) {
> > > >          if (!m_bmpReaders[index]) {
> > > > +            // We need to have already sized m_frameBufferCache before
> > this,
> > > > and
> > > > +            // we must not resize it again later (see caution in
> > > frameCount()).
> > > > +            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
> > > >              m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
> > > > dirEntry.m_imageOffset, 0, true));
> > > >              m_bmpReaders[index]->setData(m_data.get());
> > > > +            m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > > >          }
> > > > -        // Update the pointer to the buffer as it could change after
> > > > -        // m_frameBufferCache.resize().
> > > > -        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > > >          m_frameSize = dirEntry.m_size;
> > > >          bool result = m_bmpReaders[index]->decodeBMP(false);
> > > >          m_frameSize = IntSize();
> > > > @@ -300,7 +280,6 @@
> > > >          entry.m_bitCount = readUint16(6);
> > > >          entry.m_hotSpot = IntPoint();
> > > >      }
> > > > -    entry.m_byteSize = readUint32(8);
> > > >      entry.m_imageOffset = readUint32(12);
> > > >  
> > > >      // Some icons don't have a bit depth, only a color count.  Convert
> the
> > > 
> > > Apologize for this again. Could I help somehow about "failed to apply
patch"
> > > issue here?
> > 
> > That means you need to rebase. It looks like you created this CL by clicking
> the
> > revert button, but you can still run
> > 
> > git cl patch 2080673002
> > 
> > to checkout this branch. Then fix any problems and re-upload.
> 
> I've logged issue crbug.com/621488 to fix this. In the meantime I've put up a
CL
> to disable the test under DrMemory.

And, just to be clear :)

By 'to fix this' I really mean 'for you to fix this'. Please fix the underlying
problem then re-enable the test.

Thanks!

[aleksandar.stojiljkovic, 2016-06-20 13:00:19]

On 2016/06/20 12:47:35, benwells wrote:
> On 2016/06/20 12:44:54, benwells wrote:
> > On 2016/06/20 12:24:47, scroggo_chromium wrote:
> > > On 2016/06/20 11:53:33, aleksandar.stojiljkovic wrote:
> > > > On 2016/06/20 11:44:11, commit-bot: I haz the power wrote:
> > > > > Failed to apply patch for
> > > > >
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:
> > > > > While running git apply --index -3 -p1;
> > > > >   error: patch failed:
> > > > >
> > >
> third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp:196
> > > > >   Falling back to three-way merge...
> > > > >   Applied patch to
> > > > >
> > 'third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp'
> > > > with
> > > > > conflicts.
> > > > >   U
> > > third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > > 
> > > > > Patch:      
> > > > >
> 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
> > > > >
> > > >
> > >
> >
>
00d19d7eba0776767bf69ae1cb21bd02216c6ecb..e50b37c361ee927d03183f39e764da215a702aef
> > > > > 100644
> > > > > ---
> > > >
> a/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > > +++
> > > >
> b/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp
> > > > > @@ -85,14 +85,6 @@
> > > > >      return m_frameSize.isEmpty() ? ImageDecoder::setSize(width,
height)
> :
> > > > > ((IntSize(width, height) == m_frameSize) || setFailed());
> > > > >  }
> > > > >  
> > > > > -bool ICOImageDecoder::frameIsCompleteAtIndex(size_t index) const
> > > > > -{
> > > > > -    if (index >= m_dirEntries.size())
> > > > > -        return false;
> > > > > -    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
> > > > > -    return (dirEntry.m_imageOffset + dirEntry.m_byteSize) <=
> > > m_data->size();
> > > > > -}
> > > > > -
> > > > >  bool ICOImageDecoder::setFailed()
> > > > >  {
> > > > >      m_bmpReaders.clear();
> > > > > @@ -130,19 +122,6 @@
> > > > >  size_t ICOImageDecoder::decodeFrameCount()
> > > > >  {
> > > > >      decodeSize();
> > > > > -
> > > > > -    // If decodeSize() fails, return the existing number of frames. 
> This
> > > way
> > > > > -    // if we get halfway through the image before decoding fails, we
> > won't
> > > > > -    // suddenly start reporting that the image has zero frames.
> > > > > -    if (failed())
> > > > > -        return m_frameBufferCache.size();
> > > > > -
> > > > > -    // Length of sequence of completely received frames.
> > > > > -    for (size_t i = 0; i < m_dirEntries.size(); ++i) {
> > > > > -        const IconDirectoryEntry& dirEntry = m_dirEntries[i];
> > > > > -        if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) >
> > > m_data->size())
> > > > > -            return i;
> > > > > -    }
> > > > >      return m_dirEntries.size();
> > > > >  }
> > > > >  
> > > > > @@ -196,12 +175,13 @@
> > > > >  
> > > > >      if (imageType == BMP) {
> > > > >          if (!m_bmpReaders[index]) {
> > > > > +            // We need to have already sized m_frameBufferCache
before
> > > this,
> > > > > and
> > > > > +            // we must not resize it again later (see caution in
> > > > frameCount()).
> > > > > +            ASSERT(m_frameBufferCache.size() == m_dirEntries.size());
> > > > >              m_bmpReaders[index] = adoptPtr(new BMPImageReader(this,
> > > > > dirEntry.m_imageOffset, 0, true));
> > > > >              m_bmpReaders[index]->setData(m_data.get());
> > > > > +           
m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > > > >          }
> > > > > -        // Update the pointer to the buffer as it could change after
> > > > > -        // m_frameBufferCache.resize().
> > > > > -        m_bmpReaders[index]->setBuffer(&m_frameBufferCache[index]);
> > > > >          m_frameSize = dirEntry.m_size;
> > > > >          bool result = m_bmpReaders[index]->decodeBMP(false);
> > > > >          m_frameSize = IntSize();
> > > > > @@ -300,7 +280,6 @@
> > > > >          entry.m_bitCount = readUint16(6);
> > > > >          entry.m_hotSpot = IntPoint();
> > > > >      }
> > > > > -    entry.m_byteSize = readUint32(8);
> > > > >      entry.m_imageOffset = readUint32(12);
> > > > >  
> > > > >      // Some icons don't have a bit depth, only a color count. 
Convert
> > the
> > > > 
> > > > Apologize for this again. Could I help somehow about "failed to apply
> patch"
> > > > issue here?
> > > 
> > > That means you need to rebase. It looks like you created this CL by
clicking
> > the
> > > revert button, but you can still run
> > > 
> > > git cl patch 2080673002
> > > 
> > > to checkout this branch. Then fix any problems and re-upload.
> > 
> > I've logged issue crbug.com/621488 to fix this. In the meantime I've put up
a
> CL
> > to disable the test under DrMemory.
> 
> And, just to be clear :)
> 
> By 'to fix this' I really mean 'for you to fix this'. Please fix the
underlying
> problem then re-enable the test.
> 
> Thanks!

Thanks a lot. I'm on it.

[benwells, 2016-06-28 08:20:32]

Description was changed from

==========
Revert of Re-land: Blink image-decoders: (lazy) deferred image decoding support
for ICO (patchset #2 id:20001 of https://codereview.chromium.org/2065423003/ )

Reason for revert:
This again caused errors on the DrMemorybots.

First failing build with this change:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%2...

Sample error:
ICOImageDecoderTests.parseAndDecodeByteByByte:
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 2 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 3 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0

Original issue's description:
> Re-land: Blink image-decoders: (lazy) deferred image decoding support for ICO
> 
> BUG=513306
> 
> original patch review-url: https://codereview.chromium.org/2044093002
> 
> Committed: https://crrev.com/f32e119c6c7ec9e98298fcd43a285b735f021952
> Cr-Commit-Position: refs/heads/master@{#400383}

TBR=pkasting@chromium.org,scroggo@chromium.org,fmalita@chromium.org,aleksanda...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=513306
==========

to

==========
Revert of Re-land: Blink image-decoders: (lazy) deferred image decoding support
for ICO (patchset #2 id:20001 of https://codereview.chromium.org/2065423003/ )

Reason for revert:
This again caused errors on the DrMemorybots.

First failing build with this change:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%2...

Sample error:
ICOImageDecoderTests.parseAndDecodeByteByByte:
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 2 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 3 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0
c:\b\build\slave\drm-cr\build\src\third_party\webkit\source\platform\image-decoders\imagedecodertesthelpers.cpp(83):
error: Expected: (frameCount) <= (decoder->frameCount()), actual: 1 vs 0

Original issue's description:
> Re-land: Blink image-decoders: (lazy) deferred image decoding support for ICO
> 
> BUG=513306
> 
> original patch review-url: https://codereview.chromium.org/2044093002
> 
> Committed: https://crrev.com/f32e119c6c7ec9e98298fcd43a285b735f021952
> Cr-Commit-Position: refs/heads/master@{#400383}

TBR=pkasting@chromium.org,scroggo@chromium.org,fmalita@chromium.org,aleksanda...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=513306
==========