[webkit-reviews] review requested: [Bug 88665] Favicon URL list sent with favicon updated message contains out of date icon URLs : [Attachment 152323] Patch with a proposed fix for the bug (with layout tests, more recently synced, no extra files, better style, fixed CR comments, BrowserTests pass)

[bugzilla-daemon at webkit.org]

Pete Williamson <petewil at chromium.org> has asked  for review:
Bug 88665: Favicon URL list sent with favicon updated message contains out of
date icon URLs
https://bugs.webkit.org/show_bug.cgi?id=88665

Attachment 152323: Patch with a proposed fix for the bug (with layout tests,
more recently synced, no extra files, better style, fixed CR comments,
BrowserTests pass)
https://bugs.webkit.org/attachment.cgi?id=152323&action=review

------- Additional Comments from Pete Williamson <petewil at chromium.org>
The previous attempt passed the commit queue and built fine, but made some
chromium tests fail, and some Qt tests fail.

We fixed the Qt test crashes by making sure the check the head() pointer for
null in Document.cpp before de-referencing it.	We noticed the crash was in the
call to children, which would crash if the document had no html HEAD tag, so we
think this will fix the Qt crashes.

We fixed the browser tests by noticing that the previous check in Document.cpp
in addIconURLs was too restrictive - we only accepted the node as a favicon if
the mime type was set to image/x-icon, but it is legitimate for the mime type
to not be set at all on a favicon, so we removed the check for image type.  The
browser tests now all succeed.

Those are the only two changes since the last patch, both in
Document::addIconURLs