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 <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

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 <QDebug>
 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 @@
       <enum>Qt::Horizontal</enum>
      </property>
      <property name="standardButtons">
-      <set>QDialogButtonBox::Apply|QDialogButtonBox::Cancel|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::Save</set>
+      <set>QDialogButtonBox::Cancel|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::Save</set>
      </property>
     </widget>
    </item>
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 @@
   </action>
   <action name="actionAnalyzeEnabledProtocols">
    <property name="text">
-    <string>Enable Protocols…</string>
+    <string>Enabled Protocols…</string>
    </property>
    <property name="toolTip">
     <string>Enable and disable specific protocols</string>
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<WiresharkApplication::AppSignal> 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<WiresharkApplication::AppSignal> 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<AppSignal> app_signals_;
 
 protected:
     bool event(QEvent *event);