Bug 21026 - No favicon during first load
Summary: No favicon during first load
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2008-09-23 09:20 PDT by Patrick
Modified: 2009-01-29 08:02 PST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick 2008-09-23 09:20:33 PDT
Safari -> Reset Safari...

Go to a site known to have a favicon (google.com, yahoo.com, ebay.com, etc). Notice there is no favicon until you refresh the site. I turned on logging and I think the issue is that the favicon data has not been committed to the database in dispatchDidReceiveIcon and dispatchDidAddIconForPageURL so that the Image stored in the IconRecord has m_data set to NULL.
Comment 1 Mark Rowe (bdash) 2008-09-23 14:14:06 PDT
<rdar://problem/6240826>
Comment 2 Patrick 2008-10-07 06:44:19 PDT
I have a local change that switches the order of readFromDatabase() and writeToDatabase() inside IconDatabase::syncThreadMainLoop() that appears to fix the issue. I haven't seen any adverse affects but I'm not sure what to look for.
Comment 3 David Kilzer (:ddkilzer) 2008-12-10 13:31:18 PST
See also Bug 12892.

Comment 4 David Kilzer (:ddkilzer) 2008-12-10 13:55:45 PST
I believe Attachment #25925 [details] from Bug 22795 contains a (webarchive) test case that reproduces this issue reliably on Mac OS X, which should make it easier to fix.

Comment 5 Darin Adler 2008-12-25 10:17:23 PST
Could this be related to the "write before read" change in bug 22963?
Comment 6 David Kilzer (:ddkilzer) 2008-12-29 16:18:44 PST
(In reply to comment #5)
> Could this be related to the "write before read" change in bug 22963?

I don't believe that patch fixes the issue.  See Bug 22963 Comment #3.
Comment 7 David Kilzer (:ddkilzer) 2009-01-14 14:18:07 PST
(In reply to comment #4)
> I believe Attachment #25925 [details] [review] from Bug 22795 contains a (webarchive) test case
> that reproduces this issue reliably on Mac OS X, which should make it easier to
> fix.

The fix for Bug 22795 landed, but the layout test fails in different ways on different buildbots (see Bug 23331), so it's currently disabled.
Comment 8 David Kilzer (:ddkilzer) 2009-01-28 19:49:54 PST
I think this may have been fixed with <http://trac.webkit.org/changeset/40335>.

Patrick, could you verify this with a nightly build?
Comment 9 Patrick 2009-01-29 05:28:39 PST
I think change 40335 and the fix for bug 22963 have resolved this issue.
Comment 10 David Kilzer (:ddkilzer) 2009-01-29 08:02:00 PST
(In reply to comment #9)
> I think change 40335 and the fix for bug 22963 have resolved this issue.

Resolving as fixed.