Title: [162073] trunk
Revision
162073
Author
carlo...@webkit.org
Date
2014-01-15 09:37:36 -0800 (Wed, 15 Jan 2014)

Log Message

[GTK] Web process sometimes crashes when printing in synchronous mode
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.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (162072 => 162073)


--- trunk/Source/WebKit2/ChangeLog	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Source/WebKit2/ChangeLog	2014-01-15 17:37:36 UTC (rev 162073)
@@ -1,3 +1,45 @@
+2014-01-15  Carlos Garcia Campos  <cgar...@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  <tpop...@redhat.com>
 
         [SOUP] [WK2] - Disable MemoryCache when the DOCUMENT_VIEWER cache model is set

Modified: trunk/Source/WebKit2/GNUmakefile.list.am (162072 => 162073)


--- trunk/Source/WebKit2/GNUmakefile.list.am	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Source/WebKit2/GNUmakefile.list.am	2014-01-15 17:37:36 UTC (rev 162073)
@@ -1243,6 +1243,8 @@
 	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 \

Modified: trunk/Source/WebKit2/PlatformGTK.cmake (162072 => 162073)


--- trunk/Source/WebKit2/PlatformGTK.cmake	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Source/WebKit2/PlatformGTK.cmake	2014-01-15 17:37:36 UTC (rev 162073)
@@ -283,6 +283,7 @@
     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

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp (162072 => 162073)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp	2014-01-15 17:37:36 UTC (rev 162073)
@@ -73,6 +73,10 @@
 #include "RemoteScrollingCoordinator.h"
 #endif
 
+#if PLATFORM(GTK)
+#include "PrinterListGtk.h"
+#endif
+
 using namespace WebCore;
 using namespace HTMLNames;
 
@@ -596,6 +600,16 @@
     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());
 }
 

Added: trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp (0 => 162073)


--- trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp	                        (rev 0)
+++ trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.cpp	2014-01-15 17:37:36 UTC (rev 162073)
@@ -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

Added: trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h (0 => 162073)


--- trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h	                        (rev 0)
+++ trunk/Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h	2014-01-15 17:37:36 UTC (rev 162073)
@@ -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

Modified: trunk/Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp (162072 => 162073)


--- trunk/Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp	2014-01-15 17:37:36 UTC (rev 162073)
@@ -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,62 +57,49 @@
     {
     }
 
-    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;
-    }
+        m_printJob = adoptGRef(gtk_print_job_new(jobName.get(), printer, m_printSettings.get(), m_pageSetup.get()));
 
-    static void enumeratePrintersFinished(WebPrintOperationGtkUnix* printOperation)
-    {
-        if (!printOperation->m_printJob) {
-            printOperation->printDone(printerNotFoundError(printOperation->m_printContext));
-            return;
-        }
-
         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());
+        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());
 
-        printOperation->print(surface, 72, 72);
+        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);
-    }
-
     void startPage(cairo_t* cr)
     {
         if (!currentPageIsFirstPageOfSheet())

Modified: trunk/Tools/ChangeLog (162072 => 162073)


--- trunk/Tools/ChangeLog	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Tools/ChangeLog	2014-01-15 17:37:36 UTC (rev 162073)
@@ -1,3 +1,14 @@
+2014-01-15  Carlos Garcia Campos  <cgar...@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...@collabora.com>
 
         [EFL] Change test font installed path to webkitgtk-font-tests

Modified: trunk/Tools/Scripts/run-gtk-tests (162072 => 162073)


--- trunk/Tools/Scripts/run-gtk-tests	2014-01-15 17:22:32 UTC (rev 162072)
+++ trunk/Tools/Scripts/run-gtk-tests	2014-01-15 17:37:36 UTC (rev 162073)
@@ -69,7 +69,6 @@
         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),
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to