Changeset 179744 in webkit

Timestamp:

Feb 6, 2015 6:46:17 AM (9 years ago)

Author:

Carlos Garcia Campos

Message:

ASSERTION FAILED: !m_adoptionIsRequired in WTF::RefCountedBase::ref
​https://bugs.webkit.org/show_bug.cgi?id=141035

Reviewed by Sergio Villar Senin.

Rename PrinterListGtk::singleton() as PrinterListGtk::getOrCreate(), and
make it return nullptr when the shared PrinterListGtk object is
still being created. This can happen if the nested loop used by
gtk_enumerate_printers dispatches a GSource that starts a new
synchronous print operation. In that case we just ignore the
second print operation, since there's already one ongoing.

• WebProcess/WebCoreSupport/WebChromeClient.cpp:

(WebKit::WebChromeClient::print): Return early if
PrinterListGtk::getOrCreate() return nullptr.

• WebProcess/WebPage/gtk/PrinterListGtk.cpp:

(WebKit::PrinterListGtk::getOrCreate): Return nullptr if the
PrinterListGtk is still enumerating the printers.
(WebKit::PrinterListGtk::PrinterListGtk): Initialize
m_enumeratingPrinters to true before calling
gtk_enumerate_printers, and to false once it finishes.
(WebKit::PrinterListGtk::singleton): Deleted.
(WebKit::PrinterListGtk::enumeratePrintersFunction): Deleted.

• WebProcess/WebPage/gtk/PrinterListGtk.h:
• WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp: Add an

assertion here since PrinterListGtk::getOrCreate() should never
return nullptr at this point.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/WebCoreSupport/WebChromeClient.cpp (modified) (1 diff)
• WebProcess/WebPage/gtk/PrinterListGtk.cpp (modified) (1 diff)
• WebProcess/WebPage/gtk/PrinterListGtk.h (modified) (2 diffs)
• WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-02-06  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        ASSERTION FAILED: !m_adoptionIsRequired in WTF::RefCountedBase::ref
+        https://bugs.webkit.org/show_bug.cgi?id=141035
+
+        Reviewed by Sergio Villar Senin.
+
+        Rename PrinterListGtk::singleton() as PrinterListGtk::getOrCreate(), and
+        make it return nullptr when the shared PrinterListGtk object is
+        still being created. This can happen if the nested loop used by
+        gtk_enumerate_printers dispatches a GSource that starts a new
+        synchronous print operation. In that case we just ignore the
+        second print operation, since there's already one ongoing.
+
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::print): Return early if
+        PrinterListGtk::getOrCreate() return nullptr.
+        * WebProcess/WebPage/gtk/PrinterListGtk.cpp:
+        (WebKit::PrinterListGtk::getOrCreate): Return nullptr if the
+        PrinterListGtk is still enumerating the printers.
+        (WebKit::PrinterListGtk::PrinterListGtk): Initialize
+        m_enumeratingPrinters to true before calling
+        gtk_enumerate_printers, and to false once it finishes.
+        (WebKit::PrinterListGtk::singleton): Deleted.
+        (WebKit::PrinterListGtk::enumeratePrintersFunction): Deleted.
+        * WebProcess/WebPage/gtk/PrinterListGtk.h:
+        * WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp: Add an
+        assertion here since PrinterListGtk::getOrCreate() should never
+        return nullptr at this point.
+
 2015-02-05  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp

@@ -663 +663 @@
     // The PrinterListGtk class gets the list of printers in the constructor so we just need to ensure there's an instance alive
     // during the synchronous print operation.
-    RefPtr<PrinterListGtk> printerList = PrinterListGtk::singleton();
+    RefPtr<PrinterListGtk> printerList = PrinterListGtk::getOrCreate();
+    if (!printerList) {
+        // PrinterListGtk::getOrCreate() returns nullptr when called while a printers enumeration is ongoing.
+        // This can happen if a synchronous print is started by a JavaScript and another one is inmeditaley started
+        // from a JavaScript event listener. The second print operation is handled by the nested main loop used by GTK+
+        // to enumerate the printers, and we end up here trying to get a reference of an object that is being constructed.
+        // It's very unlikely that the user wants to print twice in a row, and other browsers don't do two print operations
+        // in this particular case either. So, the safest solution is to return early here and ignore the second print.
+        // See https://bugs.webkit.org/show_bug.cgi?id=141035
+        return;
+    }
 #endif
 

trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp

@@ -35 +35 @@
 PrinterListGtk* PrinterListGtk::s_sharedPrinterList = nullptr;
 
-RefPtr<PrinterListGtk> PrinterListGtk::singleton()
+RefPtr<PrinterListGtk> PrinterListGtk::getOrCreate()
 {
     if (s_sharedPrinterList)
-        return s_sharedPrinterList;
+        return s_sharedPrinterList->isEnumeratingPrinters() ? nullptr : s_sharedPrinterList;
 
     return adoptRef(new PrinterListGtk);
 }
 
-gboolean PrinterListGtk::enumeratePrintersFunction(GtkPrinter* printer)
-{
-    ASSERT(s_sharedPrinterList);
-    s_sharedPrinterList->addPrinter(printer);
-    return FALSE;
-}
-
 PrinterListGtk::PrinterListGtk()
     : m_defaultPrinter(nullptr)
+    , m_enumeratingPrinters(true)
 {
     ASSERT(!s_sharedPrinterList);
     s_sharedPrinterList = this;
-    gtk_enumerate_printers(reinterpret_cast<GtkPrinterFunc>(&enumeratePrintersFunction), nullptr, nullptr, TRUE);
+    gtk_enumerate_printers([](GtkPrinter* printer, gpointer) -> gboolean {
+        ASSERT(s_sharedPrinterList);
+        s_sharedPrinterList->addPrinter(printer);
+        return FALSE;
+    }, nullptr, nullptr, TRUE);
+    m_enumeratingPrinters = false;
 }
 

trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h

@@ -39 +39 @@
 class PrinterListGtk: public RefCounted<PrinterListGtk> {
 public:
-    static RefPtr<PrinterListGtk> singleton();
+    static RefPtr<PrinterListGtk> getOrCreate();
     ~PrinterListGtk();
 
@@ -48 +48 @@
     PrinterListGtk();
 
-    static gboolean enumeratePrintersFunction(GtkPrinter*);
     void addPrinter(GtkPrinter*);
+    bool isEnumeratingPrinters() const { return m_enumeratingPrinters; }
 
     Vector<GRefPtr<GtkPrinter>, 4> m_printerList;
     GtkPrinter* m_defaultPrinter;
+    bool m_enumeratingPrinters;
     static PrinterListGtk* s_sharedPrinterList;
 };

trunk/Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp

@@ -68 +68 @@
         m_callbackID = callbackID;
 
-        RefPtr<PrinterListGtk> printerList = PrinterListGtk::singleton();
+        RefPtr<PrinterListGtk> printerList = PrinterListGtk::getOrCreate();
+        ASSERT(printerList);
         const char* printerName = gtk_print_settings_get_printer(m_printSettings.get());
         GtkPrinter* printer = printerName ? printerList->findPrinter(printerName) : printerList->defaultPrinter();