WebCore:
authorbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2006 07:16:10 +0000 (07:16 +0000)
committerbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2006 07:16:10 +0000 (07:16 +0000)
        Reviewed by Sarge Decker

        <rdar://problem/4739892> and <rdar://problem/4729797>
        - WebCore::IconDatabase needs to have and respect an enabled() flag
        - Mail on ToT WebKit crashes in IconDatabase code when mailing a page from Safari

        * bridge/mac/WebCoreIconDatabaseBridge.h:
        * bridge/mac/WebCoreIconDatabaseBridge.mm:
        (-[WebCoreIconDatabaseBridge _setEnabled:]): Added
        (-[WebCoreIconDatabaseBridge _isEnabled]): Added
        * loader/icon/IconDatabase.cpp:
        (WebCore::IconDatabase::IconDatabase):
        (WebCore::IconDatabase::open): Don't open if disabled
        (WebCore::IconDatabase::removeAllIcons): Ignore if disabled/closed
        (WebCore::IconDatabase::setPrivateBrowsingEnabled): Ignore if disabled/closed
        (WebCore::IconDatabase::iconForPageURL): Default Icon if disabled/closed
        (WebCore::IconDatabase::isIconExpiredForIconURL): Default return if disabled/closed
        (WebCore::IconDatabase::iconURLForPageURL): Default return if disabled/closed
        (WebCore::IconDatabase::retainIconForPageURL): Ignore if disabled/closed
        (WebCore::IconDatabase::releaseIconForPageURL): Ignore if disabled/closed
        (WebCore::IconDatabase::releaseIconURL):
        (WebCore::IconDatabase::setIconDataForIconURL): Ignore if disabled/closed
        (WebCore::IconDatabase::setIconURLForPageURL): Ignore if disabled/closed
        (WebCore::IconDatabase::hasEntryForIconURL): Default return if disabled/closed
        (WebCore::IconDatabase::setEnabled): Added
        * loader/icon/IconDatabase.h:
        (WebCore::IconDatabase::enabled): Added
        * page/Frame.cpp:
        (WebCore::Frame::endIfNotLoading): do an IconDatabase::enabled() check before bothering to load the icon
        * platform/mac/ResourceLoaderMac.mm: Removed extraneous #include

WebKit:

        Reviewed by Sarge Decker

        <rdar://problem/4739892> and <rdar://problem/4729797>
        - WebCore::IconDatabase needs to have and respect an enabled() flag
        - Mail on ToT WebKit crashes in IconDatabase code when mailing a page from Safari

        * Misc/WebIconDatabase.m:
        (-[WebIconDatabase init]): If preference says icons are disabled, tell the bridge
        (-[WebIconDatabase _isEnabled]): Ask the bridge if the database is enabled

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16464 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bridge/mac/WebCoreIconDatabaseBridge.h
WebCore/bridge/mac/WebCoreIconDatabaseBridge.mm
WebCore/loader/icon/IconDatabase.cpp
WebCore/loader/icon/IconDatabase.h
WebCore/page/Frame.cpp
WebCore/platform/mac/ResourceLoaderMac.mm
WebKit/ChangeLog
WebKit/Misc/WebIconDatabase.m

index 0bc8358faaaca56033ccc80cb6ccf75cf2233052..c5ae54604de1facaf8aee34ae0398b892dec4112 100644 (file)
@@ -1,3 +1,36 @@
+2006-09-19  Brady Eidson <beidson@apple.com>
+
+        Reviewed by Sarge Decker
+
+        <rdar://problem/4739892> and <rdar://problem/4729797>
+        - WebCore::IconDatabase needs to have and respect an enabled() flag
+        - Mail on ToT WebKit crashes in IconDatabase code when mailing a page from Safari
+
+        * bridge/mac/WebCoreIconDatabaseBridge.h:
+        * bridge/mac/WebCoreIconDatabaseBridge.mm:
+        (-[WebCoreIconDatabaseBridge _setEnabled:]): Added
+        (-[WebCoreIconDatabaseBridge _isEnabled]): Added
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::IconDatabase):
+        (WebCore::IconDatabase::open): Don't open if disabled
+        (WebCore::IconDatabase::removeAllIcons): Ignore if disabled/closed
+        (WebCore::IconDatabase::setPrivateBrowsingEnabled): Ignore if disabled/closed
+        (WebCore::IconDatabase::iconForPageURL): Default Icon if disabled/closed
+        (WebCore::IconDatabase::isIconExpiredForIconURL): Default return if disabled/closed
+        (WebCore::IconDatabase::iconURLForPageURL): Default return if disabled/closed
+        (WebCore::IconDatabase::retainIconForPageURL): Ignore if disabled/closed
+        (WebCore::IconDatabase::releaseIconForPageURL): Ignore if disabled/closed
+        (WebCore::IconDatabase::releaseIconURL):
+        (WebCore::IconDatabase::setIconDataForIconURL): Ignore if disabled/closed
+        (WebCore::IconDatabase::setIconURLForPageURL): Ignore if disabled/closed
+        (WebCore::IconDatabase::hasEntryForIconURL): Default return if disabled/closed
+        (WebCore::IconDatabase::setEnabled): Added
+        * loader/icon/IconDatabase.h:
+        (WebCore::IconDatabase::enabled): Added
+        * page/Frame.cpp:
+        (WebCore::Frame::endIfNotLoading): do an IconDatabase::enabled() check before bothering to load the icon
+        * platform/mac/ResourceLoaderMac.mm: Removed extraneous #include
+
 2006-09-20  David Hyatt  <hyatt@apple.com>
 
         Massage mouse wheel handling so that it is more cross-platform.  Make
index e526a5a2f5371937c8f81c647459dfcb3caaec8e..fedceedb5e8e01b9226d8e5d6f6a71aef26208a2 100644 (file)
@@ -60,6 +60,8 @@ typedef WebCore::IconDatabase WebCoreIconDatabase;
 - (BOOL)_hasEntryForIconURL:(NSString *)iconURL;
 
 - (BOOL)_isEmpty;
+- (void)_setEnabled:(BOOL)enabled;
+- (BOOL)_isEnabled;
 @end
 
 // The WebCoreIconDatabaseBridge protocol contains methods for use by the WebCore side of the bridge.
index 65330591ffb1fdb1f2131ff03a627fffd184c1c6..8ebad809dd4aff6eb79bc2601a9d4bd1cd84d47b 100644 (file)
@@ -204,4 +204,17 @@ using namespace WebCore;
     return IconDatabase::defaultDatabaseFilename();
 }
 
+- (void)_setEnabled:(BOOL)enabled
+{
+    if (!_iconDB)
+        return;
+    _iconDB->setEnabled(enabled);
+}
+
+- (BOOL)_isEnabled
+{
+    return _iconDB ? _iconDB->enabled() : NO;
+}
+
+
 @end
index 5ece47c9cdddacd40258b84ac76d7009b07f1c98..1e29969b418887027f79e603ebb5ef4810d6345e 100644 (file)
@@ -83,6 +83,7 @@ IconDatabase::IconDatabase()
     , m_imageDataForIconURLStatement(0)
     , m_currentDB(&m_mainDB)
     , m_defaultIconDataCache(0)
+    , m_isEnabled(true)
     , m_privateBrowsingEnabled(false)
     , m_startupTimer(this, &IconDatabase::pruneUnretainedIconsOnStartup)
     , m_updateTimer(this, &IconDatabase::updateDatabase)
@@ -125,6 +126,9 @@ bool makeAllDirectories(const String& path)
 
 bool IconDatabase::open(const String& databasePath)
 {
+    if (!m_isEnabled)
+        return false;
+        
     if (isOpen()) {
         LOG_ERROR("Attempt to reopen the IconDatabase which is already open.  Must close it first.");
         return false;
@@ -200,6 +204,9 @@ void IconDatabase::close()
 
 void IconDatabase::removeAllIcons()
 {
+    if (!m_isEnabled || !isOpen())
+        return;
+        
     // We don't need to sync anything anymore since we're wiping everything.  
     // So we can kill the update timer, and clear all the hashes of "items that need syncing"
     m_updateTimer.stop();
@@ -328,6 +335,8 @@ void IconDatabase::imageDataForIconURL(const String& iconURL, Vector<unsigned ch
 
 void IconDatabase::setPrivateBrowsingEnabled(bool flag)
 {
+    if (!m_isEnabled || !isOpen())
+        return;
     if (m_privateBrowsingEnabled == flag)
         return;
     
@@ -347,6 +356,9 @@ void IconDatabase::setPrivateBrowsingEnabled(bool flag)
 
 Image* IconDatabase::iconForPageURL(const String& pageURL, const IntSize& size, bool cache)
 {   
+    if (!m_isEnabled || !isOpen())
+        return defaultIcon(size);
+        
     // See if we even have an IconURL for this PageURL...
     String iconURL = iconURLForPageURL(pageURL);
     if (iconURL.isEmpty())
@@ -370,8 +382,11 @@ Image* IconDatabase::iconForPageURL(const String& pageURL, const IntSize& size,
 // iconExpirationTime to present icons, and missingIconExpirationTime for missing icons
 bool IconDatabase::isIconExpiredForIconURL(const String& iconURL)
 {
-    if (iconURL.isEmpty()) 
-        return true;
+    // If we're disabled and someone is making this call, it is likely a return value of 
+    // false will discourage them to take any further action, which is our goal in this case
+    // Same notion for an empty iconURL - which is now defined as "never expires"
+    if (!m_isEnabled || !isOpen() || iconURL.isEmpty())
+        return false;
     
     // If we have a IconDataCache, then it definitely has the Timestamp in it
     IconDataCache* icon = m_iconURLToIconDataCacheMap.get(iconURL);
@@ -395,7 +410,7 @@ bool IconDatabase::isIconExpiredForIconURL(const String& iconURL)
     
 String IconDatabase::iconURLForPageURL(const String& pageURL)
 {    
-    if (pageURL.isEmpty()) 
+    if (!m_isEnabled || !isOpen() || pageURL.isEmpty())
         return String();
         
     if (m_pageURLToIconURLMap.contains(pageURL))
@@ -429,7 +444,7 @@ Image* IconDatabase::defaultIcon(const IntSize& size)
 
 void IconDatabase::retainIconForPageURL(const String& pageURL)
 {
-    if (pageURL.isEmpty())
+    if (!m_isEnabled || !isOpen() || pageURL.isEmpty())
         return;
     
     // If we don't have the retain count for this page, we need to setup records of its retain
@@ -471,7 +486,7 @@ void IconDatabase::retainIconForPageURL(const String& pageURL)
 
 void IconDatabase::releaseIconForPageURL(const String& pageURL)
 {
-    if (pageURL.isEmpty())
+    if (!m_isEnabled || !isOpen() || pageURL.isEmpty())
         return;
         
     // Check if this pageURL is actually retained
@@ -535,7 +550,7 @@ void IconDatabase::retainIconURL(const String& iconURL)
 void IconDatabase::releaseIconURL(const String& iconURL)
 {
     ASSERT(!iconURL.isEmpty());
-    
+        
     // If the iconURL has no retain count, we can bail
     if (!m_iconURLToRetainCount.contains(iconURL))
         return;
@@ -628,13 +643,13 @@ IconDataCache* IconDatabase::getOrCreateIconDataCache(const String& iconURL)
 void IconDatabase::setIconDataForIconURL(const void* data, int size, const String& iconURL)
 {
     ASSERT(size > -1);
+    if (!m_isEnabled || !isOpen() || iconURL.isEmpty())
+        return;
+
     if (size)
         ASSERT(data);
     else
         data = 0;
-        
-    if (iconURL.isEmpty())
-        return;
     
     // Get the IconDataCache for this IconURL (note, IconDataCacheForIconURL will create it if necessary)
     IconDataCache* icon = getOrCreateIconDataCache(iconURL);
@@ -657,7 +672,7 @@ void IconDatabase::setHaveNoIconForIconURL(const String& iconURL)
 bool IconDatabase::setIconURLForPageURL(const String& iconURL, const String& pageURL)
 {
     ASSERT(!iconURL.isEmpty());
-    if (pageURL.isEmpty())
+    if (!m_isEnabled || !isOpen() || pageURL.isEmpty())
         return false;
     
     // If the urls already map to each other, bail.
@@ -871,7 +886,7 @@ void IconDatabase::checkForDanglingPageURLs(bool pruneIfFound)
 
 bool IconDatabase::hasEntryForIconURL(const String& iconURL)
 {
-    if (iconURL.isEmpty())
+    if (!m_isEnabled || !isOpen() || iconURL.isEmpty())
         return false;
         
     // First check the in memory mapped icons...
@@ -891,6 +906,13 @@ bool IconDatabase::hasEntryForIconURL(const String& iconURL)
     return false;
 }
 
+void IconDatabase::setEnabled(bool enabled)
+{
+    if (isOpen())
+        close();
+    m_isEnabled = enabled;
+}
+
 IconDatabase::~IconDatabase()
 {
     close();
index 91a84dc95878d8845aed1189761771e2d24cfbc3..02684973d46b00a20f8817886837253a4fecfcd0 100644 (file)
@@ -46,8 +46,6 @@ class SQLStatement;
 
 class IconDatabase
 {
-//TODO - SiteIcon is never to be used outside of IconDatabase, so make it an internal and remove the friendness
-friend class SiteIcon;
 public:
     static IconDatabase* sharedIconDatabase();
     ~IconDatabase();
@@ -80,6 +78,9 @@ public:
     
     // Returns true if the set actually took place, false if the mapping already existed
     bool setIconURLForPageURL(const String& iconURL, const String& pageURL);
+    
+    void setEnabled(bool enabled);
+    bool enabled() { return m_isEnabled; }
 
     static const String& defaultDatabaseFilename();
     
@@ -187,6 +188,7 @@ private:
     
     IconDataCache* m_defaultIconDataCache;
     
+    bool m_isEnabled;
     bool m_privateBrowsingEnabled;
     
     Timer<IconDatabase> m_startupTimer;
index af1fc25bb4241d0df0132a2cf3f20c7a0030e9b0..e87841f16ece2c11d1c3e4c07dbfe147d3761b3e 100644 (file)
@@ -765,13 +765,14 @@ void Frame::endIfNotLoading()
     if (tree()->parent())
         return;
         
-    // FIXME - <rdar://problem/4729797> - To honor #2, we need to add the isEnabled() flag to WebCore::IconDatabase
+    IconDatabase* sharedIconDatabase = IconDatabase::sharedIconDatabase();
+    if (!sharedIconDatabase->enabled())
+        return;
     
     String url(iconURL().url());
     if (url.isEmpty())
         return;
     
-    IconDatabase* sharedIconDatabase = IconDatabase::sharedIconDatabase();
     // If we already have an unexpired icon, we won't kick off a load but we *will* map the appropriate URLs to it
     if (sharedIconDatabase->hasEntryForIconURL(url) && !isLoadTypeReload() && !sharedIconDatabase->isIconExpiredForIconURL(url)) {
         commitIconURLToIconDatabase();
index e59f037f6486c87a7bcb58e98fed481124b12e79..5ae77e396e47926a761c1e736df116e101c50b6b 100644 (file)
@@ -34,7 +34,6 @@
 #import "KURL.h"
 #import "FormDataMac.h"
 #import "LoaderFunctions.h"
-#import "LoaderFunctions.h"
 #import "WebCoreResourceLoaderImp.h"
 #import "Logging.h"
 #import "WebCoreFrameBridge.h"
index 2930d021cf65e1c5ea3eb6ad21f34c5e1f0d02b2..97d4f04090fe5b788327d10bebb5cee57fe00558 100644 (file)
@@ -1,3 +1,15 @@
+2006-09-19  Brady Eidson <beidson@apple.com>
+
+        Reviewed by Sarge Decker
+
+        <rdar://problem/4739892> and <rdar://problem/4729797>
+        - WebCore::IconDatabase needs to have and respect an enabled() flag
+        - Mail on ToT WebKit crashes in IconDatabase code when mailing a page from Safari
+
+        * Misc/WebIconDatabase.m:
+        (-[WebIconDatabase init]): If preference says icons are disabled, tell the bridge
+        (-[WebIconDatabase _isEnabled]): Ask the bridge if the database is enabled
+
 2006-09-19  Alexey Proskuryakov  <ap@nypop.com>
 
         Reviewed by Tim O.
index 2406ef29c7807fa10be6f11c47694e5d34fe7892..6630b4b2ff7b9aaa9aeffd410703b284c3026540 100644 (file)
@@ -81,20 +81,21 @@ NSSize WebIconLargeSize = {128, 128};
     
     _private = [[WebIconDatabasePrivate alloc] init];
 
-    // Check the user defaults and see if the icon database should even be enabled if not, we can bail from init right here
+    // Get/create the shared database bridge - bail if we fail
+    _private->databaseBridge = [WebIconDatabaseBridge sharedInstance];
+    if (!_private->databaseBridge) {
+        LOG_ERROR("Unable to create IconDatabaseBridge");
+        return self;
+    }
+    
+    // Check the user defaults and see if the icon database should even be enabled.
+    // If not, inform the bridge and bail from init right here
     NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
     NSDictionary *initialDefaults = [[NSDictionary alloc] initWithObjectsAndKeys:[NSNumber numberWithBool:YES], WebIconDatabaseEnabledDefaultsKey, nil];
     [defaults registerDefaults:initialDefaults];
     [initialDefaults release];
     if (![defaults boolForKey:WebIconDatabaseEnabledDefaultsKey]) {
-        _private->databaseBridge = nil;
-        return self;
-    }
-        
-    // Get/create the shared database bridge - bail if we fail
-    _private->databaseBridge = [WebIconDatabaseBridge sharedInstance];
-    if (!_private->databaseBridge) {
-        LOG_ERROR("Unable to create IconDatabaseBridge");
+        [_private->databaseBridge _setEnabled:NO];
         return self;
     }
     
@@ -218,7 +219,7 @@ NSSize WebIconLargeSize = {128, 128};
 - (BOOL)_isEnabled
 {
     // If we weren't enabled on startup, we marked the databaseBridge as nil
-    return _private->databaseBridge != nil;
+    return [_private->databaseBridge _isEnabled];
 }
 
 - (void)_setIconData:(NSData *)data forIconURL:(NSString *)iconURL