- Revision
- 91540
- Author
- [email protected]
- Date
- 2011-07-21 17:40:30 -0700 (Thu, 21 Jul 2011)
Log Message
Local files cannot load icons.
https://bugs.webkit.org/show_bug.cgi?id=62459
Previous policy only allowed favicons for pages whose protocol was part of HTTP family.
Changed that to allow to any url that's not empty and whose protocol is not "about".
Also added this check where it attempts to start loading the favicon, so it can avoid
wasting time downloading a resource that won't be stored and won't be used.
Patch by Rafael Brandao <[email protected]> on 2011-07-21
Reviewed by Adam Barth.
Test: manual-tests/resources/favicon-loads-for-local-files.html
* loader/icon/IconController.cpp:
(WebCore::IconController::startLoader): Added check to avoid to request a favicon
when there's no way to store it.
* loader/icon/IconDatabase.cpp:
(WebCore::IconDatabase::documentCanHaveIcon): Renamed function "pageCanHaveIcon"
to reflect better which url we're handling.
(WebCore::IconDatabase::synchronousIconForPageURL): Ditto.
(WebCore::IconDatabase::synchronousIconURLForPageURL): Ditto.
(WebCore::IconDatabase::retainIconForPageURL): Ditto.
(WebCore::IconDatabase::releaseIconForPageURL): Ditto.
(WebCore::IconDatabase::setIconURLForPageURL): Ditto.
(WebCore::IconDatabase::getOrCreatePageURLRecord): Ditto.
(WebCore::IconDatabase::importIconURLForPageURL): Ditto.
(WebCore::IconDatabase::performURLImport): Ditto.
* loader/icon/IconDatabase.h:
* loader/icon/IconDatabaseBase.h:
(WebCore::IconDatabaseBase::documentCanHaveIcon): Added it as virtual to replace its
default behavior of not allowing favicons when we have IconDatabase enabled.
* manual-tests/resources/favicon-loads-for-local-files.html: Added.
* manual-tests/resources/favicon.png: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (91539 => 91540)
--- trunk/Source/WebCore/ChangeLog 2011-07-22 00:36:26 UTC (rev 91539)
+++ trunk/Source/WebCore/ChangeLog 2011-07-22 00:40:30 UTC (rev 91540)
@@ -1,3 +1,41 @@
+2011-07-21 Rafael Brandao <[email protected]>
+
+ Local files cannot load icons.
+ https://bugs.webkit.org/show_bug.cgi?id=62459
+
+ Previous policy only allowed favicons for pages whose protocol was part of HTTP family.
+ Changed that to allow to any url that's not empty and whose protocol is not "about".
+ Also added this check where it attempts to start loading the favicon, so it can avoid
+ wasting time downloading a resource that won't be stored and won't be used.
+
+ Reviewed by Adam Barth.
+
+ Test: manual-tests/resources/favicon-loads-for-local-files.html
+
+ * loader/icon/IconController.cpp:
+ (WebCore::IconController::startLoader): Added check to avoid to request a favicon
+ when there's no way to store it.
+
+ * loader/icon/IconDatabase.cpp:
+ (WebCore::IconDatabase::documentCanHaveIcon): Renamed function "pageCanHaveIcon"
+ to reflect better which url we're handling.
+
+ (WebCore::IconDatabase::synchronousIconForPageURL): Ditto.
+ (WebCore::IconDatabase::synchronousIconURLForPageURL): Ditto.
+ (WebCore::IconDatabase::retainIconForPageURL): Ditto.
+ (WebCore::IconDatabase::releaseIconForPageURL): Ditto.
+ (WebCore::IconDatabase::setIconURLForPageURL): Ditto.
+ (WebCore::IconDatabase::getOrCreatePageURLRecord): Ditto.
+ (WebCore::IconDatabase::importIconURLForPageURL): Ditto.
+ (WebCore::IconDatabase::performURLImport): Ditto.
+ * loader/icon/IconDatabase.h:
+ * loader/icon/IconDatabaseBase.h:
+ (WebCore::IconDatabaseBase::documentCanHaveIcon): Added it as virtual to replace its
+ default behavior of not allowing favicons when we have IconDatabase enabled.
+
+ * manual-tests/resources/favicon-loads-for-local-files.html: Added.
+ * manual-tests/resources/favicon.png: Added.
+
2011-07-21 Kulanthaivel Palanichamy <[email protected]>
Fix for bug 64046 - Wrong image height in absolutely positioned div in
Modified: trunk/Source/WebCore/loader/icon/IconController.cpp (91539 => 91540)
--- trunk/Source/WebCore/loader/icon/IconController.cpp 2011-07-22 00:36:26 UTC (rev 91539)
+++ trunk/Source/WebCore/loader/icon/IconController.cpp 2011-07-22 00:40:30 UTC (rev 91540)
@@ -113,6 +113,10 @@
if (!iconDatabase().isEnabled())
return;
+ ASSERT(!m_frame->tree()->parent());
+ if (!iconDatabase().documentCanHaveIcon(m_frame->document()->url()))
+ return;
+
KURL iconURL(url());
String urlString(iconURL.string());
if (urlString.isEmpty())
Modified: trunk/Source/WebCore/loader/icon/IconDatabase.cpp (91539 => 91540)
--- trunk/Source/WebCore/loader/icon/IconDatabase.cpp 2011-07-22 00:36:26 UTC (rev 91539)
+++ trunk/Source/WebCore/loader/icon/IconDatabase.cpp 2011-07-22 00:40:30 UTC (rev 91540)
@@ -99,9 +99,9 @@
return defaultClient;
}
-static inline bool pageCanHaveIcon(const String& pageURL)
+bool IconDatabase::documentCanHaveIcon(const String& documentURL) const
{
- return protocolIsInHTTPFamily(pageURL);
+ return !documentURL.isEmpty() && !protocolIs(documentURL, "about");
}
// ************************
@@ -223,7 +223,7 @@
// pageURLOriginal cannot be stored without being deep copied first.
// We should go our of our way to only copy it if we have to store it
- if (!isOpen() || !pageCanHaveIcon(pageURLOriginal))
+ if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
return 0;
MutexLocker locker(m_urlAndIconLock);
@@ -310,7 +310,7 @@
// Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
// Also, in the case we have a real answer for the caller, we must deep copy that as well
- if (!isOpen() || !pageCanHaveIcon(pageURLOriginal))
+ if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
return String();
MutexLocker locker(m_urlAndIconLock);
@@ -400,7 +400,7 @@
// Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
- if (!isEnabled() || !pageCanHaveIcon(pageURLOriginal))
+ if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
return;
MutexLocker locker(m_urlAndIconLock);
@@ -444,7 +444,7 @@
// Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
- if (!isEnabled() || !pageCanHaveIcon(pageURLOriginal))
+ if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
return;
MutexLocker locker(m_urlAndIconLock);
@@ -579,7 +579,7 @@
ASSERT(!iconURLOriginal.isEmpty());
- if (!isOpen() || !pageCanHaveIcon(pageURLOriginal))
+ if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
return;
String iconURL, pageURL;
@@ -889,7 +889,7 @@
// Clients of getOrCreatePageURLRecord() are required to acquire the m_urlAndIconLock before calling this method
ASSERT(!m_urlAndIconLock.tryLock());
- if (!pageCanHaveIcon(pageURL))
+ if (!documentCanHaveIcon(pageURL))
return 0;
PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURL);
@@ -927,7 +927,7 @@
// This function is only for setting actual existing url mappings so assert that neither of these URLs are empty
ASSERT(!iconURL.isEmpty());
ASSERT(!pageURL.isEmpty());
- ASSERT(pageCanHaveIcon(pageURL));
+ ASSERT(documentCanHaveIcon(pageURL));
setIconURLForPageURLInSQLDatabase(iconURL, pageURL);
}
@@ -1234,7 +1234,7 @@
// so go ahead and actually create a pageURLRecord for this url even though it's not retained.
// If database cleanup *is* allowed, we don't want to bother pulling in a page url from disk that noone is actually interested
// in - we'll prune it later instead!
- if (!pageRecord && databaseCleanupCounter && pageCanHaveIcon(pageURL)) {
+ if (!pageRecord && databaseCleanupCounter && documentCanHaveIcon(pageURL)) {
pageRecord = new PageURLRecord(pageURL);
m_pageURLToRecordMap.set(pageURL, pageRecord);
}
Modified: trunk/Source/WebCore/loader/icon/IconDatabase.h (91539 => 91540)
--- trunk/Source/WebCore/loader/icon/IconDatabase.h 2011-07-22 00:36:26 UTC (rev 91539)
+++ trunk/Source/WebCore/loader/icon/IconDatabase.h 2011-07-22 00:40:30 UTC (rev 91540)
@@ -136,6 +136,7 @@
public:
virtual bool isOpen() const;
virtual String databasePath() const;
+ virtual bool documentCanHaveIcon(const String& documentURL) const;
static String defaultDatabaseFilename();
private:
Modified: trunk/Source/WebCore/loader/icon/IconDatabaseBase.h (91539 => 91540)
--- trunk/Source/WebCore/loader/icon/IconDatabaseBase.h 2011-07-22 00:36:26 UTC (rev 91539)
+++ trunk/Source/WebCore/loader/icon/IconDatabaseBase.h 2011-07-22 00:40:30 UTC (rev 91540)
@@ -165,6 +165,7 @@
// Used internally by WebCore
virtual bool isEnabled() const { return false; }
+ virtual bool documentCanHaveIcon(const String&) const { return false; }
virtual void retainIconForPageURL(const String&) { }
virtual void releaseIconForPageURL(const String&) { }
Added: trunk/Source/WebCore/manual-tests/resources/favicon-loads-for-local-files.html (0 => 91540)
--- trunk/Source/WebCore/manual-tests/resources/favicon-loads-for-local-files.html (rev 0)
+++ trunk/Source/WebCore/manual-tests/resources/favicon-loads-for-local-files.html 2011-07-22 00:40:30 UTC (rev 91540)
@@ -0,0 +1,10 @@
+<html>
+<head>
+</head>
+<link type="image/png" href="" sizes="64x64" rel="icon" />
+<body>
+<p>It's expected that you see a favicon displayed for this page when you open it as a local file.</p>
+<p>The favicon looks like this:</p>
+<img src=""
+</body>
+</html>
Added: trunk/Source/WebCore/manual-tests/resources/favicon.png (0 => 91540)
--- trunk/Source/WebCore/manual-tests/resources/favicon.png (rev 0)
+++ trunk/Source/WebCore/manual-tests/resources/favicon.png 2011-07-22 00:40:30 UTC (rev 91540)
@@ -0,0 +1,35 @@
+\x89PNG
+
+
+IHDR 0 0 W\xF9\x87 bKGD \xFE \xFE \xFE\xEBԂ pHYs H H F\xC9k>