Don't cache cells being measured in wxHtmlListBox.

This can result in a crash if the measuring code is called, possibly
indirectly, from a method of a cell object itself and if that cell is
displaced from the cache while caching the cell created in OnMeasureItem().

Closes #16651.

docs/changes.txt

@@ -116,6 +116,7 @@ All (GUI):
 - Fix wxPGChoices copy ctor (Snoits).
 - Show how to handle files on command line in docview sample (Neil Mayhew).
 - Improve wxFileCtrl::SetFilename() and SetPath() (Kevin B. McCarty).
+- Fix a crash when using animated GIFs in wxHtmlListBox.
 
 wxGTK:
 

include/wx/htmllbox.h

@@ -148,6 +148,9 @@ private:
     // returns index of item that contains given HTML cell
     size_t GetItemForCell(const wxHtmlCell *cell) const;
 
+    // Create the cell for the given item, caller is responsible for freeing it.
+    wxHtmlCell* CreateCellForItem(size_t n) const;
+
     // return physical coordinates of root wxHtmlCell of n-th item
     wxPoint GetRootCellCoords(size_t n) const;
 

src/generic/htmllbox.cpp

@@ -295,37 +295,40 @@ wxString wxHtmlListBox::OnGetItemMarkup(size_t n) const
 // wxHtmlListBox cache handling
 // ----------------------------------------------------------------------------
 
+wxHtmlCell* wxHtmlListBox::CreateCellForItem(size_t n) const
+{
+    if ( !m_htmlParser )
+    {
+        wxHtmlListBox *self = wxConstCast(this, wxHtmlListBox);
+
+        self->m_htmlParser = new wxHtmlWinParser(self);
+        m_htmlParser->SetDC(new wxClientDC(self));
+        m_htmlParser->SetFS(&self->m_filesystem);
+#if !wxUSE_UNICODE
+        if (GetFont().IsOk())
+            m_htmlParser->SetInputEncoding(GetFont().GetEncoding());
+#endif
+        // use system's default GUI font by default:
+        m_htmlParser->SetStandardFonts();
+    }
+
+    wxHtmlContainerCell *cell = (wxHtmlContainerCell *)m_htmlParser->
+            Parse(OnGetItemMarkup(n));
+    wxCHECK_MSG( cell, NULL, wxT("wxHtmlParser::Parse() returned NULL?") );
+
+    // set the cell's ID to item's index so that CellCoordsToPhysical()
+    // can quickly find the item:
+    cell->SetId(wxString::Format(wxT("%lu"), (unsigned long)n));
+
+    cell->Layout(GetClientSize().x - 2*GetMargins().x);
+
+    return cell;
+}
+
 void wxHtmlListBox::CacheItem(size_t n) const
 {
     if ( !m_cache->Has(n) )
-    {
-        if ( !m_htmlParser )
-        {
-            wxHtmlListBox *self = wxConstCast(this, wxHtmlListBox);
-
-            self->m_htmlParser = new wxHtmlWinParser(self);
-            m_htmlParser->SetDC(new wxClientDC(self));
-            m_htmlParser->SetFS(&self->m_filesystem);
-#if !wxUSE_UNICODE
-            if (GetFont().IsOk())
-                m_htmlParser->SetInputEncoding(GetFont().GetEncoding());
-#endif
-            // use system's default GUI font by default:
-            m_htmlParser->SetStandardFonts();
-        }
-
-        wxHtmlContainerCell *cell = (wxHtmlContainerCell *)m_htmlParser->
-                Parse(OnGetItemMarkup(n));
-        wxCHECK_RET( cell, wxT("wxHtmlParser::Parse() returned NULL?") );
-
-        // set the cell's ID to item's index so that CellCoordsToPhysical()
-        // can quickly find the item:
-        cell->SetId(wxString::Format(wxT("%lu"), (unsigned long)n));
-
-        cell->Layout(GetClientSize().x - 2*GetMargins().x);
-
-        m_cache->Store(n, cell);
-    }
+        m_cache->Store(n, CreateCellForItem(n));
 }
 
 void wxHtmlListBox::OnSize(wxSizeEvent& event)
@@ -425,12 +428,18 @@ void wxHtmlListBox::OnDrawItem(wxDC& dc, const wxRect& rect, size_t n) const
 
 wxCoord wxHtmlListBox::OnMeasureItem(size_t n) const
 {
-    CacheItem(n);
+    // Notice that we can't cache the cell here because we could be called from
+    // some code updating an existing cell which could be displaced from the
+    // cache if we called CacheItem() and destroyed -- resulting in a crash
+    // when we return to its method from here, see #16651.
+    wxHtmlCell * const cell = CreateCellForItem(n);
+    if ( !cell )
+        return 0;
 
-    wxHtmlCell *cell = m_cache->Get(n);
-    wxCHECK_MSG( cell, 0, wxT("this cell should be cached!") );
+    const wxCoord h = cell->GetHeight() + cell->GetDescent() + 4;
+    delete cell;
 
-    return cell->GetHeight() + cell->GetDescent() + 4;
+    return h;
 }
 
 // ----------------------------------------------------------------------------