[Webkit-unassigned] [Bug 88665] New: 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

           Summary: Favicon URL list sent with favicon updated message
                    contains out of date icon URLs
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: All
        OS/Version: All
            Status: UNCONFIRMED
          Severity: Minor
          Priority: P2
         Component: Images
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: petewil at chromium.org
                CC: dimich at chromium.org, groby at chromium.org


Overview:  If the favicon in the HTML header is changed via DOM manipulation, then webkit will send a notification that the favicon changed which contains a list of favicon URLs that still includes the previous favicon, even though it is no longer in the DOM.


Steps to Reproduce: 
1. Load a page with a favicon
2. Send an event to the page that causes its JS to change the favicon to a different one
3. Send a second event to the page to cause the JS to change the favicon again


Actual Results: 
The code handling the favicon change gets a list of icon URLs with the event which contains both the favicon from step 2 and the favicon from step 3

Expected Results: 
The code handling the favicon change should get a list of icon URLs without the favicon from step 2

Build Date & Platform:
Top of tree as of June 6, 2012.  Mac

Additional Builds and Platforms: 
The bug also occurs on PC and Linux


Additional Information: 
We build a list of URLs to pass back to handle multiple kinds of favicons, such as touch favicons for running in a touch screen environment, and regular favicons for a non touch environment.  It could potentially contain links to favicons of different sizes.  We keep a list of icon urls (m_iconURLs) and we add every new icon we see to the list, but we never remove icons for a particular page.

One potential approach we could take is to ensure that icons get removed when they are removed from the DOM, but this encounters a problem.  Presumably we will eliminate duplicates when creating the list.  Suppose we have two identical Icon URLs in the list, and we remove one when the DOM changes.  Without adding some kind of refcount scheme, we would end up removing both references.  This approach is workable, but complicated and difficult to maintain.

A second approach is to re-build the list by walking the DOM every time the list is requested.  (I have a sample patch showing this approach which I'll submit for review).  We have to consider that the second approach may be slower.

Also, while investigating I discovered that we end up with many copies of the same icon URL in the list (we add one every time the api to get them gets called).  My patch will also address this.

There is more discussion on this issue in chromium bug 125806 where we discuss the alternatives for fixing the bug in more detail.

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