import changes from `dev' branch of rmottola/Arctic-Fox:

- Bug 1194415 - Refactor BuildProgressFooter.draw() for minor |mach build| perf improvement, r=gps (3488ffb86)
- Bug 1153691 - intTestLogging() now adds timestamps and supports param substitution. r=rnewman (507a881c2)
- reenable asserts (97032f833)
- re-enable specific Windows optimization (7a581da03)
- Bug 1149815 - Pass storage function failure codes through to callers, r=asuth. (c54838b5d)
- Bug 1149373 - Ensure mozStorage async threads are shut down. r=bent (a511cfefc)
- namespace style (7a567650f)
- Bug 1193022 - clean up reference-counting in storage/; r=mak (49f115c52)
- Bug 1155193 Proxy release the Connection in mozStorageService::unregisterConnection(). r=asuth (bb9311ee5)
- bug 1189896 - Do not preallocate Sqlite connections caches for now. r=asuth (210115e89)
- Bug 1155846 - Comment out intentionally unreachable code and unused parameters in Prefetcher.jsm. r=billm (8757cba52)
- Bug 1166886 - Comment out some code that is supposed to be disabled (r=mconley) (ec5b2bd30)
- Bug 1040285 - Single Quotes should not be encoded in the path r=mcmanus,annevk (0a47fdc2f)
- Bug 1040285 - Single Quotes in HTTP request-uri Are Incorrectly Encoded as %27 r=MattN (dcce00624)
- Bug 1125989 - Avoid OS.File request lossage during worker shutdown, r=yoric (e8e8cab17)
- Bug 1164822 - Fix OS.File.remove not throwing with unexisting files. r=Yoric (dd57a069f)
- Bug 1123372 - Remove use of .toLocaleFormat() from Places. r=mak (fbfbd7fa0)

db/sqlite3/src/sqlite.def

@@ -107,6 +107,7 @@ EXPORTS
         sqlite3_result_double
         sqlite3_result_error
         sqlite3_result_error16
+        sqlite3_result_error_code
         sqlite3_result_error_nomem
         sqlite3_result_int
         sqlite3_result_int64

netwerk/test/unit/test_bug376844.js

@@ -1,7 +1,7 @@
 const testURLs = [
   ["http://example.com/<", "http://example.com/%3C"],
   ["http://example.com/>", "http://example.com/%3E"],
-  ["http://example.com/'", "http://example.com/%27"],
+  ["http://example.com/'", "http://example.com/'"],
   ["http://example.com/\"", "http://example.com/%22"],
   ["http://example.com/?<", "http://example.com/?%3C"],
   ["http://example.com/?>", "http://example.com/?%3E"],

netwerk/test/unit/test_standardurl.js

@@ -235,6 +235,14 @@ function test_escapeQueryBrackets()
   do_check_eq(url.spec, "http://[2001::1]/?a%5Bx%5D=1");
 }
 
+function test_apostropheEncoding()
+{
+  // For now, single quote is escaped everywhere _except_ the path.
+  // This policy is controlled by the bitmask in nsEscape.cpp::EscapeChars[]
+  var url = stringToURL("http://example.com/dir'/file'.ext'");
+  do_check_eq(url.spec, "http://example.com/dir'/file'.ext'");
+}
+
 function run_test()
 {
   test_setEmptyPath();
@@ -244,4 +252,5 @@ function run_test()
   test_ipv6_fail();
   test_clearedSpec();
   test_escapeQueryBrackets();
+  test_apostropheEncoding();
 }

python/mozbuild/mozbuild/controller/building.py

@@ -67,7 +67,7 @@ class TierStatus(object):
     def __init__(self, resources):
         """Accepts a SystemResourceMonitor to record results against."""
         self.tiers = OrderedDict()
-        self.active_tiers = set()
+        self.tier_status = OrderedDict()
         self.resources = resources
 
     def set_tiers(self, tiers):
@@ -78,29 +78,23 @@ class TierStatus(object):
                 finish_time=None,
                 duration=None,
             )
+            self.tier_status[tier] = None
 
     def begin_tier(self, tier):
         """Record that execution of a tier has begun."""
+        self.tier_status[tier] = 'active'
         t = self.tiers[tier]
         # We should ideally use a monotonic clock here. Unfortunately, we won't
         # have one until Python 3.
         t['begin_time'] = time.time()
         self.resources.begin_phase(tier)
-        self.active_tiers.add(tier)
 
     def finish_tier(self, tier):
         """Record that execution of a tier has finished."""
+        self.tier_status[tier] = 'finished'
         t = self.tiers[tier]
         t['finish_time'] = time.time()
         t['duration'] = self.resources.finish_phase(tier)
-        self.active_tiers.remove(tier)
-
-    def tier_status(self):
-        for tier, state in self.tiers.items():
-            active = tier in self.active_tiers
-            finished = state['finish_time'] is not None
-
-            yield tier, active, finished
 
     def tiered_resource_usage(self):
         """Obtains an object containing resource usage for tiers.

python/mozbuild/mozbuild/mach_commands.py

@@ -5,6 +5,7 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 import argparse
+import itertools
 import json
 import logging
 import operator
@@ -125,21 +126,17 @@ class BuildProgressFooter(object):
         # terminal is a blessings.Terminal.
         self._t = terminal
         self._fh = sys.stdout
-        self._monitor = monitor
-
-    def _clear_lines(self, n):
-        self._fh.write(self._t.move_x(0))
-        self._fh.write(self._t.clear_eos())
+        self.tiers = monitor.tiers.tier_status.viewitems()
 
     def clear(self):
         """Removes the footer from the current terminal."""
-        self._clear_lines(1)
+        self._fh.write(self._t.move_x(0))
+        self._fh.write(self._t.clear_eos())
 
     def draw(self):
         """Draws this footer in the terminal."""
-        tiers = self._monitor.tiers
 
-        if not tiers.tiers:
+        if not self.tiers:
             return
 
         # The drawn terminal looks something like:
@@ -148,15 +145,15 @@ class BuildProgressFooter(object):
         # This is a list of 2-tuples of (encoding function, input). None means
         # no encoding. For a full reason on why we do things this way, read the
         # big comment below.
-        parts = [('bold', 'TIER'), ':', ' ']
-
-        for tier, active, finished in tiers.tier_status():
-            if active:
-                parts.extend([('underline_yellow', tier), ' '])
-            elif finished:
-                parts.extend([('green', tier), ' '])
+        parts = [('bold', 'TIER:')]
+        append = parts.append
+        for tier, status in self.tiers:
+            if status is None:
+                append(tier)
+            elif status == 'finished':
+                append(('green', tier))
             else:
-                parts.extend([tier, ' '])
+                append(('underline_yellow', tier))
 
         # We don't want to write more characters than the current width of the
         # terminal otherwise wrapping may result in weird behavior. We can't
@@ -168,30 +165,25 @@ class BuildProgressFooter(object):
         written = 0
         write_pieces = []
         for part in parts:
-            if isinstance(part, tuple):
-                func, arg = part
+            try:
+                func, part = part
+                encoded = getattr(self._t, func)(part)
+            except ValueError:
+                encoded = part
 
-                if written + len(arg) > max_width:
-                    write_pieces.append(arg[0:max_width - written])
-                    written += len(arg)
-                    break
+            len_part = len(part)
+            len_spaces = len(write_pieces)
+            if written + len_part + len_spaces > max_width:
+                write_pieces.append(part[0:max_width - written - len_spaces])
+                written += len_part
+                break
 
-                encoded = getattr(self._t, func)(arg)
+            write_pieces.append(encoded)
+            written += len_part
 
-                write_pieces.append(encoded)
-                written += len(arg)
-            else:
-                if written + len(part) > max_width:
-                    write_pieces.append(part[0:max_width - written])
-                    written += len(part)
-                    break
-
-                write_pieces.append(part)
-                written += len(part)
         with self._t.location():
             self._t.move(self._t.height-1,0)
-            self._fh.write(''.join(write_pieces))
-        self._fh.flush()
+            self._fh.write(' '.join(write_pieces))
 
 
 class BuildOutputManager(LoggingMixin):

services/common/modules-testing/logging.js

@@ -23,11 +23,11 @@ this.initTestLogging = function initTestLogging(level) {
         this.errorsLogged += 1;
       }
 
-      return message.loggerName + "\t" + message.levelDesc + "\t" +
-        message.message + "\n";
+      return message.time + "\t" + message.loggerName + "\t" + message.levelDesc + "\t" +
+        this.formatText(message) + "\n";
     }
   };
-  LogStats.prototype.__proto__ = new Log.Formatter();
+  LogStats.prototype.__proto__ = new Log.BasicFormatter();
 
   let log = Log.repository.rootLogger;
   let logStats = new LogStats();

storage/FileSystemModule.cpp

@@ -267,7 +267,7 @@ int RowId(sqlite3_vtab_cursor* aCursor, sqlite3_int64* aRowid)
   return SQLITE_OK;
 }
 
-} // anonymous namespace
+} // namespace
 
 namespace mozilla {
 namespace storage {

storage/IStorageBindingParamsInternal.h

@@ -45,7 +45,7 @@ NS_DEFINE_STATIC_IID_ACCESSOR(IStorageBindingParamsInternal,
 #define NS_DECL_ISTORAGEBINDINGPARAMSINTERNAL \
   already_AddRefed<mozIStorageError> bind(sqlite3_stmt *aStatement) override;
 
-} // storage
-} // mozilla
+} // namespace storage
+} // namespace mozilla
 
 #endif // mozilla_storage_IStorageBindingParamsInternal_h_

storage/SQLCollations.cpp

@@ -101,7 +101,7 @@ struct Collations {
   int(*xCompare)(void*, int, const void*, int, const void*);
 };
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Exposed Functions

storage/SQLiteMutex.h

@@ -107,15 +107,15 @@ public:
   void assertCurrentThreadOwns()
   {
     NS_ASSERTION(mMutex, "No mutex associated with this wrapper!");
-//    NS_ASSERTION(sqlite3_mutex_held(mMutex),
-//                 "Mutex is not held, but we expect it to be!");
+    NS_ASSERTION(sqlite3_mutex_held(mMutex),
+                 "Mutex is not held, but we expect it to be!");
   }
 
   void assertNotCurrentThreadOwns()
   {
     NS_ASSERTION(mMutex, "No mutex associated with this wrapper!");
-//    NS_ASSERTION(sqlite3_mutex_notheld(mMutex),
-//                 "Mutex is held, but we expect it to not be!");
+    NS_ASSERTION(sqlite3_mutex_notheld(mMutex),
+                 "Mutex is held, but we expect it to not be!");
   }
 #endif // ifndef DEBUG
 

storage/StorageBaseStatementInternal.h

@@ -347,7 +347,7 @@ NS_DEFINE_STATIC_IID_ACCESSOR(StorageBaseStatementInternal,
 
   
 
-} // storage
-} // mozilla
+} // namespace storage
+} // namespace mozilla
 
 #endif // mozilla_storage_StorageBaseStatementInternal_h_

storage/TelemetryVFS.cpp

@@ -107,6 +107,21 @@ public:
       Telemetry::AccumulateTimeDelta(static_cast<Telemetry::ID>(id + mainThread),
                                      start, end);
     }
+    // We don't report SQLite I/O on Windows because we have a comprehensive
+    // mechanism for intercepting I/O on that platform that captures a superset
+    // of the data captured here.
+#if defined(MOZ_ENABLE_PROFILER_SPS) && !defined(XP_WIN)
+    if (IOInterposer::IsObservedOperation(op)) {
+      const char* main_ref  = "sqlite-mainthread";
+      const char* other_ref = "sqlite-otherthread";
+
+      // Create observation
+      IOInterposeObserver::Observation ob(op, start, end,
+                                          (mainThread ? main_ref : other_ref));
+      // Report observation
+      IOInterposer::Report(ob);
+    }
+#endif /* defined(MOZ_ENABLE_PROFILER_SPS) && !defined(XP_WIN) */
   }
 
 private:
@@ -808,7 +823,7 @@ xNextSystemCall(sqlite3_vfs *vfs, const char *zName)
   return orig_vfs->xNextSystemCall(orig_vfs, zName);
 }
 
-}
+} // namespace
 
 namespace mozilla {
 namespace storage {
@@ -873,5 +888,5 @@ sqlite3_vfs* ConstructTelemetryVFS()
   return tvfs;
 }
 
-}
-}
+} // namespace storage
+} // namespace mozilla

storage/VacuumManager.cpp

@@ -298,7 +298,7 @@ Vacuumer::notifyCompletion(bool aSucceeded)
   return NS_OK;
 }
 
-} // Anonymous namespace.
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// VacuumManager

storage/mozStorageAsyncStatement.h

@@ -101,7 +101,7 @@ private:
   friend class AsyncStatementJSHelper;
 };
 
-} // storage
-} // mozilla
+} // namespace storage
+} // namespace mozilla
 
 #endif // mozilla_storage_mozStorageAsyncStatement_h_

storage/mozStorageAsyncStatementExecution.cpp

@@ -155,7 +155,7 @@ private:
   ExecutionState mReason;
 };
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// AsyncExecuteStatements
@@ -191,7 +191,7 @@ AsyncExecuteStatements::execute(StatementDataArray &aStatements,
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Return it as the pending statement object and track it.
-  NS_ADDREF(*_stmt = event);
+  event.forget(_stmt);
   return NS_OK;
 }
 

storage/mozStorageBindingParams.cpp

@@ -98,7 +98,7 @@ sqlite3_T_blob(BindingColumnData aData,
 
 #include "variantToSQLiteT_impl.h"
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// BindingParams

storage/mozStorageConnection.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/Mutex.h"
 #include "mozilla/CondVar.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/ErrorNames.h"
 
 #include "mozIStorageAggregateFunction.h"
 #include "mozIStorageCompletionCallback.h"
@@ -67,6 +68,43 @@ namespace storage {
 
 namespace {
 
+int
+nsresultToSQLiteResult(nsresult aXPCOMResultCode)
+{
+  if (NS_SUCCEEDED(aXPCOMResultCode)) {
+    return SQLITE_OK;
+  }
+
+  switch (aXPCOMResultCode) {
+    case NS_ERROR_FILE_CORRUPTED:
+      return SQLITE_CORRUPT;
+    case NS_ERROR_FILE_ACCESS_DENIED:
+      return SQLITE_CANTOPEN;
+    case NS_ERROR_STORAGE_BUSY:
+      return SQLITE_BUSY;
+    case NS_ERROR_FILE_IS_LOCKED:
+      return SQLITE_LOCKED;
+    case NS_ERROR_FILE_READ_ONLY:
+      return SQLITE_READONLY;
+    case NS_ERROR_STORAGE_IOERR:
+      return SQLITE_IOERR;
+    case NS_ERROR_FILE_NO_DEVICE_SPACE:
+      return SQLITE_FULL;
+    case NS_ERROR_OUT_OF_MEMORY:
+      return SQLITE_NOMEM;
+    case NS_ERROR_UNEXPECTED:
+      return SQLITE_MISUSE;
+    case NS_ERROR_ABORT:
+      return SQLITE_ABORT;
+    case NS_ERROR_STORAGE_CONSTRAINT:
+      return SQLITE_CONSTRAINT;
+    default:
+      return SQLITE_ERROR;
+  }
+
+  MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE("Must return in switch above!");
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Variant Specialization Functions (variantToSQLiteT)
 
@@ -212,11 +250,17 @@ basicFunctionHelper(sqlite3_context *aCtx,
       return;
 
   nsCOMPtr<nsIVariant> result;
-  if (NS_FAILED(func->OnFunctionCall(arguments, getter_AddRefs(result)))) {
-    NS_WARNING("User function returned error code!");
-    ::sqlite3_result_error(aCtx,
-                           "User function returned error code",
-                           -1);
+  nsresult rv = func->OnFunctionCall(arguments, getter_AddRefs(result));
+  if (NS_FAILED(rv)) {
+    nsAutoCString errorMessage;
+    GetErrorName(rv, errorMessage);
+    errorMessage.InsertLiteral("User function returned ", 0);
+    errorMessage.Append('!');
+
+    NS_WARNING(errorMessage.get());
+
+    ::sqlite3_result_error(aCtx, errorMessage.get(), -1);
+    ::sqlite3_result_error_code(aCtx, nsresultToSQLiteResult(rv));
     return;
   }
   int retcode = variantToSQLiteT(aCtx, result);
@@ -331,7 +375,7 @@ WaitForUnlockNotify(sqlite3* aDatabase)
   return srv;
 }
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Local Classes
@@ -361,6 +405,10 @@ public:
     MOZ_ASSERT(onAsyncThread);
 #endif // DEBUG
 
+    nsCOMPtr<nsIRunnable> event = NS_NewRunnableMethodWithArg<nsCOMPtr<nsIThread>>
+      (mConnection, &Connection::shutdownAsyncThread, mAsyncExecutionThread);
+    (void)NS_DispatchToMainThread(event);
+
     // Internal close.
     (void)mConnection->internalClose(mNativeConnection);
 
@@ -469,7 +517,7 @@ private:
   nsCOMPtr<mozIStorageCompletionCallback> mCallback;
 };
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Connection
@@ -482,6 +530,7 @@ Connection::Connection(Service *aService,
 , threadOpenedOn(do_GetCurrentThread())
 , mDBConn(nullptr)
 , mAsyncExecutionThreadShuttingDown(false)
+, mAsyncExecutionThreadIsAlive(false)
 , mConnectionClosed(false)
 , mTransactionInProgress(false)
 , mProgressHandler(nullptr)
@@ -498,6 +547,8 @@ Connection::~Connection()
 
   MOZ_ASSERT(!mAsyncExecutionThread,
              "AsyncClose has not been invoked on this connection!");
+  MOZ_ASSERT(!mAsyncExecutionThreadIsAlive,
+             "The async execution thread should have been shutdown!");
 }
 
 NS_IMPL_ADDREF(Connection)
@@ -565,6 +616,7 @@ Connection::getAsyncExecutionTarget()
                              mAsyncExecutionThread);
   }
 
+  mAsyncExecutionThreadIsAlive = true;
   return mAsyncExecutionThread;
 }
 
@@ -888,6 +940,17 @@ Connection::isClosed()
   return mConnectionClosed;
 }
 
+void
+Connection::shutdownAsyncThread(nsIThread *aThread) {
+  MOZ_ASSERT(!mAsyncExecutionThread);
+  MOZ_ASSERT(mAsyncExecutionThreadIsAlive);
+  MOZ_ASSERT(mAsyncExecutionThreadShuttingDown);
+
+  DebugOnly<nsresult> rv = aThread->Shutdown();
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  mAsyncExecutionThreadIsAlive = false;
+}
+
 nsresult
 Connection::internalClose(sqlite3 *aNativeConnection)
 {
@@ -1520,7 +1583,7 @@ Connection::ExecuteSimpleSQLAsync(const nsACString &aSQLStatement,
     return rv;
   }
 
-  NS_ADDREF(*_handle = pendingStatement);
+  pendingStatement.forget(_handle);
   return rv;
 }
 

storage/mozStorageConnection.h

@@ -166,6 +166,11 @@ public:
    */
   nsresult internalClose(sqlite3 *aDBConn);
 
+  /**
+   * Shuts down the passed-in async thread.
+   */
+  void shutdownAsyncThread(nsIThread *aAsyncThread);
+
   /**
    * Obtains the filename of the connection.  Useful for logging.
    */
@@ -311,6 +316,12 @@ private:
    */
   bool mAsyncExecutionThreadShuttingDown;
 
+  /**
+   * Tracks whether the async thread has been initialized and Shutdown() has
+   * not yet been invoked on it.
+   */
+  DebugOnly<bool> mAsyncExecutionThreadIsAlive;
+
   /**
    * Set to true just prior to calling sqlite3_close on the
    * connection.

storage/mozStorageError.h

@@ -29,7 +29,7 @@ private:
   nsCString mMessage;
 };
 
-} // namespace stoarge
+} // namespace storage
 } // namespace mozilla
 
 #endif // mozStorageError_h

storage/mozStoragePrivateHelpers.cpp

@@ -259,7 +259,7 @@ public:
 private:
   nsCOMPtr<mozIStorageCompletionCallback> mCallback;
 };
-} // anonymous namespace
+} // namespace
 already_AddRefed<nsIRunnable>
 newCompletionEvent(mozIStorageCompletionCallback *aCallback)
 {

storage/mozStorageSQLFunctions.cpp

@@ -279,7 +279,7 @@ struct Functions {
   void (*xFunc)(::sqlite3_context*, int, sqlite3_value**);
 };
 
-} // anonymous namespace
+} // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Exposed Functions

storage/mozStorageService.cpp

@@ -324,9 +324,23 @@ Service::unregisterConnection(Connection *aConnection)
   {
     mRegistrationMutex.AssertNotCurrentThreadOwns();
     MutexAutoLock mutex(mRegistrationMutex);
-    DebugOnly<bool> removed = mConnections.RemoveElement(aConnection);
-    // Assert if we try to unregister a non-existent connection.
-    MOZ_ASSERT(removed);
+
+    for (uint32_t i = 0 ; i < mConnections.Length(); ++i) {
+      if (mConnections[i] == aConnection) {
+        nsCOMPtr<nsIThread> thread = mConnections[i]->threadOpenedOn;
+
+        // Ensure the connection is released on its opening thread.  Note, we
+        // must use .forget().take() so that we can manually cast to an
+        // unambiguous nsISupports type.
+        NS_ProxyRelease(thread,
+          static_cast<mozIStorageConnection*>(mConnections[i].forget().take()));
+
+        mConnections.RemoveElementAt(i);
+        return;
+      }
+    }
+
+    MOZ_ASSERT_UNREACHABLE("Attempt to unregister unknown storage connection!");
   }
 }
 
@@ -496,7 +510,7 @@ const sqlite3_mem_methods memMethods = {
   nullptr
 };
 
-} // anonymous namespace
+} // namespace
 
 #endif  // MOZ_STORAGE_MEMORY
 
@@ -519,6 +533,10 @@ Service::initialize()
     return convertResultCode(rc);
 #endif
 
+  // TODO (bug 1191405): do not preallocate the connections caches until we
+  // have figured the impact on our consumers and memory.
+  sqlite3_config(SQLITE_CONFIG_PAGECACHE, NULL, 0, 0);
+
   // Explicitly initialize sqlite3.  Although this is implicitly called by
   // various sqlite3 functions (and the sqlite3_open calls in our case),
   // the documentation suggests calling this directly.  So we do.
@@ -740,7 +758,7 @@ private:
   nsRefPtr<mozIStorageCompletionCallback> mCallback;
 };
 
-} // anonymous namespace
+} // namespace
 
 NS_IMETHODIMP
 Service::OpenAsyncDatabase(nsIVariant *aDatabaseStore,

storage/mozStorageStatement.h

@@ -112,7 +112,7 @@ private:
   friend class StatementJSHelper;
 };
 
-} // storage
-} // mozilla
+} // namespace storage
+} // namespace mozilla
 
 #endif // mozStorageStatement_h

storage/mozStorageStatementRow.cpp

@@ -152,4 +152,4 @@ StatementRow::Resolve(nsIXPConnectWrappedNative *aWrapper,
 }
 
 } // namespace storage
-} // namescape mozilla
+} // namespace mozilla

toolkit/components/osfile/modules/osfile_async_front.jsm

@@ -146,7 +146,10 @@ function summarizeObject(obj) {
   return obj;
 }
 
-let Scheduler = {
+// In order to expose Scheduler to the unfiltered Cu.import return value variant
+// on B2G we need to save it to `this`.  This does not make it public;
+// EXPORTED_SYMBOLS still controls that in all cases.
+let Scheduler = this.Scheduler = {
 
   /**
    * |true| once we have sent at least one message to the worker.
@@ -268,38 +271,43 @@ let Scheduler = {
     // Grab the kill queue to make sure that we
     // cannot be interrupted by another call to `kill`.
     let killQueue = this._killQueue;
+
+    // Deactivate the queue, to ensure that no message is sent
+    // to an obsolete worker (we reactivate it in the `finally`).
+    // This needs to be done right now so that we maintain relative
+    // ordering with calls to post(), etc.
+    let deferred = Promise.defer();
+    let savedQueue = this.queue;
+    this.queue = deferred.promise;
+
     return this._killQueue = Task.spawn(function*() {
 
       yield killQueue;
       // From this point, and until the end of the Task, we are the
       // only call to `kill`, regardless of any `yield`.
 
-      yield this.queue;
-
-      // Enter critical section: no yield in this block
-      // (we want to make sure that we remain the only
-      // request in the queue).
-
-      if (!this.launched || this.shutdown || !this._worker) {
-        // Nothing to kill
-        this.shutdown = this.shutdown || shutdown;
-        this._worker = null;
-        return null;
-      }
-
-      // Deactivate the queue, to ensure that no message is sent
-      // to an obsolete worker (we reactivate it in the |finally|).
-      let deferred = Promise.defer();
-      this.queue = deferred.promise;
-
-
-      // Exit critical section
-
-      let message = ["Meta_shutdown", [reset]];
+      yield savedQueue;
 
       try {
+        // Enter critical section: no yield in this block
+        // (we want to make sure that we remain the only
+        // request in the queue).
+
+        if (!this.launched || this.shutdown || !this._worker) {
+          // Nothing to kill
+          this.shutdown = this.shutdown || shutdown;
+          this._worker = null;
+          return null;
+        }
+
+        // Exit critical section
+
+        let message = ["Meta_shutdown", [reset]];
+
         Scheduler.latestReceived = [];
-        Scheduler.latestSent = [Date.now(), ...message];
+        Scheduler.latestSent = [Date.now(),
+          Task.Debugging.generateReadableStack(new Error().stack),
+          ...message];
 
         // Wait for result
         let resources;
@@ -975,7 +983,7 @@ if (!SharedAll.Constants.Win) {
 /**
  * Gets the number of bytes available on disk to the current user.
  *
- * @param {string} Platform-specific path to a directory on the disk to 
+ * @param {string} Platform-specific path to a directory on the disk to
  * query for free available bytes.
  *
  * @return {number} The number of bytes available for the current user.
@@ -1004,10 +1012,15 @@ File.removeEmptyDir = function removeEmptyDir(path, options) {
  * Remove an existing file.
  *
  * @param {string} path The name of the file.
+ * @param {*=} options Additional options.
+ *   - {bool} ignoreAbsent If |false|, throw an error if the file does
+ *     not exist. |true| by default.
+ *
+ * @throws {OS.File.Error} In case of I/O error.
  */
-File.remove = function remove(path) {
+File.remove = function remove(path, options) {
   return Scheduler.post("remove",
-    [Type.path.toMsg(path)]);
+    [Type.path.toMsg(path), options], path);
 };
 
 

toolkit/components/osfile/modules/osfile_async_worker.js

@@ -217,8 +217,8 @@ if (this.Components) {
    removeEmptyDir: function removeEmptyDir(path, options) {
      return File.removeEmptyDir(Type.path.fromMsg(path), options);
    },
-   remove: function remove(path) {
-     return File.remove(Type.path.fromMsg(path));
+   remove: function remove(path, options) {
+     return File.remove(Type.path.fromMsg(path), options);
    },
    open: function open(path, mode, options) {
      let filePath = Type.path.fromMsg(path);

toolkit/components/osfile/modules/ospath_unix.jsm

@@ -163,14 +163,14 @@ exports.split = split;
  * Returns the file:// URI file path of the given local file path.
  */
 // The case of %3b is designed to match Services.io, but fundamentally doesn't matter.
-let toFileURIExtraEncodings = {';': '%3b', '?': '%3F', "'": '%27', '#': '%23'};
+let toFileURIExtraEncodings = {';': '%3b', '?': '%3F', '#': '%23'};
 let toFileURI = function toFileURI(path) {
   let uri = encodeURI(this.normalize(path));
 
   // add a prefix, and encodeURI doesn't escape a few characters that we do
   // want to escape, so fix that up
   let prefix = "file://";
-  uri = prefix + uri.replace(/[;?'#]/g, match => toFileURIExtraEncodings[match]);
+  uri = prefix + uri.replace(/[;?#]/g, match => toFileURIExtraEncodings[match]);
 
   return uri;
 };

toolkit/components/osfile/modules/ospath_win.jsm

@@ -306,7 +306,7 @@ exports.split = split;
  * Return the file:// URI file path of the given local file path.
  */
 // The case of %3b is designed to match Services.io, but fundamentally doesn't matter.
-let toFileURIExtraEncodings = {';': '%3b', '?': '%3F', "'": '%27', '#': '%23'};
+let toFileURIExtraEncodings = {';': '%3b', '?': '%3F', '#': '%23'};
 let toFileURI = function toFileURI(path) {
   // URI-escape forward slashes and convert backward slashes to forward
   path = this.normalize(path).replace(/[\\\/]/g, m => (m=='\\')? '/' : '%2F');
@@ -315,7 +315,7 @@ let toFileURI = function toFileURI(path) {
   // add a prefix, and encodeURI doesn't escape a few characters that we do
   // want to escape, so fix that up
   let prefix = "file:///";
-  uri = prefix + uri.replace(/[;?'#]/g, match => toFileURIExtraEncodings[match]);
+  uri = prefix + uri.replace(/[;?#]/g, match => toFileURIExtraEncodings[match]);
 
   // turn e.g., file:///C: into file:///C:/
   if (uri.charAt(uri.length - 1) === ':') {

toolkit/components/osfile/tests/xpcshell/test_osfile_kill.js

@@ -0,0 +1,100 @@
+"use strict";
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+
+// We want the actual global to get at the internals since Scheduler is not
+// exported.
+var AsyncFrontGlobal = Components.utils.import(
+                         "resource://gre/modules/osfile/osfile_async_front.jsm",
+                          null);
+var Scheduler = AsyncFrontGlobal.Scheduler;
+
+/**
+ * Verify that Scheduler.kill() interacts with other OS.File requests correctly,
+ * and that no requests are lost.  This is relevant because on B2G we
+ * auto-kill the worker periodically, making it very possible for valid requests
+ * to be interleaved with the automatic kill().
+ *
+ * This test is being created with the fix for Bug 1125989 where `kill` queue
+ * management was found to be buggy.  It is a glass-box test that explicitly
+ * re-creates the observed failure situation; it is not guaranteed to prevent
+ * all future regressions.  The following is a detailed explanation of the test
+ * for your benefit if this test ever breaks or you are wondering what was the
+ * point of all this.  You might want to skim the code below first.
+ *
+ * OS.File maintains a `queue` of operations to be performed.  This queue is
+ * nominally implemented as a chain of promises.  Every time a new job is
+ * OS.File.push()ed, it effectively becomes the new `queue` promise.  (An
+ * extra promise is interposed with a rejection handler to avoid the rejection
+ * cascading, but that does not matter for our purposes.)
+ *
+ * The flaw in `kill` was that it would wait for the `queue` to complete before
+ * replacing `queue`.  As a result, another OS.File operation could use `push`
+ * (by way of OS.File.post()) to also use .then() on the same `queue` promise.
+ * Accordingly, assuming that promise was not yet resolved (due to a pending
+ * OS.File request), when it was resolved, both the task scheduled in `kill`
+ * and in `post` would be triggered.  Both of those tasks would run until
+ * encountering a call to worker.post().
+ *
+ * Re-creating this race is not entirely trivial because of the large number of
+ * promises used by the code causing control flow to repeatedly be deferred.  In
+ * a slightly simpler world we could run the follwing in the same turn of the
+ * event loop and trigger the problem.
+ * - any OS.File request
+ * - Scheduler.kill()
+ * - any OS.File request
+ *
+ * However, we need the Scheduler.kill task to reach the point where it is
+ * waiting on the same `queue` that another task has been scheduled against.
+ * Since the `kill` task yields on the `killQueue` promise prior to yielding
+ * on `queue`, however, some turns of the event loop are required.  Happily,
+ * for us, as discussed above, the problem triggers when we have two promises
+ * scheduled on the `queue`, so we can just wait to schedule the second OS.File
+ * request on the queue.  (Note that because of the additional then() added to
+ * eat rejections, there is an important difference between the value of
+ * `queue` and the value returned by the first OS.File request.)
+ */
+add_task(function* test_kill_race() {
+  // Ensure the worker has been created and that SET_DEBUG has taken effect.
+  // We have chosen OS.File.exists for our tests because it does not trigger
+  // a rejection and we absolutely do not care what the operation is other
+  // than it does not invoke a native fast-path.
+  yield OS.File.exists('foo.foo');
+
+  do_print('issuing first request');
+  let firstRequest = OS.File.exists('foo.bar');
+  let secondRequest;
+  let secondResolved = false;
+
+  // As noted in our big block comment, we want to wait to schedule the
+  // second request so that it races `kill`'s call to `worker.post`.  Having
+  // ourselves wait on the same promise, `queue`, and registering ourselves
+  // before we issue the kill request means we will get run before the `kill`
+  // task resumes and allow us to precisely create the desired race.
+  Scheduler.queue.then(function() {
+    do_print('issuing second request');
+    secondRequest = OS.File.exists('foo.baz');
+    secondRequest.then(function() {
+      secondResolved = true;
+    });
+  });
+
+  do_print('issuing kill request');
+  let killRequest = Scheduler.kill({ reset: true, shutdown: false });
+
+  // Wait on the killRequest so that we can schedule a new OS.File request
+  // after it completes...
+  yield killRequest;
+  // ...because our ordering guarantee ensures that there is at most one
+  // worker (and this usage here should not be vulnerable even with the
+  // bug present), so when this completes the secondRequest has either been
+  // resolved or lost.
+  yield OS.File.exists('foo.goz');
+
+  ok(secondResolved,
+     'The second request was resolved so we avoided the bug. Victory!');
+});
+
+function run_test() {
+  run_next_test();
+}

toolkit/components/osfile/tests/xpcshell/xpcshell.ini

@@ -39,6 +39,7 @@ support-files =
 [test_queue.js]
 [test_loader.js]
 [test_constants.js]
+[test_osfile_kill.js]
 
 # Unimplemented on Windows (bug 1022816).
 # Spurious failure on Android test farm due to non-POSIX behavior of

toolkit/components/perfmonitoring/AddonWatcher.jsm

@@ -223,11 +223,11 @@ let AddonWatcher = {
               Cu.reportError(ex.stack);
             }
           }
-	}
+        }
       } catch (ex) {
         Cu.reportError("Error in AddonWatcher._checkAddons " + ex);
         Cu.reportError(Task.Debugging.generateReadableStack(ex.stack));
-        }
+      }
     }.bind(this));
   },
   ignoreAddonForSession: function(addonid) {

toolkit/components/places/PlacesBackups.jsm

@@ -218,6 +218,24 @@ this.PlacesBackups = {
     }.bind(this));
   },
 
+  /**
+   * Generates a ISO date string (YYYY-MM-DD) from a Date object.
+   *
+   * @param dateObj
+   *        The date object to parse.
+   * @return an ISO date string.
+   */
+   toISODateString: function toISODateString(dateObj) {
+    if (!dateObj || dateObj.constructor.name != "Date" || !dateObj.getTime())
+      throw new Error("invalid date object");
+    let padDate = val => ("0" + val).substr(-2, 2);
+    return [
+      dateObj.getFullYear(),
+      padDate(dateObj.getMonth() + 1),
+      padDate(dateObj.getDate())
+    ].join("-");
+   },
+
   /**
    * Creates a filename for bookmarks backup files.
    *
@@ -233,7 +251,7 @@ this.PlacesBackups = {
     let dateObj = aDateObj || new Date();
     // Use YYYY-MM-DD (ISO 8601) as it doesn't contain illegal characters
     // and makes the alphabetical order of multiple backup files more useful.
-      return "bookmarks-" + dateObj.toLocaleFormat("%Y-%m-%d") + ".json" +
+      return "bookmarks-" + PlacesBackups.toISODateString(dateObj) + ".json" +
                             (aCompress ? "lz4" : "");
   },
 

toolkit/components/places/tests/bookmarks/test_1016953-renaming-uncompressed.js

@@ -20,7 +20,7 @@ add_task(function* test_same_date_same_hash() {
 
   // Save JSON file in backup folder with hash appended
   let dateObj = new Date();
-  let filename = "bookmarks-" + dateObj.toLocaleFormat("%Y-%m-%d") + "_" +
+  let filename = "bookmarks-" + PlacesBackups.toISODateString(dateObj) + "_" +
                   count + "_" + hash + ".json";
   let backupFile = OS.Path.join(backupFolder, filename);
   yield OS.File.move(tempPath, backupFile);
@@ -53,7 +53,7 @@ add_task(function* test_same_date_diff_hash() {
                               "bug10169583_bookmarks.json");
   let {count, hash} = yield BookmarkJSONUtils.exportToFile(tempPath);
   let dateObj = new Date();
-  let filename = "bookmarks-" + dateObj.toLocaleFormat("%Y-%m-%d") + "_" +
+  let filename = "bookmarks-" + PlacesBackups.toISODateString(dateObj) + "_" +
                   count + "_" + "differentHash==" + ".json";
   let backupFile = OS.Path.join(backupFolder, filename);
   yield OS.File.move(tempPath, backupFile);
@@ -84,9 +84,9 @@ add_task(function* test_diff_date_same_hash() {
   let {count, hash} = yield BookmarkJSONUtils.exportToFile(tempPath);
   let oldDate = new Date(2014, 1, 1);
   let curDate = new Date();
-  let oldFilename = "bookmarks-" + oldDate.toLocaleFormat("%Y-%m-%d") + "_" +
+  let oldFilename = "bookmarks-" + PlacesBackups.toISODateString(oldDate) + "_" +
                   count + "_" + hash + ".json";
-  let newFilename = "bookmarks-" + curDate.toLocaleFormat("%Y-%m-%d") + "_" +
+  let newFilename = "bookmarks-" + PlacesBackups.toISODateString(curDate) + "_" +
                   count + "_" + hash + ".json";
   let backupFile = OS.Path.join(backupFolder, oldFilename);
   let newBackupFile = OS.Path.join(backupFolder, newFilename);

toolkit/components/places/tests/bookmarks/test_477583_json-backup-in-future.js

@@ -21,7 +21,7 @@ function run_test() {
     let dateObj = new Date();
     dateObj.setYear(dateObj.getFullYear() + 1);
     let name = PlacesBackups.getFilenameForDate(dateObj);
-    do_check_eq(name, "bookmarks-" + dateObj.toLocaleFormat("%Y-%m-%d") + ".json");
+    do_check_eq(name, "bookmarks-" + PlacesBackups.toISODateString(dateObj) + ".json");
     files = bookmarksBackupDir.directoryEntries;
     while (files.hasMoreElements()) {
       let entry = files.getNext().QueryInterface(Ci.nsIFile);

toolkit/components/places/tests/bookmarks/test_818584-discard-duplicate-backups.js

@@ -33,7 +33,7 @@ add_task(function() {
   do_check_eq(backupFiles.length, 1);
   
   let matches = OS.Path.basename(backupFiles[0]).match(PlacesBackups.filenamesRegex);
-  do_check_eq(matches[1], new Date().toLocaleFormat("%Y-%m-%d"));
+  do_check_eq(matches[1], PlacesBackups.toISODateString(new Date()));
   do_check_eq(matches[2], count);
   do_check_eq(matches[3], hash);
 
@@ -49,7 +49,7 @@ add_task(function() {
   recentBackup = yield PlacesBackups.getMostRecentBackup();
   do_check_neq(recentBackup, OS.Path.join(backupFolder, oldBackupName));
   matches = OS.Path.basename(recentBackup).match(PlacesBackups.filenamesRegex);
-  do_check_eq(matches[1], new Date().toLocaleFormat("%Y-%m-%d"));
+  do_check_eq(matches[1], PlacesBackups.toISODateString(new Date()));
   do_check_eq(matches[2], count + 1);
   do_check_neq(matches[3], hash);
 

toolkit/components/places/tests/head_common.js

@@ -382,7 +382,7 @@ function shutdownPlaces(aKeepAliveConnection)
 
 const FILENAME_BOOKMARKS_HTML = "bookmarks.html";
 const FILENAME_BOOKMARKS_JSON = "bookmarks-" +
-  (new Date().toLocaleFormat("%Y-%m-%d")) + ".json";
+  (PlacesBackups.toISODateString(new Date())) + ".json";
 
 /**
  * Creates a bookmarks.html file in the profile folder from a given source file.
@@ -494,7 +494,7 @@ function check_JSON_backup(aIsAutomaticBackup) {
     let bookmarksBackupDir = gProfD.clone();
     bookmarksBackupDir.append("bookmarkbackups");
     let files = bookmarksBackupDir.directoryEntries;
-    let backup_date = new Date().toLocaleFormat("%Y-%m-%d");
+    let backup_date = PlacesBackups.toISODateString(new Date());
     while (files.hasMoreElements()) {
       let entry = files.getNext().QueryInterface(Ci.nsIFile);
       if (PlacesBackups.filenamesRegex.test(entry.leafName)) {

toolkit/components/places/tests/unit/test_utils_backups_create.js

@@ -23,9 +23,8 @@ add_task(function () {
     let randomDate = new Date(dateObj.getFullYear() - 1,
                               Math.floor(12 * Math.random()),
                               Math.floor(28 * Math.random()));
-    let dateString = randomDate.toLocaleFormat("%Y-%m-%d");
-    if (dates.indexOf(dateString) == -1)
-      dates.push(dateString);
+    if (dates.indexOf(randomDate.getTime()) == -1)
+      dates.push(randomDate.getTime());
   }
   // Sort dates from oldest to newest.
   dates.sort();
@@ -49,7 +48,7 @@ add_task(function () {
 
   yield PlacesBackups.create(NUMBER_OF_BACKUPS);
   // Add today's backup.
-  dates.push(dateObj.toLocaleFormat("%Y-%m-%d"));
+  dates.push(dateObj.getTime());
 
   // Check backups.  We have 11 dates but we the max number is 10 so the
   // oldest backup should have been removed.

xpcom/io/nsEscape.cpp

@@ -360,7 +360,7 @@ static const uint32_t EscapeChars[256] =
 {
      0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,  // 0x
      0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,  // 1x
-     0,1023,   0, 512,1023,   0,1023,   0,1023,1023,1023,1023,1023,1023, 953, 784,  // 2x   !"#$%&'()*+,-./
+     0,1023,   0, 512,1023,   0,1023, 112,1023,1023,1023,1023,1023,1023, 953, 784,  // 2x   !"#$%&'()*+,-./
   1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1008,1008,   0,1008,   0, 768,  // 3x  0123456789:;<=>?
   1008,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,  // 4x  @ABCDEFGHIJKLMNO
   1023,1023,1023,1023,1023,1023,1023,1023,1023,1023,1023, 896, 896, 896, 896,1023,  // 5x  PQRSTUVWXYZ[\]^_