Bug 24636 – Fix crash of Chromium port in use of BackForwardList in SVG images

• Component: Platform
• Status: RESOLVED DUPLICATE

[Hin-Chung Lam]

Crash filed against Chromium: http://code.google.com/p/chromium/issues/detail?id=6869

When a SVG image is created, a WebCore::Page is created with BackForwardList but
BackForwardList::m_client is never initialized. During eviction of the cached SVG image, m_client->close() is called in BackForwardList::close() resulting in a crash. Since the WebCore::Page in SVGImage doesn't have links to the frame, there's no BackForwardListClient implementation available, so we just don't care these requests to BackForwardList and don't delegate the requests when m_client is NULL.

[Hin-Chung Lam]

Created attachment 28673 [details]
patch

[Hin-Chung Lam]

Review in Chromium: http://codereview.chromium.org/42265/show

[Darin Fisher (:fishd, Google)]

maruel already fixed this:
http://trac.webkit.org/changeset/41824

[Hin-Chung Lam]

"if (m_client)" is only performed on close(), this check should be added to other functions as well.

[Hin-Chung Lam]

Created attachment 28755 [details]
adding if (m_client) check to other methods

[Hin-Chung Lam]

Comment on attachment 28755 [details]
adding if (m_client) check to other methods

Since maruel has patched the class and initialize m_client as NULL, we should avoid using m_client in this case in all methods too.

[Darin Fisher (:fishd, Google)]

This was discussed.  Please see:
https://bugs.webkit.org/show_bug.cgi?id=24398#c6

[Eric Seidel (no email)]

Comment on attachment 28755 [details]
adding if (m_client) check to other methods

I would have just added an ASSERT(m_client) before these calls in all cases:

-    return m_client->backListCount();
+    if (m_client)
+        return m_client->backListCount();
+    ASSERT_NOT_REACHED();
+    return 0;

No need to work hard to crash only in debug mode.

The changelog should mention the bug url.

Also is there no way to test this?  There should be a layout test if at all possible.

[Hin-Chung Lam]

*** This bug has been marked as a duplicate of bug 24398 ***