Qt: move method to apply/undo UAT changes to UatModel

UatModel could be constructed with a name instead of an epan_uat type.
To allow those users to save/revert the uat, make sure to expose a
method that does not require access to the underlying epan_uat type.

Change-Id: I1d1a5811c1025bd9c2a2ea1722f460e6ac33b9aa
Reviewed-on: https://code.wireshark.org/review/31793
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

5 files changed, 90 insertions, 51 deletions

diff --git a/ui/qt/io_graph_dialog.cpp b/ui/qt/io_graph_dialog.cpp
index 32344af263..a9c254ded1 100644
--- a/ui/qt/io_graph_dialog.cpp
+++ b/ui/qt/io_graph_dialog.cpp
@@ -683,20 +683,15 @@ void IOGraphDialog::keyPressEvent(QKeyEvent *event)
 
 void IOGraphDialog::reject()
 {
-    if (!iog_uat_)
+    if (!uat_model_)
         return;
 
-    //There is no "rejection" of the UAT created.  Just save what we have
-    if (iog_uat_->changed) {
-        gchar *err = NULL;
-
-        if (!uat_save(iog_uat_, &err)) {
-            report_failure("Error while saving %s: %s", iog_uat_->name, err);
-            g_free(err);
-        }
-
-        if (iog_uat_->post_update_cb) {
-            iog_uat_->post_update_cb();
+    // Changes to the I/O Graph settings are always saved,
+    // there is no possibility for "rejection".
+    QString error;
+    if (uat_model_->applyChanges(error)) {
+        if (!error.isEmpty()) {
+            report_failure("%s", qPrintable(error));
         }
     }
 
diff --git a/ui/qt/models/uat_model.cpp b/ui/qt/models/uat_model.cpp
index e482bb6d37..78b930c242 100644
--- a/ui/qt/models/uat_model.cpp
+++ b/ui/qt/models/uat_model.cpp
@@ -51,6 +51,43 @@ void UatModel::reloadUat()
     endResetModel();
 }
 
+bool UatModel::applyChanges(QString &error)
+{
+    if (uat_->changed) {
+        gchar *err = NULL;
+
+        if (!uat_save(uat_, &err)) {
+            error = QString("Error while saving %1: %2").arg(uat_->name).arg(err);
+            g_free(err);
+        }
+
+        if (uat_->post_update_cb) {
+            uat_->post_update_cb();
+        }
+        return true;
+    }
+
+    return false;
+}
+
+bool UatModel::revertChanges(QString &error)
+{
+    // Ideally this model should remember the changes made and try to undo them
+    // to avoid calling post_update_cb. Calling uat_clear + uat_load is a lazy
+    // option and might fail (e.g. when the UAT file is removed).
+    if (uat_->changed) {
+        gchar *err = NULL;
+        uat_clear(uat_);
+        if (!uat_load(uat_, NULL, &err)) {
+            error = QString("Error while loading %1: %2").arg(uat_->name).arg(err);
+            g_free(err);
+        }
+        return true;
+    }
+
+    return false;
+}
+
 Qt::ItemFlags UatModel::flags(const QModelIndex &index) const
 {
     if (!index.isValid())
diff --git a/ui/qt/models/uat_model.h b/ui/qt/models/uat_model.h
index d7ffa62d4f..36513b1888 100644
--- a/ui/qt/models/uat_model.h
+++ b/ui/qt/models/uat_model.h
@@ -48,6 +48,23 @@ public:
     bool hasErrors() const;
     void clearAll();
 
+    /**
+     * If the UAT has changed, save the contents to file and invoke the UAT
+     * post_update_cb.
+     *
+     * @param error An error while saving changes, if any.
+     * @return true if anything changed, false otherwise.
+     */
+    bool applyChanges(QString &error);
+
+    /**
+     * Undo any changes to the UAT.
+     *
+     * @param error An error while restoring the original UAT, if any.
+     * @return true if anything changed, false otherwise.
+     */
+    bool revertChanges(QString &error);
+
     QModelIndex findRowForColumnContent(QVariant columnContent, int columnToCheckAgainst, int role = Qt::DisplayRole);
 
 private:
diff --git a/ui/qt/uat_dialog.cpp b/ui/qt/uat_dialog.cpp
index bd2ebe1f5d..98088c8d83 100644
--- a/ui/qt/uat_dialog.cpp
+++ b/ui/qt/uat_dialog.cpp
@@ -344,18 +344,12 @@ void UatDialog::applyChanges()
 
 void UatDialog::acceptChanges()
 {
-    if (!uat_) return;
-
-    if (uat_->changed) {
-        gchar *err = NULL;
+    if (!uat_model_) return;
 
-        if (!uat_save(uat_, &err)) {
-            report_failure("Error while saving %s: %s", uat_->name, err);
-            g_free(err);
-        }
-
-        if (uat_->post_update_cb) {
-            uat_->post_update_cb();
+    QString error;
+    if (uat_model_->applyChanges(error)) {
+        if (!error.isEmpty()) {
+            report_failure("%s", qPrintable(error));
         }
         applyChanges();
     }
@@ -363,15 +357,22 @@ void UatDialog::acceptChanges()
 
 void UatDialog::rejectChanges()
 {
-    if (!uat_) return;
+    if (!uat_model_) return;
 
-    if (uat_->changed) {
-        gchar *err = NULL;
-        uat_clear(uat_);
-        if (!uat_load(uat_, NULL, &err)) {
-            report_failure("Error while loading %s: %s", uat_->name, err);
-            g_free(err);
+    QString error;
+    if (uat_model_->revertChanges(error)) {
+        if (!error.isEmpty()) {
+            report_failure("%s", qPrintable(error));
         }
+        // Why do we have to trigger a redissection? If the original UAT is
+        // restored and dissectors only apply changes after the post_update_cb
+        // method is invoked, then it should not be necessary to trigger
+        // redissection. One potential exception is when something modifies the
+        // UAT file after Wireshark has started, but this behavior is not
+        // supported and causes potentially unnecessary redissection whenever
+        // the preferences dialog is closed.
+        // XXX audit all UAT providers and check whether it is safe to remove
+        // the next call (that is, when their update_cb has no side-effects).
         applyChanges();
     }
 }
diff --git a/ui/qt/uat_frame.cpp b/ui/qt/uat_frame.cpp
index 2e3e86e31d..bb1dadd458 100644
--- a/ui/qt/uat_frame.cpp
+++ b/ui/qt/uat_frame.cpp
@@ -149,37 +149,26 @@ void UatFrame::applyChanges()
 
 void UatFrame::acceptChanges()
 {
-    if (!uat_) return;
-
-    if (uat_->changed) {
-        gchar *err = NULL;
-
-        if (!uat_save(uat_, &err)) {
-            report_failure("Error while saving %s: %s", uat_->name, err);
-            g_free(err);
-        }
+    if (!uat_model_) return;
 
-        if (uat_->post_update_cb) {
-            uat_->post_update_cb();
+    QString error;
+    if (uat_model_->applyChanges(error)) {
+        if (!error.isEmpty()) {
+            report_failure("%s", qPrintable(error));
         }
-
         applyChanges();
     }
 }
 
 void UatFrame::rejectChanges()
 {
-    if (!uat_) return;
+    if (!uat_model_) return;
 
-    if (uat_->changed) {
-        gchar *err = NULL;
-        uat_clear(uat_);
-        if (!uat_load(uat_, NULL, &err)) {
-            report_failure("Error while loading %s: %s", uat_->name, err);
-            g_free(err);
+    QString error;
+    if (uat_model_->revertChanges(error)) {
+        if (!error.isEmpty()) {
+            report_failure("%s", qPrintable(error));
         }
-        //Filter expressions don't affect dissection, so there is no need to
-        //send any events to that effect
     }
 }