From d4abc27d67fcec3e6f9cf2c384bfba5375c8f486 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos Date: Mon, 14 Aug 2017 16:08:02 +0000 Subject: [PATCH] Merge r220694 - [GTK][WPE] Avoid emitting WebKitFaviconDatabase::favicon-changed multiple times while setting an icon https://bugs.webkit.org/show_bug.cgi?id=175531 Reviewed by Michael Catanzaro. When webkitFaviconDatabaseSetIconForPageURL() is called, both setIconURLForPageURL() and setIconDataForIconURL() might notify the client, which ends up emitting the WebKitFaviconDatabase::favicon-changed signal and calling webkitFaviconDatabaseSetIconURLForPageURL(). Both things are already done by webkitFaviconDatabaseSetIconForPageURL() itself, so we can just ignore the client notification while setting a new icon. * UIProcess/API/glib/WebKitFaviconDatabase.cpp: (webkitFaviconDatabaseSetIconURLForPageURL): Return early if isSettingIcon is true. (webkitFaviconDatabaseSetIconForPageURL): Set isSettingIcon to true for the scope. --- Source/WebKit/ChangeLog | 17 +++++++++++++++++ .../API/glib/WebKitFaviconDatabase.cpp | 8 ++++++++ 2 files changed, 25 insertions(+) diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index a8de23f24a6e..59d9512f3576 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,20 @@ +2017-08-14 Carlos Garcia Campos + + [GTK][WPE] Avoid emitting WebKitFaviconDatabase::favicon-changed multiple times while setting an icon + https://bugs.webkit.org/show_bug.cgi?id=175531 + + Reviewed by Michael Catanzaro. + + When webkitFaviconDatabaseSetIconForPageURL() is called, both setIconURLForPageURL() and setIconDataForIconURL() + might notify the client, which ends up emitting the WebKitFaviconDatabase::favicon-changed signal and calling + webkitFaviconDatabaseSetIconURLForPageURL(). Both things are already done by + webkitFaviconDatabaseSetIconForPageURL() itself, so we can just ignore the client notification while setting a + new icon. + + * UIProcess/API/glib/WebKitFaviconDatabase.cpp: + (webkitFaviconDatabaseSetIconURLForPageURL): Return early if isSettingIcon is true. + (webkitFaviconDatabaseSetIconForPageURL): Set isSettingIcon to true for the scope. + 2017-08-14 Carlos Garcia Campos [GTK][WPE] Crash in IconDatabase::IconRecord::setImageData() diff --git a/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp b/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp index 0cc94077e115..05d63fd4c4bd 100644 --- a/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp +++ b/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -75,6 +76,7 @@ struct _WebKitFaviconDatabasePrivate { PendingIconRequestMap pendingIconRequests; HashMap pageURLToIconURLMap; bool isURLImportCompleted; + bool isSettingIcon; }; WEBKIT_DEFINE_TYPE(WebKitFaviconDatabase, webkit_favicon_database, G_TYPE_OBJECT) @@ -193,6 +195,9 @@ static void webkitFaviconDatabaseSetIconURLForPageURL(WebKitFaviconDatabase* dat return; priv->pageURLToIconURLMap.set(pageURL, iconURL); + if (priv->isSettingIcon) + return; + g_signal_emit(database, signals[FAVICON_CHANGED], 0, pageURL.utf8().data(), iconURL.utf8().data()); } @@ -212,6 +217,8 @@ class WebKitIconDatabaseClient final : public IconDatabaseClient { void didChangeIconForPageURL(const String& pageURL) override { + if (m_database->priv->isSettingIcon) + return; String iconURL = m_database->priv->iconDatabase->synchronousIconURLForPageURL(pageURL); webkitFaviconDatabaseSetIconURLForPageURL(m_database, iconURL, pageURL); } @@ -305,6 +312,7 @@ void webkitFaviconDatabaseSetIconForPageURL(WebKitFaviconDatabase* database, con return; WebKitFaviconDatabasePrivate* priv = database->priv; + SetForScope change(priv->isSettingIcon, true); priv->iconDatabase->setIconURLForPageURL(icon.url.string(), pageURL); priv->iconDatabase->setIconDataForIconURL(SharedBuffer::create(iconData.bytes(), iconData.size()), icon.url.string()); webkitFaviconDatabaseSetIconURLForPageURL(database, icon.url.string(), pageURL);