From 374439daa17fe2692746857b920616354ce658e7 Mon Sep 17 00:00:00 2001 From: Gerald Combs Date: Wed, 26 Aug 2015 11:24:29 -0700 Subject: [PATCH] Don't emit app signals from dialogs. Emitting PacketDissectionChanged from a dialog on can render the main window unusable on OS X. A workaround for this was added to the preferences dialog in g8fc2327. Generalize the workaround and use it elsewhere. Fix the "Enabled Protocols" action name while we're here. Bug: 11361 Bug: 11448 Change-Id: I89e98daaaedc877d3b13b0d33b6f3be033e323d7 Reviewed-on: https://code.wireshark.org/review/10271 Petri-Dish: Gerald Combs Tested-by: Petri Dish Buildbot Reviewed-by: Gerald Combs --- ui/qt/decode_as_dialog.cpp | 2 +- ui/qt/enabled_protocols_dialog.cpp | 12 ++++++++---- ui/qt/enabled_protocols_dialog.h | 2 ++ ui/qt/enabled_protocols_dialog.ui | 2 +- ui/qt/main_window.ui | 2 +- ui/qt/main_window_slots.cpp | 27 ++++++++++++++++---------- ui/qt/preferences_dialog.cpp | 7 ++----- ui/qt/preferences_dialog.h | 3 --- ui/qt/protocol_preferences_menu.cpp | 3 +++ ui/qt/sctp_chunk_statistics_dialog.cpp | 4 ++++ ui/qt/uat_dialog.cpp | 4 ++-- ui/qt/wireshark_application.cpp | 20 +++++++++++++++++++ ui/qt/wireshark_application.h | 8 ++++++++ 13 files changed, 69 insertions(+), 27 deletions(-) diff --git a/ui/qt/decode_as_dialog.cpp b/ui/qt/decode_as_dialog.cpp index 7edf2f248b..ed45346adb 100644 --- a/ui/qt/decode_as_dialog.cpp +++ b/ui/qt/decode_as_dialog.cpp @@ -587,7 +587,7 @@ void DecodeAsDialog::applyChanges() delete(dissector_info); } - wsApp->emitAppSignal(WiresharkApplication::PacketDissectionChanged); + wsApp->queueAppSignal(WiresharkApplication::PacketDissectionChanged); } void DecodeAsDialog::on_buttonBox_clicked(QAbstractButton *button) diff --git a/ui/qt/enabled_protocols_dialog.cpp b/ui/qt/enabled_protocols_dialog.cpp index 7026f9607f..283f2ab0f6 100644 --- a/ui/qt/enabled_protocols_dialog.cpp +++ b/ui/qt/enabled_protocols_dialog.cpp @@ -46,6 +46,7 @@ enum heuristic_type_ = 1002 }; +#include class EnableProtocolTreeWidgetItem : public QTreeWidgetItem { public: @@ -245,11 +246,11 @@ bool EnabledProtocolsDialog::applyChanges() if ((*it)->checkState(PROTOCOL_COLUMN) == Qt::Checked) { - redissect |= it_cast->applyValue(TRUE); + redissect |= it_cast->applyValue(true); } else { - redissect |= it_cast->applyValue(FALSE); + redissect |= it_cast->applyValue(false); } ++it; } @@ -311,10 +312,13 @@ void EnabledProtocolsDialog::on_buttonBox_accepted() if (applyChanges()) { writeChanges(); - wsApp->emitAppSignal(WiresharkApplication::PacketDissectionChanged); + wsApp->queueAppSignal(WiresharkApplication::PacketDissectionChanged); } } +#if 0 +// If we ever find and fix the bug behind queueAppSignal we can re-enable +// this. void EnabledProtocolsDialog::on_buttonBox_clicked(QAbstractButton *button) { if (button == ui->buttonBox->button(QDialogButtonBox::Apply)) @@ -328,8 +332,8 @@ void EnabledProtocolsDialog::on_buttonBox_clicked(QAbstractButton *button) wsApp->emitAppSignal(WiresharkApplication::PacketDissectionChanged); } } - } +#endif void EnabledProtocolsDialog::on_buttonBox_helpRequested() { diff --git a/ui/qt/enabled_protocols_dialog.h b/ui/qt/enabled_protocols_dialog.h index ca14f93145..761a4f4ec9 100644 --- a/ui/qt/enabled_protocols_dialog.h +++ b/ui/qt/enabled_protocols_dialog.h @@ -47,7 +47,9 @@ private slots: void on_disable_all_button__clicked(); void on_search_line_edit__textChanged(const QString &search_re); void on_buttonBox_accepted(); +#if 0 void on_buttonBox_clicked(QAbstractButton *button); +#endif void on_buttonBox_helpRequested(); private: diff --git a/ui/qt/enabled_protocols_dialog.ui b/ui/qt/enabled_protocols_dialog.ui index e52c1a127d..6740f808a2 100644 --- a/ui/qt/enabled_protocols_dialog.ui +++ b/ui/qt/enabled_protocols_dialog.ui @@ -100,7 +100,7 @@ Qt::Horizontal - QDialogButtonBox::Apply|QDialogButtonBox::Cancel|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::Save + QDialogButtonBox::Cancel|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::Save diff --git a/ui/qt/main_window.ui b/ui/qt/main_window.ui index 3db7781e8a..36d6773475 100644 --- a/ui/qt/main_window.ui +++ b/ui/qt/main_window.ui @@ -2604,7 +2604,7 @@ - Enable Protocols… + Enabled Protocols… Enable and disable specific protocols diff --git a/ui/qt/main_window_slots.cpp b/ui/qt/main_window_slots.cpp index 348707b497..599edc738d 100644 --- a/ui/qt/main_window_slots.cpp +++ b/ui/qt/main_window_slots.cpp @@ -1972,11 +1972,9 @@ void MainWindow::showPreferencesDialog(PreferencesDialog::PreferencesPane start_ pref_dialog.setPane(start_pane); pref_dialog.exec(); - // Emitting PacketDissectionChanged directly from PreferencesDialog - // can cause problems. Queue them up and emit them here. - foreach (WiresharkApplication::AppSignal app_signal, pref_dialog.appSignals()) { - wsApp->emitAppSignal(app_signal); - } + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } void MainWindow::showPreferencesDialog(QString module_name) @@ -1986,11 +1984,9 @@ void MainWindow::showPreferencesDialog(QString module_name) pref_dialog.setPane(module_name); pref_dialog.exec(); - // Emitting PacketDissectionChanged directly from PreferencesDialog - // can cause problems. Queue them up and emit them here. - foreach (WiresharkApplication::AppSignal app_signal, pref_dialog.appSignals()) { - wsApp->emitAppSignal(app_signal); - } + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } void MainWindow::on_actionEditPreferences_triggered() @@ -2369,6 +2365,9 @@ void MainWindow::on_actionAnalyzeDisplayFilterMacros_triggered() UatDialog uat_dlg(parentWidget(), dfm_uat); uat_dlg.exec(); + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } void MainWindow::on_actionAnalyzeCreateAColumn_triggered() @@ -2466,6 +2465,10 @@ void MainWindow::on_actionAnalyzeEnabledProtocols_triggered() { EnabledProtocolsDialog enable_proto_dialog(this); enable_proto_dialog.exec(); + + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } void MainWindow::on_actionAnalyzeDecodeAs_triggered() @@ -2480,6 +2483,10 @@ void MainWindow::on_actionAnalyzeDecodeAs_triggered() connect(this, SIGNAL(setCaptureFile(capture_file*)), &da_dialog, SLOT(setCaptureFile(capture_file*))); da_dialog.exec(); + + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } #ifdef HAVE_LUA diff --git a/ui/qt/preferences_dialog.cpp b/ui/qt/preferences_dialog.cpp index 58e7a8cbc5..f8c16f83dc 100644 --- a/ui/qt/preferences_dialog.cpp +++ b/ui/qt/preferences_dialog.cpp @@ -842,14 +842,11 @@ void PreferencesDialog::on_buttonBox_accepted() wsApp->setMonospaceFont(prefs.gui_qt_font_name); - /* Now destroy the "Preferences" dialog. */ -// window_destroy(GTK_WIDGET(parent_w)); - if (must_redissect) { /* Redissect all the packets, and re-evaluate the display filter. */ - app_signals_ << WiresharkApplication::PacketDissectionChanged; + wsApp->queueAppSignal(WiresharkApplication::PacketDissectionChanged); } - app_signals_ << WiresharkApplication::PreferencesChanged; + wsApp->queueAppSignal(WiresharkApplication::PreferencesChanged); } void PreferencesDialog::on_buttonBox_helpRequested() diff --git a/ui/qt/preferences_dialog.h b/ui/qt/preferences_dialog.h index 7f083da461..95582e05f7 100644 --- a/ui/qt/preferences_dialog.h +++ b/ui/qt/preferences_dialog.h @@ -64,8 +64,6 @@ public: void setPane(PreferencesPane start_pane); void setPane(const QString module_name); - const QList appSignals() const { return app_signals_; } - protected: void showEvent(QShowEvent *evt); void keyPressEvent(QKeyEvent *evt); @@ -81,7 +79,6 @@ private: QString saved_string_pref_; QComboBox *cur_combo_box_; int saved_combo_idx_; - QList app_signals_; private slots: void on_prefsTree_currentItemChanged(QTreeWidgetItem *current, QTreeWidgetItem *previous); diff --git a/ui/qt/protocol_preferences_menu.cpp b/ui/qt/protocol_preferences_menu.cpp index 12999ec89b..600f0c4bc9 100644 --- a/ui/qt/protocol_preferences_menu.cpp +++ b/ui/qt/protocol_preferences_menu.cpp @@ -101,6 +101,9 @@ public: void showUatDialog() { UatDialog uat_dlg(parentWidget(), pref_->varp.uat); uat_dlg.exec(); + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); } private: diff --git a/ui/qt/sctp_chunk_statistics_dialog.cpp b/ui/qt/sctp_chunk_statistics_dialog.cpp index ae5ce965c9..a92872a47c 100644 --- a/ui/qt/sctp_chunk_statistics_dialog.cpp +++ b/ui/qt/sctp_chunk_statistics_dialog.cpp @@ -289,6 +289,10 @@ void SCTPChunkStatisticsDialog::on_actionChunkTypePreferences_triggered() UatDialog *uatdialog = new UatDialog(this, pref->varp.uat); uatdialog->exec(); + // Emitting PacketDissectionChanged directly from a QDialog can cause + // problems on OS X. + wsApp->flushAppSignals(); + ui->tableWidget->clear(); ui->tableWidget->setRowCount(0); ui->tableWidget->setHorizontalHeaderItem(0, new QTableWidgetItem(QString(tr("Association")))); diff --git a/ui/qt/uat_dialog.cpp b/ui/qt/uat_dialog.cpp index e2a6e957fe..8e39b68b57 100644 --- a/ui/qt/uat_dialog.cpp +++ b/ui/qt/uat_dialog.cpp @@ -496,11 +496,11 @@ void UatDialog::applyChanges() if (uat_->flags & UAT_AFFECTS_FIELDS) { /* Recreate list with new fields and redissect packets */ - wsApp->emitAppSignal(WiresharkApplication::FieldsChanged); + wsApp->queueAppSignal(WiresharkApplication::FieldsChanged); } if (uat_->flags & UAT_AFFECTS_DISSECTION) { /* Just redissect packets if we have any */ - wsApp->emitAppSignal(WiresharkApplication::PacketDissectionChanged); + wsApp->queueAppSignal(WiresharkApplication::PacketDissectionChanged); } } diff --git a/ui/qt/wireshark_application.cpp b/ui/qt/wireshark_application.cpp index 6a6425e2a7..eb85278e56 100644 --- a/ui/qt/wireshark_application.cpp +++ b/ui/qt/wireshark_application.cpp @@ -661,6 +661,26 @@ void WiresharkApplication::emitAppSignal(AppSignal signal) } } +// Flush any collected app signals. +// +// On OS X emitting PacketDissectionChanged from a dialog can +// render the application unusable: +// https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=11361 +// https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=11448 +// Work around the problem by queueing up app signals and emitting them +// after the dialog is closed. +// +// The following bugs might be related although they don't describe the +// exact behavior we're working around here: +// https://bugreports.qt.io/browse/QTBUG-38512 +// https://bugreports.qt.io/browse/QTBUG-38600 +void WiresharkApplication::flushAppSignals() +{ + while (!app_signals_.isEmpty()) { + wsApp->emitAppSignal(app_signals_.takeFirst()); + } +} + void WiresharkApplication::emitStatCommandSignal(const QString &menu_path, const char *arg, void *userdata) { emit openStatCommandDialog(menu_path, arg, userdata); diff --git a/ui/qt/wireshark_application.h b/ui/qt/wireshark_application.h index 1173d84bc2..8340581e77 100644 --- a/ui/qt/wireshark_application.h +++ b/ui/qt/wireshark_application.h @@ -72,6 +72,13 @@ public: void registerUpdate(register_action_e action, const char *message); void emitAppSignal(AppSignal signal); + // Emitting app signals (PacketDissectionChanged in particular) from + // dialogs on OS X can be problematic. Dialogs should call queueAppSignal + // instead. + void queueAppSignal(AppSignal signal) { app_signals_ << signal; } + // Flush queued app signals. Should be called from the main window after + // each dialog that calls queueAppSignal closes. + void flushAppSignals(); void emitStatCommandSignal(const QString &menu_path, const char *arg, void *userdata); void emitTapParameterSignal(const QString cfg_abbr, const QString arg, void *userdata); void addDynamicMenuGroupItem(int group, QAction *sg_action); @@ -121,6 +128,7 @@ private: QIcon normal_icon_; QIcon capture_icon_; static QString window_title_separator_; + QList app_signals_; protected: bool event(QEvent *event);