[GTK] Web process sometimes crashes when printing in synchronous mode
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2014 17:37:36 +0000 (17:37 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2014 17:37:36 +0000 (17:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=126979

Reviewed by Gustavo Noronha Silva.

Source/WebKit2:

When printing synchronously in GTK+ we need to make sure that we
have a list of Printers before starting the print operation. Getting
the list of printers is done synchronously by GTK+, but using a
nested main loop that might process IPC messages comming from the
UI process like EndPrinting. When the EndPrinting message is
received while the printer list is being populated, the print
operation is finished unexpectely and the web process crashes. 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. In case of asynchronous printing the
printer list will be created during the print operation without
any risk, because the EndPrinting message is not sent until the
printing callback has been received in the UI process.

* GNUmakefile.list.am: Add new files to compilation.
* PlatformGTK.cmake: Ditto.
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::print): Ensure PrinterListGtk is created
before the synchronous printing and destroyed afterwards.
* WebProcess/WebPage/gtk/PrinterListGtk.cpp: Added.
(WebKit::PrinterListGtk::shared): Return the singleton.
(WebKit::PrinterListGtk::enumeratePrintersFunction): Callback
called by gtk_enumerate_printers() when a new printer is found.
(WebKit::PrinterListGtk::PrinterListGtk): Call
gtk_enumerate_printers() in syhchronous mode.
(WebKit::PrinterListGtk::~PrinterListGtk):
(WebKit::PrinterListGtk::addPrinter): Add the printer to the list
and set the default printer if needed.
(WebKit::PrinterListGtk::findPrinter): Find the printer for the
given name.
* WebProcess/WebPage/gtk/PrinterListGtk.h: Added.
* WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp: Use
PrinterListGtk class to find the printer instead of calling
gtk_enumerate_printers().

Tools:

* Scripts/run-gtk-tests:
(TestRunner): Unskip
/webkit2/WebKitPrintOperation/close-after-print.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@162073 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/GNUmakefile.list.am
Source/WebKit2/PlatformGTK.cmake
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp [new file with mode: 0644]
Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h [new file with mode: 0644]
Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp
Tools/ChangeLog
Tools/Scripts/run-gtk-tests

index 76e63bb..f1f4bf2 100644 (file)
@@ -1,3 +1,45 @@
+2014-01-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Web process sometimes crashes when printing in synchronous mode
+        https://bugs.webkit.org/show_bug.cgi?id=126979
+
+        Reviewed by Gustavo Noronha Silva.
+
+        When printing synchronously in GTK+ we need to make sure that we
+        have a list of Printers before starting the print operation. Getting
+        the list of printers is done synchronously by GTK+, but using a
+        nested main loop that might process IPC messages comming from the
+        UI process like EndPrinting. When the EndPrinting message is
+        received while the printer list is being populated, the print
+        operation is finished unexpectely and the web process crashes. 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. In case of asynchronous printing the
+        printer list will be created during the print operation without
+        any risk, because the EndPrinting message is not sent until the
+        printing callback has been received in the UI process.
+
+        * GNUmakefile.list.am: Add new files to compilation.
+        * PlatformGTK.cmake: Ditto.
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::print): Ensure PrinterListGtk is created
+        before the synchronous printing and destroyed afterwards.
+        * WebProcess/WebPage/gtk/PrinterListGtk.cpp: Added.
+        (WebKit::PrinterListGtk::shared): Return the singleton.
+        (WebKit::PrinterListGtk::enumeratePrintersFunction): Callback
+        called by gtk_enumerate_printers() when a new printer is found.
+        (WebKit::PrinterListGtk::PrinterListGtk): Call
+        gtk_enumerate_printers() in syhchronous mode.
+        (WebKit::PrinterListGtk::~PrinterListGtk):
+        (WebKit::PrinterListGtk::addPrinter): Add the printer to the list
+        and set the default printer if needed.
+        (WebKit::PrinterListGtk::findPrinter): Find the printer for the
+        given name.
+        * WebProcess/WebPage/gtk/PrinterListGtk.h: Added.
+        * WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp: Use
+        PrinterListGtk class to find the printer instead of calling
+        gtk_enumerate_printers().
+
 2014-01-15  Tomas Popela  <tpopela@redhat.com>
 
         [SOUP] [WK2] - Disable MemoryCache when the DOCUMENT_VIEWER cache model is set
index 66f179c..471d8b1 100644 (file)
@@ -1243,6 +1243,8 @@ webkit2_sources += \
        Source/WebKit2/WebProcess/WebPage/TapHighlightController.h \
        Source/WebKit2/WebProcess/WebPage/atk/WebPageAccessibilityObject.h \
        Source/WebKit2/WebProcess/WebPage/atk/WebPageAccessibilityObjectAtk.cpp \
+       Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h \
+       Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp \
        Source/WebKit2/WebProcess/WebPage/gtk/WebInspectorGtk.cpp \
        Source/WebKit2/WebProcess/WebPage/gtk/WebPageGtk.cpp \
        Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp \
index 98842c2..fcca853 100644 (file)
@@ -283,6 +283,7 @@ list(APPEND WebKit2_SOURCES
     WebProcess/WebPage/atk/WebPageAccessibilityObjectAtk.cpp
 
     WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp
+    WebProcess/WebPage/gtk/PrinterListGtk.cpp
     WebProcess/WebPage/gtk/WebInspectorGtk.cpp
     WebProcess/WebPage/gtk/WebPageGtk.cpp
     WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp
index 10a911d..6bf25a6 100644 (file)
 #include "RemoteScrollingCoordinator.h"
 #endif
 
+#if PLATFORM(GTK)
+#include "PrinterListGtk.h"
+#endif
+
 using namespace WebCore;
 using namespace HTMLNames;
 
@@ -596,6 +600,16 @@ void WebChromeClient::print(Frame* frame)
     WebFrame* webFrame = webFrameLoaderClient ? webFrameLoaderClient->webFrame() : 0;
     ASSERT(webFrame);
 
+#if PLATFORM(GTK) && defined(HAVE_GTK_UNIX_PRINTING)
+    // When printing synchronously in GTK+ we need to make sure that we have a list of Printers before starting the print operation.
+    // Getting the list of printers is done synchronously by GTK+, but using a nested main loop that might process IPC messages
+    // comming from the UI process like EndPrinting. When the EndPriting message is received while the printer list is being populated,
+    // the print operation is finished unexpectely and the web process crashes, see https://bugs.webkit.org/show_bug.cgi?id=126979.
+    // 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::shared();
+#endif
+
     m_page->sendSync(Messages::WebPageProxy::PrintFrame(webFrame->frameID()), Messages::WebPageProxy::PrintFrame::Reply());
 }
 
diff --git a/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp b/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp
new file mode 100644 (file)
index 0000000..2d54bcc
--- /dev/null
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2014 Igalia S.L.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "PrinterListGtk.h"
+
+#ifdef HAVE_GTK_UNIX_PRINTING
+
+#include <gtk/gtkunixprint.h>
+
+namespace WebKit {
+
+PrinterListGtk* PrinterListGtk::s_sharedPrinterList = nullptr;
+
+RefPtr<PrinterListGtk> PrinterListGtk::shared()
+{
+    if (s_sharedPrinterList)
+        return 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)
+{
+    ASSERT(!s_sharedPrinterList);
+    s_sharedPrinterList = this;
+    gtk_enumerate_printers(reinterpret_cast<GtkPrinterFunc>(&enumeratePrintersFunction), nullptr, nullptr, TRUE);
+}
+
+PrinterListGtk::~PrinterListGtk()
+{
+    ASSERT(s_sharedPrinterList);
+    s_sharedPrinterList = nullptr;
+}
+
+void PrinterListGtk::addPrinter(GtkPrinter* printer)
+{
+    m_printerList.append(printer);
+    if (gtk_printer_is_default(printer))
+        m_defaultPrinter = printer;
+}
+
+GtkPrinter* PrinterListGtk::findPrinter(const char* printerName) const
+{
+    for (auto& printer : m_printerList) {
+        if (!strcmp(printerName, gtk_printer_get_name(printer.get())))
+            return printer.get();
+    }
+    return nullptr;
+}
+
+} // namespace WebKit
+
+#endif // HAVE_GTK_UNIX_PRINTING
diff --git a/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h b/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h
new file mode 100644 (file)
index 0000000..8d8a952
--- /dev/null
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2014 Igalia S.L.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef PrinterListGtk_h
+#define PrinterListGtk_h
+
+#ifdef HAVE_GTK_UNIX_PRINTING
+
+#include <wtf/RefCounted.h>
+#include <wtf/Vector.h>
+#include <wtf/gobject/GRefPtr.h>
+
+typedef struct _GtkPrinter GtkPrinter;
+
+namespace WebKit {
+
+class PrinterListGtk: public RefCounted<PrinterListGtk> {
+public:
+    static RefPtr<PrinterListGtk> shared();
+    ~PrinterListGtk();
+
+    GtkPrinter* findPrinter(const char*) const;
+    GtkPrinter* defaultPrinter() const { return m_defaultPrinter; }
+
+private:
+    PrinterListGtk();
+
+    static gboolean enumeratePrintersFunction(GtkPrinter*);
+    void addPrinter(GtkPrinter*);
+
+    Vector<GRefPtr<GtkPrinter>, 4> m_printerList;
+    GtkPrinter* m_defaultPrinter;
+    static PrinterListGtk* s_sharedPrinterList;
+};
+
+} // namespace WebKit
+
+#endif // HAVE_GTK_UNIX_PRINTING
+
+#endif // WebPrintOperationGtk_h
index 304159c..7f750e8 100644 (file)
@@ -40,6 +40,7 @@
 #include <wtf/gobject/GOwnPtr.h>
 
 #ifdef HAVE_GTK_UNIX_PRINTING
+#include "PrinterListGtk.h"
 #include <cairo-pdf.h>
 #include <cairo-ps.h>
 #include <gtk/gtkunixprint.h>
@@ -56,60 +57,47 @@ public:
     {
     }
 
-    static gboolean enumeratePrintersFunction(GtkPrinter* printer, WebPrintOperationGtkUnix* printOperation)
+    void startPrint(WebCore::PrintContext* printContext, uint64_t callbackID)
     {
-        const char* printerName = gtk_print_settings_get_printer(printOperation->printSettings());
-        if ((printerName && strcmp(printerName, gtk_printer_get_name(printer)))
-            || (!printerName && !gtk_printer_is_default(printer)))
-            return FALSE;
+        m_printContext = printContext;
+        m_callbackID = callbackID;
+
+        RefPtr<PrinterListGtk> printerList = PrinterListGtk::shared();
+        const char* printerName = gtk_print_settings_get_printer(m_printSettings.get());
+        GtkPrinter* printer = printerName ? printerList->findPrinter(printerName) : printerList->defaultPrinter();
+        if (!printer) {
+            printDone(printerNotFoundError(m_printContext));
+            return;
+        }
 
         static int jobNumber = 0;
         const char* applicationName = g_get_application_name();
         GOwnPtr<char>jobName(g_strdup_printf("%s job #%d", applicationName ? applicationName : "WebKit", ++jobNumber));
-        printOperation->m_printJob = adoptGRef(gtk_print_job_new(jobName.get(), printer,
-                                                                 printOperation->printSettings(),
-                                                                 printOperation->pageSetup()));
-        return TRUE;
-    }
-
-    static void enumeratePrintersFinished(WebPrintOperationGtkUnix* printOperation)
-    {
-        if (!printOperation->m_printJob) {
-            printOperation->printDone(printerNotFoundError(printOperation->m_printContext));
-            return;
-        }
+        m_printJob = adoptGRef(gtk_print_job_new(jobName.get(), printer, m_printSettings.get(), m_pageSetup.get()));
 
         GOwnPtr<GError> error;
-        cairo_surface_t* surface = gtk_print_job_get_surface(printOperation->m_printJob.get(), &error.outPtr());
+        cairo_surface_t* surface = gtk_print_job_get_surface(m_printJob.get(), &error.outPtr());
         if (!surface) {
-            printOperation->printDone(printError(printOperation->m_printContext, error->message));
+            printDone(printError(m_printContext, error->message));
             return;
         }
 
         int rangesCount;
-        printOperation->m_pageRanges = gtk_print_job_get_page_ranges(printOperation->m_printJob.get(), &rangesCount);
-        printOperation->m_pageRangesCount = rangesCount;
-        printOperation->m_pagesToPrint = gtk_print_job_get_pages(printOperation->m_printJob.get());
-        printOperation->m_needsRotation = gtk_print_job_get_rotate(printOperation->m_printJob.get());
+        m_pageRanges = gtk_print_job_get_page_ranges(m_printJob.get(), &rangesCount);
+        m_pageRangesCount = rangesCount;
+        m_pagesToPrint = gtk_print_job_get_pages(m_printJob.get());
+        m_needsRotation = gtk_print_job_get_rotate(m_printJob.get());
 
         // Manual capabilities.
-        printOperation->m_numberUp = gtk_print_job_get_n_up(printOperation->m_printJob.get());
-        printOperation->m_numberUpLayout = gtk_print_job_get_n_up_layout(printOperation->m_printJob.get());
-        printOperation->m_pageSet = gtk_print_job_get_page_set(printOperation->m_printJob.get());
-        printOperation->m_reverse = gtk_print_job_get_reverse(printOperation->m_printJob.get());
-        printOperation->m_copies = gtk_print_job_get_num_copies(printOperation->m_printJob.get());
-        printOperation->m_collateCopies = gtk_print_job_get_collate(printOperation->m_printJob.get());
-        printOperation->m_scale = gtk_print_job_get_scale(printOperation->m_printJob.get());
-
-        printOperation->print(surface, 72, 72);
-    }
-
-    void startPrint(WebCore::PrintContext* printContext, uint64_t callbackID)
-    {
-        m_printContext = printContext;
-        m_callbackID = callbackID;
-        gtk_enumerate_printers(reinterpret_cast<GtkPrinterFunc>(enumeratePrintersFunction), this,
-            reinterpret_cast<GDestroyNotify>(enumeratePrintersFinished), m_printMode == PrintInfo::PrintModeSync);
+        m_numberUp = gtk_print_job_get_n_up(m_printJob.get());
+        m_numberUpLayout = gtk_print_job_get_n_up_layout(m_printJob.get());
+        m_pageSet = gtk_print_job_get_page_set(m_printJob.get());
+        m_reverse = gtk_print_job_get_reverse(m_printJob.get());
+        m_copies = gtk_print_job_get_num_copies(m_printJob.get());
+        m_collateCopies = gtk_print_job_get_collate(m_printJob.get());
+        m_scale = gtk_print_job_get_scale(m_printJob.get());
+
+        print(surface, 72, 72);
     }
 
     void startPage(cairo_t* cr)
index c1ac99a..91a6fbb 100644 (file)
@@ -1,3 +1,14 @@
+2014-01-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Web process sometimes crashes when printing in synchronous mode
+        https://bugs.webkit.org/show_bug.cgi?id=126979
+
+        Reviewed by Gustavo Noronha Silva.
+
+        * Scripts/run-gtk-tests:
+        (TestRunner): Unskip
+        /webkit2/WebKitPrintOperation/close-after-print.
+
 2014-01-15  ChangSeok Oh  <changseok.oh@collabora.com>
 
         [EFL] Change test font installed path to webkitgtk-font-tests
index 51f71eb..c0a91f5 100755 (executable)
@@ -69,7 +69,6 @@ class TestRunner:
         SkippedTest("WebKit2Gtk/TestUIClient", "/webkit2/WebKitWebView/mouse-target", "Test times out after r150890", 117689),
         SkippedTest("WebKit2Gtk/TestContextMenu", SkippedTest.ENTIRE_SUITE, "Test times out after r150890", 117689),
         SkippedTest("WebKit2Gtk/TestWebKitWebView", "/webkit2/WebKitWebView/snapshot", "Test fails", 120404),
-        SkippedTest("WebKit2Gtk/TestPrinting", "/webkit2/WebKitPrintOperation/close-after-print", "Test times out", 126979),
         SkippedTest("WebKit2/TestWebKit2", "WebKit2.MouseMoveAfterCrash", "Test is flaky", 85066),
         SkippedTest("WebKit2/TestWebKit2", "WebKit2.NewFirstVisuallyNonEmptyLayoutForImages", "Test is flaky", 85066),
         SkippedTest("WebKit2/TestWebKit2", "WebKit2.NewFirstVisuallyNonEmptyLayoutFrames", "Test fails", 85037),