Original report at http://crbug.com/42212 If an iframe is set to a url that gets downloaded, the frame loader never commits to the load and never sets m_URL (since it never actually performed a navigation). If we try to save the page, we get to WebPageSerializerImpl::serialize() and try to do operations on each of the urls in turn, reach the invalid url of the downloaded iframe, and crash. There are two ways to fix this: 1. Check the url in WebPageSerializerImpl before using it 2. Ensure FrameLoader::m_url is always valid. I'm inclined to go with (1), since this crash appears to be occurring in Chromium only.
Created attachment 55044 [details] patch
Comment on attachment 55044 [details] patch Please add a test. I think we have the ability to unit test the Chromium WebKit API.
A test that covers this functionality would need to bring up a full WebFrame and be able to exercise it. I don't believe that is possible with the testing tools currently available for the Chromium API in webkit.org (I think it requires the tools chromium has in test_shell_tests). If you would like me to write a unit test, I'm happy to do so, but I think it would need to be in src.chromium.org for now.
Comment on attachment 55044 [details] patch Resetting r? for the reason outlined in my previous post (a test that covers this functionality would need to live in the chromium repo at the moment).
(In reply to comment #4) > (From update of attachment 55044 [details]) > Resetting r? for the reason outlined in my previous post (a test that covers > this functionality would need to live in the chromium repo at the moment). FYI, the test for this patch is up for review at http://codereview.chromium.org/1917007
Comment on attachment 55044 [details] patch Needs tests or explanation of why testing is impossible.
(In reply to comment #6) > (From update of attachment 55044 [details]) > Needs tests or explanation of why testing is impossible. In the changelog, right? Because I think Nate provided a pretty good explanation in the bug.
Yes, in the ChangeLog. I don't (and I guess Adam doesn't either) read bugs during most reviews (except to validate that no one else has objected).
Created attachment 55388 [details] patch + updated changelog
Comment on attachment 55388 [details] patch + updated changelog ok
http://trac.webkit.org/changeset/59169 might have broken Chromium Win Release The following changes are on the blame list: http://trac.webkit.org/changeset/59168 http://trac.webkit.org/changeset/59169 http://trac.webkit.org/changeset/59170 http://trac.webkit.org/changeset/59165 http://trac.webkit.org/changeset/59166 http://trac.webkit.org/changeset/59167
http://trac.webkit.org/changeset/59169