In the chromium port, when loading test_shell with a svg file, reload the "page" will crash test_shell.
Created attachment 28315 [details] proposed patch
The header for BackForwardList contains the following: #if PLATFORM(CHROMIUM) // Must be called before any other methods. void setClient(BackForwardListClient* client) { m_client = client; } #endif This implies that your code is using this class incorrectly. Of course, it may be that the existing design of the class is not ideal and it should be changed, but if that is the case the comment is no longer correct.
What is happening in that particular case is that SVGImage::dataChanged() creates a new dummy Page() which doesn't have any back-forward logic. I think null-initializing is fine in this case since ~Page() always calls m_backForwardList->close(). I can update the comment in BackForwardList.h for BackForwardListClient::setClient() to // Must be called before any other methods except close().
Comment on attachment 28315 [details] proposed patch >Index: WebCore/ChangeLog ... >+ Reviewed by NOBODY (OOPS!). >+ >+ Fix a crash when loading a non html page (like a svg file) in test_shell >+ and reloading the page. >+ >+ * history/BackForwardListChromium.cpp: please add a bug link to the ChangeLog. >Index: WebCore/history/BackForwardListChromium.cpp ... >+ if (m_client) >+ m_client->close(); we should probably null check each usage and probably add some ASSERTs. wdyt?
Created attachment 28418 [details] Fixed ChangeLog
I don't think it is a good idea to NULL check each functions and add ASSERTs. Having m_client null-initialized will help there and this was a very particular case. I looked at directly extracting m_client from m_page and it is very deep down multiple objects hierarchy.
Please don't use NULL in C++ code.
Comment on attachment 28418 [details] Fixed ChangeLog >Index: WebCore/history/BackForwardListChromium.cpp ... >+ , m_client(NULL) As mentioned, this should be 0 instead of NULL. Please post a revised patch, and then LGTM.
Created attachment 28587 [details] new patch file Replace NULL with 0.
Landed as http://trac.webkit.org/changeset/41824
*** Bug 24636 has been marked as a duplicate of this bug. ***