Changeset 25557 in webkit

Timestamp:

Sep 13, 2007 6:46:54 PM (17 years ago)

Author:

beidson

Message:

Reviewed by Maciej

<rdar://problem/5480437> - No site icon at launch and related error messages

The error message was actually indicative of a larger bug that might've resulted in icons getting
improperly pruned because they were never added to the set of retained page URLs.

To solve the no-icon-at-launch problem, we send the "didReceiveIcon:" delegate call for every page
load that has an icon, whether the icon comes in from network, from disk, or was already in ram

• loader/FrameLoader.cpp: (WebCore::FrameLoader::startIconLoader): Always send the didReceiveIcon delegate call when an icon's image data is known

• loader/icon/IconDatabase.cpp: (WebCore::IconDatabase::retainIconForPageURL): Much more accurately track the set of retained pages by adding them when their retain count moves from 0 to 1

• loader/icon/PageURLRecord.h: (WebCore::PageURLRecord::retain): Correctly distinguish the "retain count just went from 0 to 1" case in the return value

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/FrameLoader.cpp (modified) (2 diffs)
• loader/icon/IconDatabase.cpp (modified) (2 diffs)
• loader/icon/PageURLRecord.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-09-13  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej
+
+        <rdar://problem/5480437> - No site icon at launch and related error messages
+
+        The error message was actually indicative of a larger bug that might've resulted in icons getting
+        improperly pruned because they were never added to the set of retained page URLs.
+
+        To solve the no-icon-at-launch problem, we send the "didReceiveIcon:" delegate call for every page 
+        load that has an icon, whether the icon comes in from network, from disk, or was already in ram
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::startIconLoader): Always send the didReceiveIcon delegate call when an icon's 
+          image data is known
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::retainIconForPageURL): Much more accurately track the set of retained pages by
+          adding them when their retain count moves from 0 to 1
+
+        * loader/icon/PageURLRecord.h:
+        (WebCore::PageURLRecord::retain): Correctly distinguish the "retain count just went from 0 to 1" case
+          in the return value
+
 2007-09-13  Brady Eidson  <beidson@apple.com>
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -1062 +1062 @@
             // If the icon data hasn't been read in from disk yet, kick off the read of the icon from the database to make sure someone
             // has done it.  This is after registering for the notification so the WebView can call the appropriate delegate method.
-            // Otherwise if the icon data *is* available, and this icon load was previously deferred, we need to notifiy the delegate
+            // Otherwise if the icon data *is* available, notify the delegate
             if (!iconDatabase()->iconDataKnownForIconURL(urlString)) {
                 LOG(IconDatabase, "Told not to load icon %s but icon data is not yet available - registering for notification and requesting load from disk", urlString.ascii().data());
@@ -1068 +1068 @@
                 iconDatabase()->iconForPageURL(m_URL.url(), IntSize(0, 0));
                 iconDatabase()->iconForPageURL(originalRequestURL().url(), IntSize(0, 0));
-            } else if (m_mayLoadIconLater)
+            } else
                 m_client->dispatchDidReceiveIcon();
                 

trunk/WebCore/loader/icon/IconDatabase.cpp

@@ -441 +441 @@
         record = new PageURLRecord(pageURL);
         m_pageURLToRecordMap.set(pageURL, record);
+    }
+    
+    if (!record->retain()) {
+        if (pageURL.isNull())
+            pageURL = pageURLOriginal.copy();
+
+        // This page just had its retain count bumped from 0 to 1 - Record that fact
         m_retainedPageURLs.add(pageURL);
-    }
-    
-    if (record->retain()) {
+
         // If we read the iconURLs yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot, 
         // so we bail here and skip those steps
@@ -453 +458 @@
         // If this pageURL waiting to be sync'ed, update the sync record
         // This saves us in the case where a page was ready to be deleted from the database but was just retained - so theres no need to delete it!
-        if (!m_privateBrowsingEnabled && m_pageURLsPendingSync.contains(pageURLOriginal)) {
-            if (pageURL.isNull())
-                pageURL = pageURLOriginal.copy();
-        
+        if (!m_privateBrowsingEnabled && m_pageURLsPendingSync.contains(pageURL)) {
             LOG(IconDatabase, "Bringing %s back from the brink", pageURL.utf8().data());
             m_pageURLsPendingSync.set(pageURL, record->snapshot());

trunk/WebCore/loader/icon/PageURLRecord.h

@@ -63 +63 @@
     PageURLSnapshot snapshot(bool forDeletion = false) const;
 
-    inline bool retain() { return ++m_retainCount; }
+    // Returns false if the page wasn't retained beforehand, true if the retain count was already 1 or higher
+    inline bool retain() { return m_retainCount++; }
 
+    // Returns true if the page is still retained after the call.  False if the retain count just dropped to 0
     inline bool release()
     {