[libsoup/hsts] Clean up the usage of the SoupHSTSEnforcer::changed signal

[Claudio Saavedra <csaavedra src gnome org>]

commit 13dafe54d66d2c5ee10fb7121298ce8cc70f3fa2
Author: Claudio Saavedra <csaavedra igalia com>
Date:   Wed Sep 26 15:38:00 2018 +0300

    Clean up the usage of the SoupHSTSEnforcer::changed signal
    
    The signal is emitted for any change in the enforcer policies.
    Callers are responsible for verifying whether the changed policy
    is permanent/session or not.
    
    Check this in the DB enforcer, before modifying the database.
    
    Document that the ::changed signal callbacks shouldn't modify
    the policies received as parameters.
    
    Remove comments that are no longer relevant.

 libsoup/soup-hsts-enforcer-db.c |  5 +++++
 libsoup/soup-hsts-enforcer.c    | 12 ++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)
---
diff --git a/libsoup/soup-hsts-enforcer-db.c b/libsoup/soup-hsts-enforcer-db.c
index da863341..f14f86cd 100644
--- a/libsoup/soup-hsts-enforcer-db.c
+++ b/libsoup/soup-hsts-enforcer-db.c
@@ -252,6 +252,11 @@ soup_hsts_enforcer_db_changed (SoupHSTSEnforcer *hsts_enforcer,
        SoupHSTSEnforcerDBPrivate *priv = SOUP_HSTS_ENFORCER_DB (hsts_enforcer)->priv;
        char *query;
 
+       /* Session policies do not need to be stored in the database. */
+       if ((old_policy && soup_hsts_policy_is_permanent (old_policy)) ||
+           (new_policy && soup_hsts_policy_is_permanent (new_policy)))
+               return;
+
        if (priv->db == NULL) {
                if (open_db (hsts_enforcer))
                        return;
diff --git a/libsoup/soup-hsts-enforcer.c b/libsoup/soup-hsts-enforcer.c
index 1279a494..ed71861d 100644
--- a/libsoup/soup-hsts-enforcer.c
+++ b/libsoup/soup-hsts-enforcer.c
@@ -161,6 +161,9 @@ soup_hsts_enforcer_class_init (SoupHSTSEnforcerClass *hsts_enforcer_class)
         * @new_policy will be %NULL. If a policy has been changed,
         * @old_policy will contain its old value, and @new_policy its
         * new value.
+        *
+        * Note that you shouldn't modify the policies from a callback to
+        * this signal.
         **/
        signals[CHANGED] =
                g_signal_new ("changed",
@@ -208,9 +211,7 @@ should_remove_expired_host_policy (G_GNUC_UNUSED gpointer key,
        if (soup_hsts_policy_is_expired (policy)) {
                /* This will emit the ::changed signal before the
                   policy is actually removed from the policies hash
-                  table, which could be problematic, or not. On the
-                  other hand, I have my doubts that the ::changed
-                  signal has any use.
+                  table, which could be problematic, or not.
                */
                soup_hsts_enforcer_changed (enforcer, policy, NULL);
                soup_hsts_policy_free (policy);
@@ -268,7 +269,7 @@ soup_hsts_enforcer_replace_policy (SoupHSTSEnforcer *hsts_enforcer,
        g_assert (old_policy);
 
        g_hash_table_replace (policies, g_strdup (domain), soup_hsts_policy_copy (new_policy));
-       if (!is_permanent && !soup_hsts_policy_equal (old_policy, new_policy))
+       if (!soup_hsts_policy_equal (old_policy, new_policy))
                soup_hsts_enforcer_changed (hsts_enforcer, old_policy, new_policy);
        soup_hsts_policy_free (old_policy);
 
@@ -299,8 +300,7 @@ soup_hsts_enforcer_insert_policy (SoupHSTSEnforcer *hsts_enforcer,
        g_assert (!g_hash_table_contains (policies, domain));
 
        g_hash_table_insert (policies, g_strdup (domain), soup_hsts_policy_copy (policy));
-       if (!is_permanent)
-               soup_hsts_enforcer_changed (hsts_enforcer, NULL, policy);
+       soup_hsts_enforcer_changed (hsts_enforcer, NULL, policy);
 }
 
 /**