[Webkit-unassigned] [Bug 88665] Favicon URL list sent with favicon updated message contains out of date icon URLs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88665


Pete Williamson <petewil at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #146608|0                           |1
        is obsolete|                            |
 Attachment #147382|                            |review?
               Flag|                            |




--- Comment #4 from Pete Williamson <petewil at chromium.org>  2012-06-13 12:00:25 PST ---
Created an attachment (id=147382)
 --> (https://bugs.webkit.org/attachment.cgi?id=147382&action=review)
Patch with a proposed fix for the bug (adds unit tests, fixes style issue)

This extends the base patch, which fixes a bug with stale icon URLs in the document IconURL list.  Basically, if DOM manipulation replaced an icon URL with a newer one, we were keeping the older icon URL around even though it was no longer in the DOM.  There is also a fix for duplicates in the iconURL list, which the code had been creating before.

This patch adds a bunch of unit testing around the favicons to address the review comments that it has never before been a well tested feature.  

To do so, I had to extend the unit test framework.  First, I exposed two new methods on WebDocument to allow the tests to get to the underlying URL list maintained in the document class, and call the "reset" function.  If there is a better way than exposing them in the WebDocument, I'm all ears.

Second, I added some new methods to the LayoutTestController to allow the unit tests to request a dump of the iconURL list from the document.  I could add a second method to expose the recalculate URL list function, but I was trying to keep it minimal, let me know if anyone thinks it would be useful.

Then, I modified the TestShell class to dump the icon list (and call recalculate) when requested.

With all this done, I brought back the existing test for the icon properties, made it work, and removed it from the testExpectations file now that it passes reliably.  I then added two new tests.  One tests a list of icon URLs in the head element, the other tests changing the URL in the head element 3 times.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.