Bug 1012214 - Settings service doesn't call handle callback created i…

…n lock object. r=bent
shellylin · May 20, 2014 · 3b1d888 · 3b1d888
1 parent 2a34efc
commit 3b1d888
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 16 deletions.
diff --git a/dom/settings/SettingsService.js b/dom/settings/SettingsService.js
@@ -26,14 +26,15 @@ const SETTINGSSERVICELOCK_CONTRACTID = "@mozilla.org/settingsServiceLock;1";
 const SETTINGSSERVICELOCK_CID        = Components.ID("{d7a395a0-e292-11e1-834e-1761d57f5f99}");
 const nsISettingsServiceLock         = Ci.nsISettingsServiceLock;
 
-function SettingsServiceLock(aSettingsService)
+function SettingsServiceLock(aSettingsService, aTransactionCallback)
 {
   if (DEBUG) debug("settingsServiceLock constr!");
   this._open = true;
   this._busy = false;
   this._requests = new Queue();
   this._settingsService = aSettingsService;
   this._transaction = null;
+  this._transactionCallback = aTransactionCallback;
 }
 
 SettingsServiceLock.prototype = {
@@ -62,6 +63,14 @@ SettingsServiceLock.prototype = {
     }
   },
 
+  callTransactionHandle: function callTransactionHandle() {
+    try {
+      this._transactionCallback ? this._transactionCallback.handle() : null;
+    } catch (e) {
+      dump("settings 'Transaction handle' callback threw an exception, dropping: " + e + "\n");
+    }
+  },
+
   process: function process() {
     debug("process!");
     let lock = this;
@@ -157,20 +166,20 @@ SettingsServiceLock.prototype = {
     lock._open = true;
   },
 
-  createTransactionAndProcess: function(aCallback) {
+  createTransactionAndProcess: function() {
     if (this._settingsService._settingsDB._db) {
       let lock;
       while (lock = this._settingsService._locks.dequeue()) {
         if (!lock._transaction) {
           lock._transaction = lock._settingsService._settingsDB._db.transaction(SETTINGSSTORE_NAME, "readwrite");
-          if (aCallback) {
-            lock._transaction.oncomplete = aCallback.handle;
+          if (lock._transactionCallback) {
+            lock._transaction.oncomplete = lock.callTransactionHandle.bind(lock);
             lock._transaction.onabort = function(event) {
               let message = '';
               if (event.target.error) {
                 message = event.target.error.name + ': ' + event.target.error.message;
               }
-              this.callAbort(aCallback, message);
+              this.callAbort(lock._transactionCallback.handleAbort, message);
             };
           }
         }
@@ -229,10 +238,10 @@ SettingsService.prototype = {
   },
 
   createLock: function createLock(aCallback) {
-    var lock = new SettingsServiceLock(this);
+    var lock = new SettingsServiceLock(this, aCallback);
     this._locks.enqueue(lock);
     this._settingsDB.ensureDB(
-      function() { lock.createTransactionAndProcess(aCallback); },
+      function() { lock.createTransactionAndProcess(); },
       function() { dump("SettingsService failed to open DB!\n"); }
     );
     this.nextTick(function() { this._open = false; }, lock);

diff --git a/dom/settings/tests/chrome.ini b/dom/settings/tests/chrome.ini
@@ -2,3 +2,5 @@
 
 [test_settings_service.js]
 [test_settings_service.xul]
+[test_settings_service_callback.js]
+[test_settings_service_callback.xul]
diff --git a/dom/settings/tests/moz.build b/dom/settings/tests/moz.build
@@ -7,5 +7,5 @@
 MOCHITEST_MANIFESTS += ['mochitest.ini']
 
 if CONFIG['MOZ_B2G']:
-  MOCHITEST_CHROME_MANIFESTS += ['chrome.ini']
+    MOCHITEST_CHROME_MANIFESTS += ['chrome.ini']
 
diff --git a/dom/settings/tests/test_settings_service.js b/dom/settings/tests/test_settings_service.js
@@ -75,21 +75,14 @@ let tests = [
 
   /* Observer tests */
   function() {
-    const XPCOM_SHUTDOWN        = "xpcom-shutdown";
     const MOZSETTINGS_CHANGED   = "mozsettings-changed";
-
     const TEST_OBSERVER_KEY     = "test.observer.key";
     const TEST_OBSERVER_VALUE   = true;
     const TEST_OBSERVER_MESSAGE = "test.observer.message";
 
     let observerCount = 2;
 
     function observer(subject, topic, data) {
-      if (topic === XPCOM_SHUTDOWN) {
-        Services.obs.removeObserver(this, XPCOM_SHUTDOWN);
-        Services.obs.removeObserver(this, MOZSETTINGS_CHANGED);
-        return;
-      }
 
       if (topic !== MOZSETTINGS_CHANGED) {
         ok(false, "Event is not mozsettings-changed.");
@@ -114,11 +107,11 @@ let tests = [
       --observerCount;
 
       if (observerCount === 0) {
+        Services.obs.removeObserver(this, MOZSETTINGS_CHANGED);
         next();
       }
     }
 
-    Services.obs.addObserver(observer, XPCOM_SHUTDOWN, false);
     Services.obs.addObserver(observer, MOZSETTINGS_CHANGED, false);
 
     let lock = SettingsService.createLock();

diff --git a/dom/settings/tests/test_settings_service_callback.js b/dom/settings/tests/test_settings_service_callback.js
@@ -0,0 +1,47 @@
+"use strict";
+
+const Cu = Components.utils;
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
+
+SimpleTest.waitForExplicitFinish();
+
+XPCOMUtils.defineLazyServiceGetter(this, "SettingsService",
+                                   "@mozilla.org/settingsService;1",
+                                   "nsISettingsService");
+
+let tests = [
+  function () {
+    let callback = {
+      handle: function() {
+        ok(true, "handle called!");
+        next();
+      },
+
+      handleAbort: function(name) {
+        ok(false, "error: " + name);
+        next();
+      }
+    }
+    let lock = SettingsService.createLock(callback);
+    lock.set("xasdf", true, null, null);
+  }
+];
+
+function next() {
+  let step = tests.shift();
+  if (step) {
+    try {
+      step();
+    } catch(e) {
+      ok(false, "Test threw: " + e);
+    }
+  } else {
+    SimpleTest.finish();
+  }
+}
+
+next();
diff --git a/dom/settings/tests/test_settings_service_callback.xul b/dom/settings/tests/test_settings_service_callback.xul
@@ -0,0 +1,19 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1012214
+-->
+<window title="Mozilla Bug 1012214"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1012214"
+     target="_blank">Mozilla Bug 1012214</a>
+  </body>
+
+  <script type="application/javascript;version=1.7" src="test_settings_service_callback.js" />
+</window>