[webkit-changes] [219863] trunk/Source/WebKit

[carlosgc]

Title: [219863] trunk/Source/WebKit

Revision

219863

Author

carlo...@webkit.org

Date

2017-07-24 23:49:11 -0700 (Mon, 24 Jul 2017)

Log Message

[GTK] ASSERTION FAILED: client in WebKit::IconDatabase::setClient
https://bugs.webkit.org/show_bug.cgi?id=174788

Patch by Michael Catanzaro <mcatanz...@igalia.com> on 2017-07-24
Reviewed by Carlos Garcia Campos.

IconDatabase ensures that setClient is never called with nullptr, but WebKitFaviconDatabase
does exactly that. First, ensure it's safe to unset the client. Next, we can either remove
the assertions from setClient or else automatically unset the client in IconDatabase::close.
I opted to do both.

* UIProcess/API/glib/IconDatabase.cpp:
(WebKit::IconDatabase::close):
(WebKit::IconDatabase::setIconDataForIconURL):
(WebKit::IconDatabase::setIconURLForPageURL):
* UIProcess/API/glib/WebKitFaviconDatabase.cpp:
(_WebKitFaviconDatabasePrivate::~_WebKitFaviconDatabasePrivate): Deleted.
(webkitFaviconDatabaseDispose): Deleted.

Modified Paths

• trunk/Source/WebKit/ChangeLog
• trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp
• trunk/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp

Diff

Modified: trunk/Source/WebKit/ChangeLog (219862 => 219863)

--- trunk/Source/WebKit/ChangeLog	2017-07-25 06:45:53 UTC (rev 219862)
+++ trunk/Source/WebKit/ChangeLog	2017-07-25 06:49:11 UTC (rev 219863)
@@ -1,5 +1,25 @@
 2017-07-24  Michael Catanzaro  <mcatanz...@igalia.com>
 
+        [GTK] ASSERTION FAILED: client in WebKit::IconDatabase::setClient
+        https://bugs.webkit.org/show_bug.cgi?id=174788
+
+        Reviewed by Carlos Garcia Campos.
+
+        IconDatabase ensures that setClient is never called with nullptr, but WebKitFaviconDatabase
+        does exactly that. First, ensure it's safe to unset the client. Next, we can either remove
+        the assertions from setClient or else automatically unset the client in IconDatabase::close.
+        I opted to do both.
+
+        * UIProcess/API/glib/IconDatabase.cpp:
+        (WebKit::IconDatabase::close):
+        (WebKit::IconDatabase::setIconDataForIconURL):
+        (WebKit::IconDatabase::setIconURLForPageURL):
+        * UIProcess/API/glib/WebKitFaviconDatabase.cpp:
+        (_WebKitFaviconDatabasePrivate::~_WebKitFaviconDatabasePrivate): Deleted.
+        (webkitFaviconDatabaseDispose): Deleted.
+
+2017-07-24  Michael Catanzaro  <mcatanz...@igalia.com>
+
         [GTK][WPE] Remove useless conditional in IconDatabase::setIconDataForIconURL
         https://bugs.webkit.org/show_bug.cgi?id=174792
 

Modified: trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp (219862 => 219863)

--- trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp	2017-07-25 06:45:53 UTC (rev 219862)
+++ trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp	2017-07-25 06:49:11 UTC (rev 219863)
@@ -191,10 +191,8 @@
 
 void IconDatabase::setClient(std::unique_ptr<IconDatabaseClient>&& client)
 {
-    // We don't allow a null client, because we never null check it anywhere in this code
-    // Also don't allow a client change after the thread has already began
+    // Don't allow a client change after the thread has already began
     // (setting the client should occur before the database is opened)
-    ASSERT(client);
     ASSERT(!m_syncThreadRunning);
     if (!client || m_syncThreadRunning)
         return;
@@ -257,6 +255,8 @@
     // But if it is closed, notify the client now.
     if (!isOpen() && m_client)
         m_client->didClose();
+
+    m_client = nullptr;
 }
 
 void IconDatabase::removeAllIcons()
@@ -586,7 +586,8 @@
 
     for (auto& pageURL : pageURLs) {
         LOG(IconDatabase, "Dispatching notification that retaining pageURL %s has a new icon", urlForLogging(pageURL).ascii().data());
-        m_client->didChangeIconForPageURL(pageURL);
+        if (m_client)
+            m_client->didChangeIconForPageURL(pageURL);
     }
 }
 
@@ -653,7 +654,8 @@
         scheduleOrDeferSyncTimer();
 
         LOG(IconDatabase, "Dispatching notification that we changed an icon mapping for url %s", urlForLogging(pageURL).ascii().data());
-        m_client->didChangeIconForPageURL(pageURL);
+        if (m_client)
+            m_client->didChangeIconForPageURL(pageURL);
     }
 }
 

Modified: trunk/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp (219862 => 219863)

--- trunk/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp	2017-07-25 06:45:53 UTC (rev 219862)
+++ trunk/Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp	2017-07-25 06:49:11 UTC (rev 219863)
@@ -70,11 +70,6 @@
 typedef HashMap<String, PendingIconRequestVector*> PendingIconRequestMap;
 
 struct _WebKitFaviconDatabasePrivate {
-    ~_WebKitFaviconDatabasePrivate()
-    {
-        iconDatabase->setClient(nullptr);
-    }
-
     std::unique_ptr<IconDatabase> iconDatabase;
     Vector<std::pair<String, Function<void(bool)>>> pendingLoadDecisions;
     PendingIconRequestMap pendingIconRequests;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes