Backing out bug 893276 for causing bug 1058840.
authorDave Townsend <dtownsend@oxymoronical.com>
Fri, 19 Sep 2014 11:28:55 -0700
changeset 206259 d94e895df00963e9e8d1c05dd94f490e6b2193a6
parent 206258 a5e8a4bff19453058c41eed9633691f4a7bff990
child 206260 615a9b66e66f921b51b8aefd43f0b9b592fe720c
push id27519
push userkwierso@gmail.com
push dateFri, 19 Sep 2014 23:56:39 +0000
treeherdermozilla-central@a85324dfc960 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs893276, 1058840
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backing out bug 893276 for causing bug 1058840.
addon-sdk/source/lib/sdk/simple-storage.js
addon-sdk/source/test/test-simple-storage.js
--- a/addon-sdk/source/lib/sdk/simple-storage.js
+++ b/addon-sdk/source/lib/sdk/simple-storage.js
@@ -3,26 +3,23 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 module.metadata = {
   "stability": "stable"
 };
 
-const { Cc, Ci, Cu } = require("chrome");
+const { Cc, Ci } = require("chrome");
 const file = require("./io/file");
 const prefs = require("./preferences/service");
 const jpSelf = require("./self");
 const timer = require("./timers");
 const unload = require("./system/unload");
 const { emit, on, off } = require("./event/core");
-const { defer } = require('./core/promise');
-
-const { Task } = Cu.import("resource://gre/modules/Task.jsm", {});
 
 const WRITE_PERIOD_PREF = "extensions.addon-sdk.simple-storage.writePeriod";
 const WRITE_PERIOD_DEFAULT = 300000; // 5 minutes
 
 const QUOTA_PREF = "extensions.addon-sdk.simple-storage.quota";
 const QUOTA_DEFAULT = 5242880; // 5 MiB
 
 const JETPACK_DIR_BASENAME = "jetpack";
@@ -33,78 +30,29 @@ Object.defineProperties(exports, {
     get: function() { return manager.root; },
     set: function(value) { manager.root = value; }
   },
   quotaUsage: {
     get: function() { return manager.quotaUsage; }
   }
 });
 
-function getHash(data) {
-  let { promise, resolve } = defer();
-
-  let crypto = Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
-  crypto.init(crypto.MD5);
-
-  let listener = {
-    onStartRequest: function() { },
-
-    onDataAvailable: function(request, context, inputStream, offset, count) {
-      crypto.updateFromStream(inputStream, count);
-    },
-
-    onStopRequest: function(request, context, status) {
-      resolve(crypto.finish(false));
-    }
-  };
-
-  let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
-                  createInstance(Ci.nsIScriptableUnicodeConverter);
-  converter.charset = "UTF-8";
-  let stream = converter.convertToInputStream(data);
-  let pump = Cc["@mozilla.org/network/input-stream-pump;1"].
-             createInstance(Ci.nsIInputStreamPump);
-  pump.init(stream, -1, -1, 0, 0, true);
-  pump.asyncRead(listener, null);
-
-  return promise;
-}
-
-function writeData(filename, data) {
-  let { promise, resolve, reject } = defer();
-
-  let stream = file.open(filename, "w");
-  try {
-    stream.writeAsync(data, err => {
-      if (err)
-        reject(err);
-      else
-        resolve();
-    });
-  }
-  catch (err) {
-    // writeAsync closes the stream after it's done, so only close on error.
-    stream.close();
-    reject(err);
-  }
-
-  return promise;
-}
-
 // A generic JSON store backed by a file on disk.  This should be isolated
 // enough to move to its own module if need be...
 function JsonStore(options) {
   this.filename = options.filename;
   this.quota = options.quota;
   this.writePeriod = options.writePeriod;
   this.onOverQuota = options.onOverQuota;
   this.onWrite = options.onWrite;
-  this.hash = null;
+
   unload.ensure(this);
-  this.startTimer();
+
+  this.writeTimer = timer.setInterval(this.write.bind(this),
+                                      this.writePeriod);
 }
 
 JsonStore.prototype = {
   // The store's root.
   get root() {
     return this.isRootInited ? this._root : {};
   },
 
@@ -128,28 +76,21 @@ JsonStore.prototype = {
   // Percentage of quota used, as a number [0, Inf).  > 1 implies over quota.
   // Undefined if there is no quota.
   get quotaUsage() {
     return this.quota > 0 ?
            JSON.stringify(this.root).length / this.quota :
            undefined;
   },
 
-  startTimer: function JsonStore_startTimer() {
-    timer.setTimeout(() => {
-      this.write().then(this.startTimer.bind(this));
-    }, this.writePeriod);
-  },
-
   // Removes the backing file and all empty subdirectories.
   purge: function JsonStore_purge() {
     try {
       // This'll throw if the file doesn't exist.
       file.remove(this.filename);
-      this.hash = null;
       let parentPath = this.filename;
       do {
         parentPath = file.dirname(parentPath);
         // This'll throw if the dir isn't empty.
         file.rmdir(parentPath);
       } while (file.basename(parentPath) !== JETPACK_DIR_BASENAME);
     }
     catch (err) {}
@@ -159,35 +100,41 @@ JsonStore.prototype = {
   read: function JsonStore_read() {
     try {
       let str = file.read(this.filename);
 
       // Ideally we'd log the parse error with console.error(), but logged
       // errors cause tests to fail.  Supporting "known" errors in the test
       // harness appears to be non-trivial.  Maybe later.
       this.root = JSON.parse(str);
-      let self = this;
-      getHash(str).then(hash => this.hash = hash);
     }
     catch (err) {
       this.root = {};
-      this.hash = null;
     }
   },
 
+  // If the store is under quota, writes the root to the backing file.
+  // Otherwise quota observers are notified and nothing is written.
+  write: function JsonStore_write() {
+    if (this.quotaUsage > 1)
+      this.onOverQuota(this);
+    else
+      this._write();
+  },
+
   // Cleans up on unload.  If unloading because of uninstall, the store is
   // purged; otherwise it's written.
   unload: function JsonStore_unload(reason) {
-    timer.clearTimeout(this.writeTimer);
+    timer.clearInterval(this.writeTimer);
     this.writeTimer = null;
 
     if (reason === "uninstall")
       this.purge();
     else
-      this.write();
+      this._write();
   },
 
   // True if the root is an empty object.
   get _isEmpty() {
     if (this.root && typeof(this.root) === "object") {
       let empty = true;
       for (let key in this.root) {
         empty = false;
@@ -196,50 +143,42 @@ JsonStore.prototype = {
       return empty;
     }
     return false;
   },
 
   // Writes the root to the backing file, notifying write observers when
   // complete.  If the store is over quota or if it's empty and the store has
   // never been written, nothing is written and write observers aren't notified.
-  write: Task.async(function JsonStore_write() {
+  _write: function JsonStore__write() {
     // Don't write if the root is uninitialized or if the store is empty and the
     // backing file doesn't yet exist.
     if (!this.isRootInited || (this._isEmpty && !file.exists(this.filename)))
       return;
 
-    let data = JSON.stringify(this.root);
-
     // If the store is over quota, don't write.  The current under-quota state
     // should persist.
-    if ((this.quota > 0) && (data.length > this.quota)) {
-      this.onOverQuota(this);
-      return;
-    }
-
-    // Hash the data to compare it to any previously written data
-    let hash = yield getHash(data);
-
-    if (hash == this.hash)
+    if (this.quotaUsage > 1)
       return;
 
     // Finally, write.
+    let stream = file.open(this.filename, "w");
     try {
-      yield writeData(this.filename, data);
-
-      this.hash = hash;
-      if (this.onWrite)
-        this.onWrite(this);
+      stream.writeAsync(JSON.stringify(this.root), function writeAsync(err) {
+        if (err)
+          console.error("Error writing simple storage file: " + this.filename);
+        else if (this.onWrite)
+          this.onWrite(this);
+      }.bind(this));
     }
     catch (err) {
-      console.error("Error writing simple storage file: " + this.filename);
-      console.error(err);
+      // writeAsync closes the stream after it's done, so only close on error.
+      stream.close();
     }
-  })
+  }
 };
 
 
 // This manages a JsonStore singleton and tailors its use to simple storage.
 // The root of the JsonStore is lazy-loaded:  The backing file is only read the
 // first time the root's gotten.
 let manager = ({
   jsonStore: null,
--- a/addon-sdk/source/test/test-simple-storage.js
+++ b/addon-sdk/source/test/test-simple-storage.js
@@ -1,17 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const file = require("sdk/io/file");
 const prefs = require("sdk/preferences/service");
 
 const QUOTA_PREF = "extensions.addon-sdk.simple-storage.quota";
-const WRITE_PERIOD_PREF = "extensions.addon-sdk.simple-storage.writePeriod";
 
 let {Cc,Ci} = require("chrome");
 
 const { Loader } = require("sdk/test/loader");
 const { id } = require("sdk/self");
 
 let storeFile = Cc["@mozilla.org/file/directory_service;1"].
                 getService(Ci.nsIProperties).
@@ -30,23 +29,22 @@ exports.testSetGet = function (assert, d
   let loader = Loader(module);
   let ss = loader.require("sdk/simple-storage");
   manager(loader).jsonStore.onWrite = function (storage) {
     assert.ok(file.exists(storeFilename), "Store file should exist");
 
     // Load the module again and make sure the value stuck.
     loader = Loader(module);
     ss = loader.require("sdk/simple-storage");
-    assert.equal(ss.storage.foo, val, "Value should persist");
     manager(loader).jsonStore.onWrite = function (storage) {
-      assert.fail("Nothing should be written since `storage` was not changed.");
+      file.remove(storeFilename);
+      done();
     };
+    assert.equal(ss.storage.foo, val, "Value should persist");
     loader.unload();
-    file.remove(storeFilename);
-    done();
   };
   let val = "foo";
   ss.storage.foo = val;
   assert.equal(ss.storage.foo, val, "Value read should be value set");
   loader.unload();
 };
 
 exports.testSetGetRootArray = function (assert, done) {
@@ -157,21 +155,20 @@ exports.testQuotaExceededHandle = functi
       let numProps = 0;
       for (let prop in ss.storage)
         numProps++;
       assert.ok(numProps, 2,
                   "Store should contain 2 values: " + ss.storage.toSource());
       assert.equal(ss.storage.x, 4, "x value should be correct");
       assert.equal(ss.storage.y, 5, "y value should be correct");
       manager(loader).jsonStore.onWrite = function (storage) {
-        assert.fail("Nothing should be written since `storage` was not changed.");
+        prefs.reset(QUOTA_PREF);
+        done();
       };
       loader.unload();
-      prefs.reset(QUOTA_PREF);
-      done();
     };
     loader.unload();
   });
   // This will be JSON.stringify()ed to: {"a":1,"b":2,"c":3} (19 bytes)
   ss.storage = { a: 1, b: 2, c: 3 };
   manager(loader).jsonStore.write();
 };
 
@@ -195,19 +192,16 @@ exports.testQuotaExceededNoHandle = func
       };
       loader.unload();
 
       loader = Loader(module);
       ss = loader.require("sdk/simple-storage");
       assert.equal(ss.storage, val,
                        "Over-quota value should not have been written, " +
                        "old value should have persisted: " + ss.storage);
-      manager(loader).jsonStore.onWrite = function (storage) {
-        assert.fail("Nothing should be written since `storage` was not changed.");
-      };
       loader.unload();
       prefs.reset(QUOTA_PREF);
       done();
     });
     manager(loader).jsonStore.write();
   };
 
   let val = "foo";
@@ -252,194 +246,16 @@ exports.testUninstall = function (assert
     loader.unload("uninstall");
     assert.ok(!file.exists(storeFilename), "Store file should be removed");
     done();
   };
   ss.storage.foo = "foo";
   loader.unload();
 };
 
-exports.testChangeInnerArray = function(assert, done) {
-  prefs.set(WRITE_PERIOD_PREF, 10);
-
-  let expected = {
-    x: [5, 7],
-    y: [7, 28],
-    z: [6, 2]
-  };
-
-  // Load the module, set a value.
-  let loader = Loader(module);
-  let ss = loader.require("sdk/simple-storage");
-  manager(loader).jsonStore.onWrite = function (storage) {
-    assert.ok(file.exists(storeFilename), "Store file should exist");
-
-    // Load the module again and check the result
-    loader = Loader(module);
-    ss = loader.require("sdk/simple-storage");
-    assert.equal(JSON.stringify(ss.storage),
-                     JSON.stringify(expected), "Should see the expected object");
-
-    // Add a property
-    ss.storage.x.push(["bar"]);
-    expected.x.push(["bar"]);
-    manager(loader).jsonStore.onWrite = function (storage) {
-      assert.equal(JSON.stringify(ss.storage),
-                       JSON.stringify(expected), "Should see the expected object");
-
-      // Modify a property
-      ss.storage.y[0] = 42;
-      expected.y[0] = 42;
-      manager(loader).jsonStore.onWrite = function (storage) {
-        assert.equal(JSON.stringify(ss.storage),
-                         JSON.stringify(expected), "Should see the expected object");
-
-        // Delete a property
-        delete ss.storage.z[1];
-        delete expected.z[1];
-        manager(loader).jsonStore.onWrite = function (storage) {
-          assert.equal(JSON.stringify(ss.storage),
-                           JSON.stringify(expected), "Should see the expected object");
-
-          // Modify the new inner-object
-          ss.storage.x[2][0] = "baz";
-          expected.x[2][0] = "baz";
-          manager(loader).jsonStore.onWrite = function (storage) {
-            assert.equal(JSON.stringify(ss.storage),
-                             JSON.stringify(expected), "Should see the expected object");
-
-            manager(loader).jsonStore.onWrite = function (storage) {
-              assert.fail("Nothing should be written since `storage` was not changed.");
-            };
-            loader.unload();
-
-            // Load the module again and check the result
-            loader = Loader(module);
-            ss = loader.require("sdk/simple-storage");
-            assert.equal(JSON.stringify(ss.storage),
-                             JSON.stringify(expected), "Should see the expected object");
-            loader.unload();
-            file.remove(storeFilename);
-            prefs.reset(WRITE_PERIOD_PREF);
-            done();
-          };
-        };
-      };
-    };
-  };
-
-  ss.storage = {
-    x: [5, 7],
-    y: [7, 28],
-    z: [6, 2]
-  };
-  assert.equal(JSON.stringify(ss.storage),
-                   JSON.stringify(expected), "Should see the expected object");
-
-  loader.unload();
-};
-
-exports.testChangeInnerObject = function(assert, done) {
-  prefs.set(WRITE_PERIOD_PREF, 10);
-
-  let expected = {
-    x: {
-      a: 5,
-      b: 7
-    },
-    y: {
-      c: 7,
-      d: 28
-    },
-    z: {
-      e: 6,
-      f: 2
-    }
-  };
-
-  // Load the module, set a value.
-  let loader = Loader(module);
-  let ss = loader.require("sdk/simple-storage");
-  manager(loader).jsonStore.onWrite = function (storage) {
-    assert.ok(file.exists(storeFilename), "Store file should exist");
-
-    // Load the module again and check the result
-    loader = Loader(module);
-    ss = loader.require("sdk/simple-storage");
-    assert.equal(JSON.stringify(ss.storage),
-                     JSON.stringify(expected), "Should see the expected object");
-
-    // Add a property
-    ss.storage.x.g = {foo: "bar"};
-    expected.x.g = {foo: "bar"};
-    manager(loader).jsonStore.onWrite = function (storage) {
-      assert.equal(JSON.stringify(ss.storage),
-                       JSON.stringify(expected), "Should see the expected object");
-
-      // Modify a property
-      ss.storage.y.c = 42;
-      expected.y.c = 42;
-      manager(loader).jsonStore.onWrite = function (storage) {
-        assert.equal(JSON.stringify(ss.storage),
-                         JSON.stringify(expected), "Should see the expected object");
-
-        // Delete a property
-        delete ss.storage.z.f;
-        delete expected.z.f;
-        manager(loader).jsonStore.onWrite = function (storage) {
-          assert.equal(JSON.stringify(ss.storage),
-                           JSON.stringify(expected), "Should see the expected object");
-
-          // Modify the new inner-object
-          ss.storage.x.g.foo = "baz";
-          expected.x.g.foo = "baz";
-          manager(loader).jsonStore.onWrite = function (storage) {
-            assert.equal(JSON.stringify(ss.storage),
-                             JSON.stringify(expected), "Should see the expected object");
-
-            manager(loader).jsonStore.onWrite = function (storage) {
-              assert.fail("Nothing should be written since `storage` was not changed.");
-            };
-            loader.unload();
-
-            // Load the module again and check the result
-            loader = Loader(module);
-            ss = loader.require("sdk/simple-storage");
-            assert.equal(JSON.stringify(ss.storage),
-                             JSON.stringify(expected), "Should see the expected object");
-            loader.unload();
-            file.remove(storeFilename);
-            prefs.reset(WRITE_PERIOD_PREF);
-            done();
-          };
-        };
-      };
-    };
-  };
-
-  ss.storage = {
-    x: {
-      a: 5,
-      b: 7
-    },
-    y: {
-      c: 7,
-      d: 28
-    },
-    z: {
-      e: 6,
-      f: 2
-    }
-  };
-  assert.equal(JSON.stringify(ss.storage),
-                   JSON.stringify(expected), "Should see the expected object");
-
-  loader.unload();
-};
-
 exports.testSetNoSetRead = function (assert, done) {
   // Load the module, set a value.
   let loader = Loader(module);
   let ss = loader.require("sdk/simple-storage");
   manager(loader).jsonStore.onWrite = function (storage) {
     assert.ok(file.exists(storeFilename), "Store file should exist");
 
     // Load the module again but don't access ss.storage.
@@ -448,23 +264,22 @@ exports.testSetNoSetRead = function (ass
     manager(loader).jsonStore.onWrite = function (storage) {
       assert.fail("Nothing should be written since `storage` was not accessed.");
     };
     loader.unload();
 
     // Load the module a third time and make sure the value stuck.
     loader = Loader(module);
     ss = loader.require("sdk/simple-storage");
-    assert.equal(ss.storage.foo, val, "Value should persist");
     manager(loader).jsonStore.onWrite = function (storage) {
-      assert.fail("Nothing should be written since `storage` was not changed.");
+      file.remove(storeFilename);
+      done();
     };
+    assert.equal(ss.storage.foo, val, "Value should persist");
     loader.unload();
-    file.remove(storeFilename);
-    done();
   };
   let val = "foo";
   ss.storage.foo = val;
   assert.equal(ss.storage.foo, val, "Value read should be value set");
   loader.unload();
 };
 
 
@@ -475,23 +290,22 @@ function setGetRoot(assert, done, val, c
   let loader = Loader(module);
   let ss = loader.require("sdk/simple-storage");
   manager(loader).jsonStore.onWrite = function () {
     assert.ok(file.exists(storeFilename), "Store file should exist");
 
     // Load the module again and make sure the value stuck.
     loader = Loader(module);
     ss = loader.require("sdk/simple-storage");
-    assert.ok(compare(ss.storage, val), "Value should persist");
-    manager(loader).jsonStore.onWrite = function (storage) {
-      assert.fail("Nothing should be written since `storage` was not changed.");
+    manager(loader).jsonStore.onWrite = function () {
+      file.remove(storeFilename);
+      done();
     };
+    assert.ok(compare(ss.storage, val), "Value should persist");
     loader.unload();
-    file.remove(storeFilename);
-    done();
   };
   ss.storage = val;
   assert.ok(compare(ss.storage, val), "Value read should be value set");
   loader.unload();
 }
 
 function setGetRootError(assert, done, val, msg) {
   let pred = new RegExp("storage must be one of the following types: " +