From 3ebbf5d1af88fe84d7c9caa7f3762a79b63d8cf3 Mon Sep 17 00:00:00 2001
From: Francois Marier <francois@mozilla.com>
Date: Tue, 29 May 2018 18:06:49 -0700
Subject: [PATCH] Bug 1452445 - Fix update error observer in unit test. r=gcp

We should only observe for update errors while we are expecting
a successful update.

MozReview-Commit-ID: 3grGhmxqhIX
---
 .../tests/unit/head_urlclassifier.js          | 24 +++++++++++++------
 .../tests/unit/test_hashcompleter_v4.js       |  4 ++++
 .../tests/unit/test_listmanager.js            |  4 ++--
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/toolkit/components/url-classifier/tests/unit/head_urlclassifier.js b/toolkit/components/url-classifier/tests/unit/head_urlclassifier.js
index c0c902af753f..457144089274 100644
--- a/toolkit/components/url-classifier/tests/unit/head_urlclassifier.js
+++ b/toolkit/components/url-classifier/tests/unit/head_urlclassifier.js
@@ -436,14 +436,24 @@ function waitUntilMetaDataSaved(expectedState, expectedChecksum, callback) {
   });
 }
 
+var gUpdateFinishedObserverEnabled = false;
+var gUpdateFinishedObserver = function (aSubject, aTopic, aData) {
+  info("[" + aTopic + "] " + aData);
+  if (aData != "success") {
+    updateError(aData);
+  }
+};
+
 function throwOnUpdateErrors() {
-  Services.obs.addObserver(function observer(aSubject, aTopic, aData) {
-    info("[" + aTopic + "] " + aData);
-    if (aData != "success") {
-      Services.obs.removeObserver(observer, aTopic);
-      updateError(aData);
-    }
-  }, "safebrowsing-update-finished");
+  Services.obs.addObserver(gUpdateFinishedObserver, "safebrowsing-update-finished");
+  gUpdateFinishedObserverEnabled = true;
+}
+
+function stopThrowingOnUpdateErrors() {
+  if (gUpdateFinishedObserverEnabled) {
+    Services.obs.removeObserver(gUpdateFinishedObserver, "safebrowsing-update-finished");
+    gUpdateFinishedObserverEnabled = false;
+  }
 }
 
 cleanUp();
diff --git a/toolkit/components/url-classifier/tests/unit/test_hashcompleter_v4.js b/toolkit/components/url-classifier/tests/unit/test_hashcompleter_v4.js
index 2bee00032e7c..63a42376db3f 100644
--- a/toolkit/components/url-classifier/tests/unit/test_hashcompleter_v4.js
+++ b/toolkit/components/url-classifier/tests/unit/test_hashcompleter_v4.js
@@ -239,3 +239,7 @@ function run_test() {
   gHttpServV4.start(5555);
   run_next_test();
 }
+
+registerCleanupFunction(function() {
+  stopThrowingOnUpdateErrors();
+});
diff --git a/toolkit/components/url-classifier/tests/unit/test_listmanager.js b/toolkit/components/url-classifier/tests/unit/test_listmanager.js
index ec99e6b476fa..e7a4e3586af2 100644
--- a/toolkit/components/url-classifier/tests/unit/test_listmanager.js
+++ b/toolkit/components/url-classifier/tests/unit/test_listmanager.js
@@ -192,8 +192,6 @@ add_test(function test_getGethashUrl() {
 });
 
 function run_test() {
-  throwOnUpdateErrors();
-
   // Setup primary testing server.
   gHttpServ = new HttpServer();
   gHttpServ.registerDirectory("/", do_get_cwd());
@@ -304,12 +302,14 @@ function run_test() {
 // A trick to force updating tables. However, before calling this, we have to
 // call disableAllUpdates() first to clean up the updateCheckers in listmanager.
 function forceTableUpdate() {
+  throwOnUpdateErrors();
   Services.prefs.setCharPref(PREF_NEXTUPDATETIME, "1");
   Services.prefs.setCharPref(PREF_NEXTUPDATETIME_V4, "1");
   gListManager.maybeToggleUpdateChecking();
 }
 
 function disableAllUpdates() {
+  stopThrowingOnUpdateErrors();
   TEST_TABLE_DATA_LIST.forEach(t => gListManager.disableUpdate(t.tableName));
   gListManager.disableUpdate(TEST_TABLE_DATA_V4.tableName);
 }