Improve favicon fetching, fix leak

k-meleon/FavIconList.cpp

@@ -291,54 +291,54 @@ int CFavIconList::AddIcon(const char* uri, KmImage* img, const char* pageUri)
 	return index;
 }
 
-int CFavIconList::GetIcon(const TCHAR* aUrl)
+bool CFavIconList::GetFaviconForPage(nsIURI* aPageURI, nsIURI** _retval)
 {
-	int index = GetDefaultIcon();
-	if (m_urlMap.Lookup(aUrl, index))
-		return index + m_iOffset;
-
-	nsCOMPtr<nsIURI> URI;
-	nsCString nsUri = CStringToNSCString(aUrl);
-	nsresult rv = NewURI(getter_AddRefs(URI), nsUri);
-	if (NS_FAILED(rv)||!URI) return index;
+	nsCOMPtr<nsIFaviconService> fis = do_GetService("@mozilla.org/browser/favicon-service;1");
+	if (!fis) return false;
 
-	index = GetFavIcon(URI);
-	if (index != GetDefaultIcon()) 
-		return index;
+	nsCOMPtr<nsIURI> faviconURI;
+	nsresult rv = fis->GetFaviconForPage(aPageURI, getter_AddRefs(faviconURI));
+	if (NS_FAILED(rv)) faviconURI = aPageURI;
 
-	return GetHostIcon(aUrl);
+	NS_IF_ADDREF(*_retval = faviconURI);
+	return true;
 }
 
-int CFavIconList::GetHostIcon(const TCHAR* aUrl)
+int CFavIconList::GetIconForPage(const TCHAR* aUrl)
 {
 	int index = GetDefaultIcon();
 
-	const char* url;
-#ifdef _UNICODE
-	nsCString _str;
-	NS_UTF16ToCString(nsDependentString(aUrl), NS_CSTRING_ENCODING_UTF8, _str);
-	url = _str.get();
-#else
-	url = aUrl;
-#endif
+	nsCOMPtr<nsIFaviconService> fis = do_GetService("@mozilla.org/browser/favicon-service;1");
+	if (!fis) return index;
 
 	nsCOMPtr<nsIURI> URI;
-	nsCString nsUri;
-	nsUri.Assign(url);
-	nsresult rv = NewURI(getter_AddRefs(URI), nsUri);
+	nsCString spec = CStringToNSCString(aUrl);
+	nsresult rv = NewURI(getter_AddRefs(URI), spec);
 	if (NS_FAILED(rv)||!URI) return index;
-	URI->SetPath(NS_LITERAL_CSTRING(""));
-	URI->SetScheme(NS_LITERAL_CSTRING("http"));
-	URI->GetSpec(nsUri);
-	if (!m_urlMap.Lookup(NSCStringToCString(nsUri), index)) {		
-		return GetFavIcon(URI);
-	}
+	
+	nsCOMPtr<nsIURI> faviconURI;
+	rv = fis->GetFaviconForPage(URI, getter_AddRefs(faviconURI));
+	if (NS_FAILED(rv)) faviconURI = URI;
+	//NS_ENSURE_SUCCESS(rv, index);
+	
+	faviconURI->GetSpec(spec);
+	if (m_urlMap.Lookup(NSCStringToCString(spec), index))
+		return index + m_iOffset;
 
-	return index + m_iOffset;
+	uint32_t dataLen = 0;
+	uint8_t* data = 0;
+	nsCString mime;
+	rv = fis->GetFaviconData(faviconURI, mime, &dataLen, &data);
+	if (!NS_SUCCEEDED(rv) || !dataLen)
+		return index;//AddIcon(spec.get(), NULL, nullptr);	
+	
+	CComPtr<IStream> stream;
+	stream.Attach(SHCreateMemStream(data, dataLen));
+	KmImage img;
+	img.Load(stream);
+	return AddIcon(spec.get(), &img, nullptr);	
 }
 
-
-
 int CFavIconList::GetIcon(nsIURI *aURI, nsIURI* aPageURI, BOOL download)
 {
 	int index = GetDefaultIcon();
@@ -349,10 +349,12 @@ int CFavIconList::GetIcon(nsIURI *aURI, nsIURI* aPageURI, BOOL download)
 	aURI->GetSpec(nsUri);
 	USES_CONVERSION;
 
-	nsRefPtr<iconCallback> ic = new iconCallback(this, aURI, aPageURI);
-	nsCOMPtr<mozIAsyncFavicons> afis = GetIconService();
-	if (!afis) return FALSE;
-	nsresult rv = afis->SetAndFetchFaviconForPage(aPageURI, aURI, false, nsIFaviconService::FAVICON_LOAD_NON_PRIVATE, ic);
+	if (download) {
+		nsRefPtr<iconCallback> ic = new iconCallback(this, aURI, aPageURI);
+		nsCOMPtr<mozIAsyncFavicons> afis = GetIconService();
+		if (!afis) return FALSE;
+		nsresult rv = afis->SetAndFetchFaviconForPage(aPageURI, aURI, false, nsIFaviconService::FAVICON_LOAD_NON_PRIVATE, ic);
+	}
 
 	if (m_urlMap.Lookup(A2CT(nsUri.get()), index)) {
 		if (index != GetDefaultIcon())
@@ -451,6 +453,10 @@ int CFavIconList::GetFavIcon(nsIURI* iconURI)
 	nsCOMPtr<nsIFaviconService> fis = do_GetService("@mozilla.org/browser/favicon-service;1");
 	if (!fis) return 0;
 
+	nsCOMPtr<nsIURI> faviconURI;
+	if (NS_SUCCEEDED(fis->GetFaviconForPage(iconURI, getter_AddRefs(faviconURI))))
+		iconURI = faviconURI;
+
 	uint32_t dataLen = 0;
 	uint8_t* data = 0;
 	nsCString mime;
@@ -479,7 +485,8 @@ bool CFavIconList::DwnFavIcon(nsIURI* iconURI, nsIURI* pageURI, bool reload)
 	iconURI->GetScheme(scheme);
 
 	iconObserver* io = new iconObserver(this, iconURI, pageURI);
-	if (!nsImageObserver::LoadImage(io, iconURI)) {
+	if (!kImageObserver::LoadImage(io, iconURI)) {
+		delete io;
 		return false;
 	}
 	

k-meleon/FavIconList.h

@@ -42,6 +42,9 @@ private:
 	CImageList mSized;
 	nsCOMPtr<mozIAsyncFavicons> mIconService;	
 
+	int GetFavIcon(nsIURI* iconURI);
+	bool DwnFavIcon(nsIURI* iconURI, nsIURI* pageURI = NULL, bool reload = false);
+
 public:
 	CFavIconList();
 	virtual ~CFavIconList();
@@ -53,16 +56,14 @@ public:
 	//int AddIcon(const char* uri, HICON icon, const char* pageUri = nullptr);
 	int AddIcon(const char* uri, KmImage*, const char* pageUri = nullptr);
 
-	int GetHostIcon(const TCHAR* aUri);
-	int GetIcon(const TCHAR* uri);
+	bool CFavIconList::GetFaviconForPage(nsIURI* aPageURI, nsIURI** _retval);
+	int GetIconForPage(const TCHAR* uri);
 	int GetIcon(nsIURI* aUri, nsIURI* aPageURI = NULL, BOOL download = FALSE);
 	CImageList* GetSizedList() {return &mSized;}
 	void RefreshIcon(nsIURI* aURI);
 	void ResetCache();
 	void LoadDefaultIcon();
-	
-	int GetFavIcon(nsIURI* iconURI);
-	bool DwnFavIcon(nsIURI* iconURI, nsIURI* pageURI = NULL, bool reload = false);
+		
 	static void DwnCall(char* , TCHAR* , nsresult, void* );
 
 	inline int GetDefaultIcon() {return m_iDefaultIcon;}

k-meleon/KmImage.cpp

@@ -76,15 +76,13 @@ NS_IMETHODIMP nsImageObserver::Notify(imgIRequest *aProxy, int32_t aType, const
     return NS_OK;
 }
 
-void nsImageObserver::OnDownload(nsIURI* aUri, nsresult ns, LPSTREAM stream, LPCTSTR aName)
+void kImageObserver::OnDownload(nsIURI* aUri, nsresult ns, LPSTREAM stream, LPCTSTR aName)
 {
 	if (NS_SUCCEEDED(ns) && stream) {
 		KmImage img;
 		img.Load(stream);
 		mObserver->ImageLoaded(img);		
 	}
-	delete mObserver;
-	NS_RELEASE_THIS();
 }
 
 static void
@@ -542,7 +540,7 @@ bool KmImage::Load(LPSTREAM stream)
 			stream->CopyTo(ss, gnagna, &lread, &lwrit);
 
 			mGdiBitmap = Gdiplus::Bitmap::FromStream(stream);
-			stream->Release();
+			ss->Release();
 			return mGdiBitmap != nullptr;
 		}
 		free(pIconDir);
@@ -563,6 +561,8 @@ bool KmImage::Load(LPCTSTR path)
 	}
 
 	mGdiBitmap = Gdiplus::Bitmap::FromFile(path);
+	if (!mGdiBitmap) return false;
+
 	if (mGdiBitmap->GetLastStatus() != Gdiplus::Ok) {
 		delete mGdiBitmap;
 		mGdiBitmap = nullptr;

k-meleon/KmImage.h

@@ -26,27 +26,18 @@ class KmImage;
 
 interface IImageObserver {
  	virtual void ImageLoaded(KmImage& img) = 0;
+	virtual ~IImageObserver() {};
 };
 
 class nsImageObserver : 
 	public imgINotificationObserver, 
-	public nsSupportsWeakReference,
-	public IDownloadObserver
+	public nsSupportsWeakReference	
 {
 	NS_DECL_ISUPPORTS
 	NS_DECL_IMGINOTIFICATIONOBSERVER
 
-	nsImageObserver(IImageObserver* observer) : mObserver(observer), mNeedRelease(false) {}
-	virtual ~nsImageObserver() { }
-
+public:
 	static bool LoadImage(IImageObserver* observer, nsIURI* imgUri) 
-	{
-		nsImageObserver* obs = new nsImageObserver(observer);
-		NS_ADDREF(obs);
-		return DownloadToStream(imgUri, obs);
-	}
-
-	static bool LoadImageGecko(IImageObserver* observer, nsIURI* imgUri) 
 	{
 		nsresult rv;
 		nsCOMPtr<imgILoader> loader = do_GetService("@mozilla.org/image/loader;1", &rv);
@@ -58,13 +49,29 @@ class nsImageObserver :
 			nullptr, nullptr, getter_AddRefs(obs->mRequest)));
 	}
 
-	void OnDownload(nsIURI*, nsresult, LPSTREAM stream, LPCTSTR);
-
 protected:
-	IImageObserver* mObserver;
+	nsImageObserver(IImageObserver* observer) : mObserver(observer), mNeedRelease(false) {}
+	virtual ~nsImageObserver() { if (mObserver) delete mObserver; }
 	nsCOMPtr<imgIRequest> mRequest;
 	HBITMAP CreateDIB(imgIRequest *aRequest);
 	bool mNeedRelease;
+	IImageObserver* mObserver;	
+};
+
+class kImageObserver : public IDownloadObserver
+{
+public:
+	static bool LoadImage(IImageObserver* observer, nsIURI* imgUri) 
+	{
+		kImageObserver* obs = new kImageObserver(observer);
+		return DownloadToStream(imgUri, obs);
+	}	
+
+protected:
+	kImageObserver(IImageObserver* observer) : mObserver(observer) {}
+	virtual ~kImageObserver() {if (mObserver) delete mObserver; }
+	void OnDownload(nsIURI*, nsresult, LPSTREAM stream, LPCTSTR);
+	IImageObserver* mObserver;	
 };
 
 class KmImage
@@ -117,7 +124,7 @@ public:
 	bool LoadFromSkin(LPCTSTR name, LPRECT rect = nullptr, bool single = false);
 	bool LoadFromBitmap(HBITMAP hbmp, bool reverse = false);
 	bool LoadFromIcon(HICON hicon);
-
+	bool IsValid() { return mGdiBitmap!= nullptr; }
 	int AddToImageList(CImageList& list, int index = -1);
 	~KmImage() {
 		Clean();

k-meleon/KmSkin.cpp

@@ -403,7 +403,7 @@ int KmIconList::AddIcon(LPCTSTR coldImgPath, LPCTSTR hotImgPath, LPCTSTR deadImg
 		nsCOMPtr<nsIURI> uri;
 		NewURI(getter_AddRefs(uri), CStringToNSString(coldImgPath));
 
-		if (!nsImageObserver::LoadImage(io, uri)) {
+		if (!kImageObserver::LoadImage(io, uri)) {
 			delete io;
 			return -1;
 		}

k-meleon/MozUtils.cpp

@@ -557,7 +557,7 @@ BOOL GetBackgroundImageSrc(nsIDOMNode *aNode, CString& aUrl)
 	return true;
 }
 
-BOOL LogMessage(const char* category, const char* message, const char* file, uint line, uint flags)
+BOOL LogMessage(const char* category, const char* message, const char* file, uint32_t line, uint32_t flags)
 {
 	nsCOMPtr<nsIConsoleService> consoleService = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
 	if (!consoleService) FALSE;

k-meleon/MozUtils.h

@@ -41,6 +41,7 @@ BOOL LogMessage(const char* category, const char* message, const char* file, uin
 
 interface IDownloadObserver {
 	virtual void OnDownload(nsIURI*, nsresult, LPSTREAM, LPCTSTR) = 0;
+	virtual ~IDownloadObserver() {};
 };
 
 bool DownloadToStream(nsIURI* uri, IDownloadObserver*);
@@ -61,6 +62,7 @@ public:
 	}
 
 	virtual ~streamListener() { 
+		if (mObserver) delete mObserver;
 	}
 protected:
 	IDownloadObserver* mObserver;

k-meleon/Plugins.cpp

@@ -555,7 +555,8 @@ int SetMozillaSessionHistory (HWND hWnd, const char **titles, const char **urls,
 		view->GetBrowserGlue()->UpdateCurrentURI(uri);
 		view->GetBrowserGlue()->mPendingLocation = urls[index];
 		view->GetBrowserGlue()->mHIndex = index;
-		view->GetBrowserGlue()->mIcon = theApp.favicons.GetIcon(A2CW(urls[index]));
+		theApp.favicons.GetFaviconForPage(uri, getter_AddRefs(view->GetBrowserGlue()->mIconURI));
+		view->GetBrowserGlue()->mIcon = theApp.favicons.GetIconForPage(A2CW(urls[index]));
 		view->GetBrowserGlue()->SetBrowserTitle(NSUTF8StringToCString(nsDependentCString(titles[index])));
 		uri->SetPath(NS_LITERAL_CSTRING(""));
 		nsCString host;
@@ -1795,7 +1796,7 @@ int GetTabsList(HWND hWnd, HWND* list, unsigned size)
 int GetIconIdx(const char* host)
 {
 	USES_CONVERSION;
-	return theApp.favicons.GetIcon(A2CT(host));
+	return theApp.favicons.GetIconForPage(A2CT(host));
 }
 
 void ReleaseCmdID(UINT id)

k-meleon/urlbar.cpp

@@ -327,7 +327,7 @@ void CACListBox::DrawItem(LPDRAWITEMSTRUCT lpDrawItemStruct)
 	lpDrawItemStruct->rcItem.left += 2;
 	CImageList* iconList = theApp.favicons.GetSizedList();
 	int topMargin = (lpDrawItemStruct->rcItem.bottom - lpDrawItemStruct->rcItem.top - theApp.skin.GetDefHeight()) / 2;
-	iconList->Draw(&dc, theApp.favicons.GetIcon(strText), 
+	iconList->Draw(&dc, theApp.favicons.GetIconForPage(strText), 
 		CPoint(lpDrawItemStruct->rcItem.left, lpDrawItemStruct->rcItem.top + topMargin), 
 		ILD_TRANSPARENT);
 	lpDrawItemStruct->rcItem.left += theApp.skin.GetDefWidth() + 2;
@@ -850,7 +850,7 @@ void CUrlBar::OnCbenGetdispinfo(NMHDR *pNMHDR, LRESULT *pResult)
 	{
 		//url = W2T(pCBEx->ceItem.pszText); 
 		GetLBText(pCBEx->ceItem.iItem, url);
-		pCBEx->ceItem.iImage = pCBEx->ceItem.iSelectedImage = theApp.favicons.GetHostIcon(url);
+		pCBEx->ceItem.iImage = pCBEx->ceItem.iSelectedImage = theApp.favicons.GetIconForPage(url);
 	}
 	
 	*pResult = 0;