Changeset 219861 in webkit

Timestamp:

Jul 24, 2017 11:39:30 PM (7 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] Icon database error and crash
​https://bugs.webkit.org/show_bug.cgi?id=174760

Reviewed by Michael Catanzaro.

The crash is a debug ASSERT that happens when the IconRecord image is created in one thread and destroyed in
another one. IconDatabase creates and destroys IconRecord objects in both database and main thread. The
IconRecord is destroyed when the icon is no longer retained, and we only release icons when we fail to get the
image data (including pages that don't have a favicon). We can prevent this crash from happening if we ensure we
never create an Image for an IconRecord when the given image data is nullptr.

• UIProcess/API/glib/IconDatabase.cpp:

(WebKit::IconDatabase::IconRecord::setImageData):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/glib/IconDatabase.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-07-24  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Icon database error and crash
+        https://bugs.webkit.org/show_bug.cgi?id=174760
+
+        Reviewed by Michael Catanzaro.
+
+        The crash is a debug ASSERT that happens when the IconRecord image is created in one thread and destroyed in
+        another one. IconDatabase creates and destroys IconRecord objects in both database and main thread. The
+        IconRecord is destroyed when the icon is no longer retained, and we only release icons when we fail to get the
+        image data (including pages that don't have a favicon). We can prevent this crash from happening if we ensure we
+        never create an Image for an IconRecord when the given image data is nullptr.
+
+        * UIProcess/API/glib/IconDatabase.cpp:
+        (WebKit::IconDatabase::IconRecord::setImageData):
+
 2017-07-24  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp

@@ -117 +117 @@
 void IconDatabase::IconRecord::setImageData(RefPtr<SharedBuffer>&& data)
 {
+    m_dataSet = true;
+
     // It's okay to delete the raw image here. Any existing clients using this icon will be
     // managing an image that was created with a copy of this raw image data.
+    if (!data->size()) {
+        m_image = nullptr;
+        return;
+    }
+
     m_image = BitmapImage::create();
-
-    // Copy the provided data into the buffer of the new Image object.
     if (m_image->setData(WTFMove(data), true) < EncodedDataStatus::SizeAvailable) {
         LOG(IconDatabase, "Manual image data for iconURL '%s' FAILED - it was probably invalid image data", m_iconURL.ascii().data());
         m_image = nullptr;
     }
-
-    m_dataSet = true;
 }