From 71bd47b5d0fa5323efb4966579929a45f8c486ca Mon Sep 17 00:00:00 2001
From: Adrian Wielgosik <adrian.wielgosik@gmail.com>
Date: Fri, 8 Feb 2019 23:09:20 +0000
Subject: [PATCH 01/15] Bug 1492629 - de-COM mozJSComponentLoader. r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D18388

--HG--
extra : moz-landing-system : lando
---
 js/xpconnect/loader/mozJSComponentLoader.cpp | 34 ++++----------------
 js/xpconnect/loader/mozJSComponentLoader.h   | 12 +++----
 xpcom/build/XPCOMInit.cpp                    | 34 ++++----------------
 xpcom/build/moz.build                        |  1 +
 xpcom/build/nsXPCOMPrivate.h                 |  6 ----
 5 files changed, 19 insertions(+), 68 deletions(-)

diff --git a/js/xpconnect/loader/mozJSComponentLoader.cpp b/js/xpconnect/loader/mozJSComponentLoader.cpp
index e2307e12b98f..4924a824df0a 100644
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -30,7 +30,6 @@
 #include "mozJSComponentLoader.h"
 #include "mozJSLoaderUtils.h"
 #include "nsIXPConnect.h"
-#include "nsIObserverService.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIFileURL.h"
 #include "nsIJARURI.h"
@@ -66,9 +65,6 @@ using namespace mozilla::loader;
 using namespace xpc;
 using namespace JS;
 
-static const char kObserverServiceContractID[] =
-    "@mozilla.org/observer-service;1";
-
 #define JS_CACHE_PREFIX(aType) "jsloader/" aType
 
 /**
@@ -297,7 +293,7 @@ static nsresult ReportOnCallerUTF8(JSCLContextHelper& helper,
 mozJSComponentLoader::~mozJSComponentLoader() {
   if (mInitialized) {
     NS_ERROR(
-        "'xpcom-shutdown-loaders' was not fired before cleaning up "
+        "UnloadModules() was not explicitly called before cleaning up "
         "mozJSComponentLoader");
     UnloadModules();
   }
@@ -307,8 +303,6 @@ mozJSComponentLoader::~mozJSComponentLoader() {
 
 StaticRefPtr<mozJSComponentLoader> mozJSComponentLoader::sSelf;
 
-NS_IMPL_ISUPPORTS(mozJSComponentLoader, nsIObserver)
-
 nsresult mozJSComponentLoader::ReallyInit() {
   MOZ_ASSERT(!mInitialized);
 
@@ -323,14 +317,6 @@ nsresult mozJSComponentLoader::ReallyInit() {
     mShareLoaderGlobal = Preferences::GetBool("jsloader.shareGlobal");
   }
 
-  nsresult rv;
-  nsCOMPtr<nsIObserverService> obsSvc =
-      do_GetService(kObserverServiceContractID, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = obsSvc->AddObserver(this, "xpcom-shutdown-loaders", false);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   mInitialized = true;
 
   return NS_OK;
@@ -538,6 +524,12 @@ void mozJSComponentLoader::InitStatics() {
   sSelf = new mozJSComponentLoader();
 }
 
+void mozJSComponentLoader::Unload() {
+  if (sSelf) {
+    sSelf->UnloadModules();
+  }
+}
+
 void mozJSComponentLoader::Shutdown() {
   MOZ_ASSERT(sSelf);
   sSelf = nullptr;
@@ -1406,18 +1398,6 @@ nsresult mozJSComponentLoader::Unload(const nsACString& aLocation) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-mozJSComponentLoader::Observe(nsISupports* subject, const char* topic,
-                              const char16_t* data) {
-  if (!strcmp(topic, "xpcom-shutdown-loaders")) {
-    UnloadModules();
-  } else {
-    NS_ERROR("Unexpected observer topic.");
-  }
-
-  return NS_OK;
-}
-
 size_t mozJSComponentLoader::ModuleEntry::SizeOfIncludingThis(
     MallocSizeOf aMallocSizeOf) const {
   size_t n = aMallocSizeOf(this);
diff --git a/js/xpconnect/loader/mozJSComponentLoader.h b/js/xpconnect/loader/mozJSComponentLoader.h
index ea0814a3e36c..f458c9988ee7 100644
--- a/js/xpconnect/loader/mozJSComponentLoader.h
+++ b/js/xpconnect/loader/mozJSComponentLoader.h
@@ -14,7 +14,6 @@
 #include "mozilla/StaticPtr.h"
 #include "nsAutoPtr.h"
 #include "nsISupports.h"
-#include "nsIObserver.h"
 #include "nsIURI.h"
 #include "nsClassHashtable.h"
 #include "nsDataHashtable.h"
@@ -34,12 +33,9 @@ class ScriptPreloader;
 #  define STARTUP_RECORDER_ENABLED
 #endif
 
-class mozJSComponentLoader final : public nsIObserver {
+class mozJSComponentLoader final {
  public:
-  NS_DECL_ISUPPORTS
-  NS_DECL_NSIOBSERVER
-
-  mozJSComponentLoader();
+  NS_INLINE_DECL_REFCOUNTING(mozJSComponentLoader);
 
   void GetLoadedModules(nsTArray<nsCString>& aLoadedModules);
   void GetLoadedComponents(nsTArray<nsCString>& aLoadedComponents);
@@ -53,6 +49,7 @@ class mozJSComponentLoader final : public nsIObserver {
   void FindTargetObject(JSContext* aCx, JS::MutableHandleObject aTargetObject);
 
   static void InitStatics();
+  static void Unload();
   static void Shutdown();
 
   static mozJSComponentLoader* Get() {
@@ -84,7 +81,8 @@ class mozJSComponentLoader final : public nsIObserver {
   nsresult AnnotateCrashReport();
 
  protected:
-  virtual ~mozJSComponentLoader();
+  mozJSComponentLoader();
+  ~mozJSComponentLoader();
 
   friend class XPCJSRuntime;
 
diff --git a/xpcom/build/XPCOMInit.cpp b/xpcom/build/XPCOMInit.cpp
index aa85d050060f..766cdb469ad8 100644
--- a/xpcom/build/XPCOMInit.cpp
+++ b/xpcom/build/XPCOMInit.cpp
@@ -14,6 +14,7 @@
 #include "mozilla/SharedThreadPool.h"
 #include "mozilla/VideoDecoderManagerChild.h"
 #include "mozilla/XPCOM.h"
+#include "mozJSComponentLoader.h"
 #include "nsXULAppAPI.h"
 
 #ifndef ANDROID
@@ -591,7 +592,6 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
   }
 
   nsresult rv;
-  nsCOMPtr<nsISimpleEnumerator> moduleLoaders;
 
   // Notify observers of xpcom shutting down
   {
@@ -662,13 +662,8 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
     // xpcshell tests replacing the service) modules being unloaded.
     mozilla::InitLateWriteChecks();
 
-    // We save the "xpcom-shutdown-loaders" observers to notify after
-    // the observerservice is gone.
     if (observerService) {
       mozilla::KillClearOnShutdown(ShutdownPhase::ShutdownLoaders);
-      observerService->EnumerateObservers(NS_XPCOM_SHUTDOWN_LOADERS_OBSERVER_ID,
-                                          getter_AddRefs(moduleLoaders));
-
       observerService->Shutdown();
     }
   }
@@ -698,28 +693,11 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
   free(gGREBinPath);
   gGREBinPath = nullptr;
 
-  if (moduleLoaders) {
-    bool more;
-    nsCOMPtr<nsISupports> el;
-    while (NS_SUCCEEDED(moduleLoaders->HasMoreElements(&more)) && more) {
-      moduleLoaders->GetNext(getter_AddRefs(el));
-
-      // Don't worry about weak-reference observers here: there is
-      // no reason for weak-ref observers to register for
-      // xpcom-shutdown-loaders
-
-      // FIXME: This can cause harmless writes from sqlite committing
-      // log files. We have to ignore them before we can move
-      // the mozilla::PoisonWrite call before this point. See bug
-      // 834945 for the details.
-      nsCOMPtr<nsIObserver> obs(do_QueryInterface(el));
-      if (obs) {
-        obs->Observe(nullptr, NS_XPCOM_SHUTDOWN_LOADERS_OBSERVER_ID, nullptr);
-      }
-    }
-
-    moduleLoaders = nullptr;
-  }
+  // FIXME: This can cause harmless writes from sqlite committing
+  // log files. We have to ignore them before we can move
+  // the mozilla::PoisonWrite call before this point. See bug
+  // 834945 for the details.
+  mozJSComponentLoader::Unload();
 
   // Clear the profiler's JS context before cycle collection. The profiler will
   // notify the JS engine that it can let go of any data it's holding on to for
diff --git a/xpcom/build/moz.build b/xpcom/build/moz.build
index fcf3dd756c0c..b8489e492976 100755
--- a/xpcom/build/moz.build
+++ b/xpcom/build/moz.build
@@ -100,6 +100,7 @@ LOCAL_INCLUDES += [
     '../threads',
     '/chrome',
     '/docshell/base',
+    '/js/xpconnect/loader',
 ]
 
 if CONFIG['MOZ_VPX']:
diff --git a/xpcom/build/nsXPCOMPrivate.h b/xpcom/build/nsXPCOMPrivate.h
index 7a56a6997514..8de8145655fd 100644
--- a/xpcom/build/nsXPCOMPrivate.h
+++ b/xpcom/build/nsXPCOMPrivate.h
@@ -16,12 +16,6 @@
  */
 #define NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID "xpcom-shutdown-threads"
 
-/**
- * During this shutdown notification all module loaders must unload XPCOM
- * modules.
- */
-#define NS_XPCOM_SHUTDOWN_LOADERS_OBSERVER_ID "xpcom-shutdown-loaders"
-
 // PUBLIC
 namespace mozilla {
 

From 40dce564995d1198ca1026f516f91089b5b2641b Mon Sep 17 00:00:00 2001
From: Robert Strong <robert.bugzilla@gmail.com>
Date: Fri, 8 Feb 2019 22:57:49 +0000
Subject: [PATCH 02/15] Bug 1526147 - Change DEBUG_AUS_TEST so it is declared
 in shared.js. r=mhowell

Renames DEBUG_AUS_TEST which used to be a const to gDebugTest.
Moved the declaration of gDebugTest to shared.js since it is easier to lint these files with it declared in shared.js

Differential Revision: https://phabricator.services.mozilla.com/D19118

--HG--
extra : moz-landing-system : lando
---
 toolkit/mozapps/update/tests/browser/head.js         |  9 ++++++---
 toolkit/mozapps/update/tests/chrome/utils.js         | 12 ++++++------
 toolkit/mozapps/update/tests/data/shared.js          |  7 ++++---
 .../mozapps/update/tests/data/xpcshellUtilsAUS.js    | 10 +++++-----
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/toolkit/mozapps/update/tests/browser/head.js b/toolkit/mozapps/update/tests/browser/head.js
index f90a79e84565..9a86884d9f62 100644
--- a/toolkit/mozapps/update/tests/browser/head.js
+++ b/toolkit/mozapps/update/tests/browser/head.js
@@ -14,8 +14,6 @@ const BIN_SUFFIX = (AppConstants.platform == "win" ? ".exe" : "");
 const FILE_UPDATER_BIN = "updater" + (AppConstants.platform == "macosx" ? ".app" : BIN_SUFFIX);
 const FILE_UPDATER_BIN_BAK = FILE_UPDATER_BIN + ".bak";
 
-var DEBUG_AUS_TEST = true;
-
 const LOG_FUNCTION = info;
 
 const MAX_UPDATE_COPY_ATTEMPTS = 10;
@@ -32,6 +30,11 @@ Services.scriptloader.loadSubScript(DATA_URI_SPEC + "shared.js", this);
 
 let gOriginalUpdateAutoValue = null;
 
+// Set to true to log additional information for debugging. To log additional
+// information for individual tests set gDebugTest to false here and to true in
+// the test's onload function.
+gDebugTest = true;
+
 /**
  * Creates the continue file used to signal that update staging or the mock http
  * server should continue. The delay this creates allows the tests to verify the
@@ -180,7 +183,7 @@ async function setAppUpdateAutoEnabledHelper(enabled) {
 add_task(async function setDefaults() {
   await SpecialPowers.pushPrefEnv({
     set: [
-      [PREF_APP_UPDATE_LOG, DEBUG_AUS_TEST],
+      [PREF_APP_UPDATE_LOG, gDebugTest],
       // See bug 1505790 - uses a very large value to prevent the sync code
       // from running since it has nothing to do with these tests.
       ["services.sync.autoconnectDelay", 600000],
diff --git a/toolkit/mozapps/update/tests/chrome/utils.js b/toolkit/mozapps/update/tests/chrome/utils.js
index 2eba6f7db561..d0c8fdb08c90 100644
--- a/toolkit/mozapps/update/tests/chrome/utils.js
+++ b/toolkit/mozapps/update/tests/chrome/utils.js
@@ -136,14 +136,14 @@ var gDocElem;
 var gPrefToCheck;
 var gUseTestUpdater = false;
 
-// Set to true to log additional information for debugging. To log additional
-// information for an individual test set DEBUG_AUS_TEST to true in the test's
-// onload function.
-var DEBUG_AUS_TEST = true;
-
 /* import-globals-from ../data/shared.js */
 Services.scriptloader.loadSubScript(DATA_URI_SPEC + "shared.js", this);
 
+// Set to true to log additional information for debugging. To log additional
+// information for individual tests set gDebugTest to false here and to true in
+// the test's onload function.
+gDebugTest = true;
+
 /**
  * The current test in TESTS array.
  */
@@ -799,7 +799,7 @@ function restoreUpdaterBackup() {
  * finished.
  */
 function setupPrefs() {
-  if (DEBUG_AUS_TEST) {
+  if (gDebugTest) {
     Services.prefs.setBoolPref(PREF_APP_UPDATE_LOG, true);
   }
 
diff --git a/toolkit/mozapps/update/tests/data/shared.js b/toolkit/mozapps/update/tests/data/shared.js
index f66d2a937d2e..513afea807d0 100644
--- a/toolkit/mozapps/update/tests/data/shared.js
+++ b/toolkit/mozapps/update/tests/data/shared.js
@@ -6,7 +6,7 @@
 
 // Definitions needed to run eslint on this file.
 /* global AppConstants, DATA_URI_SPEC, LOG_FUNCTION */
-/* global Services, URL_HOST, DEBUG_AUS_TEST */
+/* global Services, URL_HOST */
 
 const {FileUtils} = ChromeUtils.import("resource://gre/modules/FileUtils.jsm");
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -90,6 +90,7 @@ const PR_CREATE_FILE = 0x08;
 const PR_TRUNCATE    = 0x20;
 
 var gChannel;
+var gDebugTest = false;
 
 /* import-globals-from sharedUpdateXML.js */
 Services.scriptloader.loadSubScript(DATA_URI_SPEC + "sharedUpdateXML.js", this);
@@ -761,7 +762,7 @@ function logTestInfo(aText, aCaller) {
 }
 
 /**
- * Logs TEST-INFO messages when DEBUG_AUS_TEST evaluates to true.
+ * Logs TEST-INFO messages when gDebugTest evaluates to true.
  *
  * @param  aText
  *         The text to log.
@@ -770,7 +771,7 @@ function logTestInfo(aText, aCaller) {
  *         Components.stack.caller will be used.
  */
 function debugDump(aText, aCaller) {
-  if (DEBUG_AUS_TEST) {
+  if (gDebugTest) {
     let caller = aCaller ? aCaller : Components.stack.caller;
     logTestInfo(aText, caller);
   }
diff --git a/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js b/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
index eb554a26c8ff..0d24dc622071 100644
--- a/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
+++ b/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
@@ -183,9 +183,9 @@ const DATA_URI_SPEC = Services.io.newFileURI(do_get_file("", false)).spec;
 load("shared.js");
 
 // Set to true to log additional information for debugging. To log additional
-// information for an individual test set DEBUG_AUS_TEST to true in the test's
-// run_test function.
-var DEBUG_AUS_TEST = true;
+// information for individual tests set gDebugTest to false here and to true in
+// the test's onload function.
+gDebugTest = true;
 
 // Setting gDebugTestLog to true will create log files for the tests in
 // <objdir>/_tests/xpcshell/toolkit/mozapps/update/tests/<testdir>/ except for
@@ -1003,7 +1003,7 @@ function dumpOverride(aText) {
  * inspected.
  */
 function doTestFinish() {
-  if (DEBUG_AUS_TEST) {
+  if (gDebugTest) {
     // This prevents do_print errors from being printed by the xpcshell test
     // harness due to nsUpdateService.js logging to the console when the
     // app.update.log preference is true.
@@ -1028,7 +1028,7 @@ function doTestFinish() {
  */
 function setDefaultPrefs() {
   Services.prefs.setBoolPref(PREF_APP_UPDATE_DISABLEDFORTESTING, false);
-  if (DEBUG_AUS_TEST) {
+  if (gDebugTest) {
     // Enable Update logging
     Services.prefs.setBoolPref(PREF_APP_UPDATE_LOG, true);
   } else {

From a298106d25787448c300b4e3cf414ea5809d3698 Mon Sep 17 00:00:00 2001
From: Robert Strong <robert.bugzilla@gmail.com>
Date: Fri, 8 Feb 2019 23:03:23 +0000
Subject: [PATCH 03/15] Bug 1526221 - Remove getAppBaseDir function from tests
 since getGreBinDir provides the same functionality and move readFileBytes to
 a common file. r=mhowell

Differential Revision: https://phabricator.services.mozilla.com/D19130

--HG--
extra : moz-landing-system : lando
---
 .../update/tests/browser/app_update.sjs       | 29 ----------
 .../mozapps/update/tests/chrome/update.sjs    | 29 ----------
 toolkit/mozapps/update/tests/data/shared.js   | 43 +--------------
 .../update/tests/data/sharedUpdateXML.js      | 53 +++++++++++++++----
 .../update/tests/data/xpcshellUtilsAUS.js     |  3 --
 .../marAppApplyDirLockedStageFailure_win.js   |  4 +-
 ...pApplyUpdateAppBinInUseStageSuccess_win.js |  2 +-
 ...marAppApplyDirLockedStageFailureSvc_win.js |  4 +-
 ...plyUpdateAppBinInUseStageSuccessSvc_win.js |  2 +-
 9 files changed, 51 insertions(+), 118 deletions(-)

diff --git a/toolkit/mozapps/update/tests/browser/app_update.sjs b/toolkit/mozapps/update/tests/browser/app_update.sjs
index 290d25fa27e9..5db31583b23b 100644
--- a/toolkit/mozapps/update/tests/browser/app_update.sjs
+++ b/toolkit/mozapps/update/tests/browser/app_update.sjs
@@ -210,32 +210,3 @@ function parseQueryString(aQueryString) {
 
   return params;
 }
-
-/**
- * Reads the binary contents of a file and returns it as a string.
- *
- * @param  aFile
- *         The file to read from.
- * @return The contents of the file as a string.
- */
-function readFileBytes(aFile) {
-  let fis = Cc["@mozilla.org/network/file-input-stream;1"].
-            createInstance(Ci.nsIFileInputStream);
-  fis.init(aFile, -1, -1, false);
-  let bis = Cc["@mozilla.org/binaryinputstream;1"].
-            createInstance(Ci.nsIBinaryInputStream);
-  bis.setInputStream(fis);
-  let data = [];
-  let count = fis.available();
-  while (count > 0) {
-    let bytes = bis.readByteArray(Math.min(65535, count));
-    data.push(String.fromCharCode.apply(null, bytes));
-    count -= bytes.length;
-    if (bytes.length == 0) {
-      throw "Nothing read from input stream!";
-    }
-  }
-  data.join('');
-  fis.close();
-  return data.toString();
-}
diff --git a/toolkit/mozapps/update/tests/chrome/update.sjs b/toolkit/mozapps/update/tests/chrome/update.sjs
index 8fdca9e6d004..ac6202e08abb 100644
--- a/toolkit/mozapps/update/tests/chrome/update.sjs
+++ b/toolkit/mozapps/update/tests/chrome/update.sjs
@@ -178,32 +178,3 @@ function parseQueryString(aQueryString) {
 
   return params;
 }
-
-/**
- * Reads the binary contents of a file and returns it as a string.
- *
- * @param  aFile
- *         The file to read from.
- * @return The contents of the file as a string.
- */
-function readFileBytes(aFile) {
-  let fis = Cc["@mozilla.org/network/file-input-stream;1"].
-            createInstance(Ci.nsIFileInputStream);
-  fis.init(aFile, -1, -1, false);
-  let bis = Cc["@mozilla.org/binaryinputstream;1"].
-            createInstance(Ci.nsIBinaryInputStream);
-  bis.setInputStream(fis);
-  let data = [];
-  let count = fis.available();
-  while (count > 0) {
-    let bytes = bis.readByteArray(Math.min(65535, count));
-    data.push(String.fromCharCode.apply(null, bytes));
-    count -= bytes.length;
-    if (bytes.length == 0) {
-      throw "Nothing read from input stream!";
-    }
-  }
-  data.join('');
-  fis.close();
-  return data.toString();
-}
diff --git a/toolkit/mozapps/update/tests/data/shared.js b/toolkit/mozapps/update/tests/data/shared.js
index 513afea807d0..4f0329683a22 100644
--- a/toolkit/mozapps/update/tests/data/shared.js
+++ b/toolkit/mozapps/update/tests/data/shared.js
@@ -329,38 +329,6 @@ function readFile(aFile) {
   return text;
 }
 
-/**
- * Reads the binary contents of a file and returns it as a string.
- *
- * @param  aFile
- *         The file to read from.
- * @return The contents of the file as a string.
- */
-function readFileBytes(aFile) {
-  debugDump("attempting to read file, path: " + aFile.path);
-  let fis = Cc["@mozilla.org/network/file-input-stream;1"].
-            createInstance(Ci.nsIFileInputStream);
-  // Specifying -1 for ioFlags will open the file with the default of PR_RDONLY.
-  // Specifying -1 for perm will open the file with the default of 0.
-  fis.init(aFile, -1, -1, Ci.nsIFileInputStream.CLOSE_ON_EOF);
-  let bis = Cc["@mozilla.org/binaryinputstream;1"].
-            createInstance(Ci.nsIBinaryInputStream);
-  bis.setInputStream(fis);
-  let data = [];
-  let count = fis.available();
-  while (count > 0) {
-    let bytes = bis.readByteArray(Math.min(65535, count));
-    data.push(String.fromCharCode.apply(null, bytes));
-    count -= bytes.length;
-    if (bytes.length == 0) {
-      throw "Nothing read from input stream!";
-    }
-  }
-  data = data.join("");
-  fis.close();
-  return data.toString();
-}
-
 /* Returns human readable status text from the updates.properties bundle */
 function getStatusText(aErrCode) {
   return getString("check_error-" + aErrCode);
@@ -451,7 +419,7 @@ function getStageDirFile(aRelPath) {
   if (AppConstants.platform == "macosx") {
     file = getUpdateDirFile(DIR_PATCH);
   } else {
-    file = getAppBaseDir();
+    file = getGREBinDir();
   }
   file.append(DIR_UPDATED);
   if (aRelPath) {
@@ -580,15 +548,6 @@ function getCurrentProcessDir() {
   return Services.dirsvc.get(NS_XPCOM_CURRENT_PROCESS_DIR, Ci.nsIFile);
 }
 
-/**
- * Gets the application base directory.
- *
- * @return  nsIFile object for the application base directory.
- */
-function getAppBaseDir() {
-  return Services.dirsvc.get(XRE_EXECUTABLE_FILE, Ci.nsIFile).parent;
-}
-
 /**
  * Returns the Gecko Runtime Engine directory where files other than executable
  * binaries are located. On Mac OS X this will be <bundle>/Contents/Resources/
diff --git a/toolkit/mozapps/update/tests/data/sharedUpdateXML.js b/toolkit/mozapps/update/tests/data/sharedUpdateXML.js
index c9d219639ead..8de8cbe7d08c 100644
--- a/toolkit/mozapps/update/tests/data/sharedUpdateXML.js
+++ b/toolkit/mozapps/update/tests/data/sharedUpdateXML.js
@@ -3,15 +3,17 @@
  */
 
 /**
- * Helper functions for creating xml strings used by application update tests.
- *
- * !IMPORTANT - This file contains everything needed (along with dependencies)
- * by the updates.sjs file used by the mochitest-chrome tests. Since xpcshell
- * used by the http server is launched with -v 170 this file must not use
- * features greater than JavaScript 1.7.
+ * Shared code for xpcshell, mochitests-chrome, mochitest-browser-chrome, and
+ * SJS server-side scripts for the test http server.
  */
 
-/* eslint-disable no-undef */
+/**
+ * Helper functions for creating xml strings used by application update tests.
+ */
+
+/* import-globals-from ../browser/testConstants.js */
+
+/* global Services, UpdateUtils, gURLData */
 
 const FILE_SIMPLE_MAR = "simple.mar";
 const SIZE_SIMPLE_MAR = "1404";
@@ -132,9 +134,11 @@ function getRemoteUpdateString(aUpdateProps, aPatches) {
     updateProps[name] = aUpdateProps[name];
   }
 
+  // To test that text nodes are handled properly the string returned contains
+  // spaces and newlines.
   return getUpdateString(updateProps) + ">\n " +
          aPatches +
-         "\n</update>";
+         "\n</update>\n";
 }
 
 /**
@@ -217,7 +221,7 @@ function getLocalUpdateString(aUpdateProps, aPatches) {
       this._appVersion = val;
     },
     buildID: "20080811053724",
-    channel: gDefaultPrefBranch.getCharPref(PREF_APP_UPDATE_CHANNEL),
+    channel: UpdateUtils ? UpdateUtils.getUpdateChannel() : "default",
     custom1: null,
     custom2: null,
     detailsURL: URL_HTTP_UPDATE_SJS + "?uiURL=DETAILS",
@@ -348,3 +352,34 @@ function getPatchString(aPatchProps) {
          custom2 +
          size;
 }
+
+/**
+ * Reads the binary contents of a file and returns it as a string.
+ *
+ * @param  aFile
+ *         The file to read from.
+ * @return The contents of the file as a string.
+ */
+function readFileBytes(aFile) {
+  let fis = Cc["@mozilla.org/network/file-input-stream;1"].
+            createInstance(Ci.nsIFileInputStream);
+  // Specifying -1 for ioFlags will open the file with the default of PR_RDONLY.
+  // Specifying -1 for perm will open the file with the default of 0.
+  fis.init(aFile, -1, -1, Ci.nsIFileInputStream.CLOSE_ON_EOF);
+  let bis = Cc["@mozilla.org/binaryinputstream;1"].
+            createInstance(Ci.nsIBinaryInputStream);
+  bis.setInputStream(fis);
+  let data = [];
+  let count = fis.available();
+  while (count > 0) {
+    let bytes = bis.readByteArray(Math.min(65535, count));
+    data.push(String.fromCharCode.apply(null, bytes));
+    count -= bytes.length;
+    if (bytes.length == 0) {
+      throw new Error("Nothing read from input stream!");
+    }
+  }
+  data = data.join("");
+  fis.close();
+  return data.toString();
+}
diff --git a/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js b/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
index 0d24dc622071..c75b35e65b44 100644
--- a/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
+++ b/toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js
@@ -150,7 +150,6 @@ var gHandle;
 
 var gGREDirOrig;
 var gGREBinDirOrig;
-var gAppDirOrig;
 
 var gPIDPersistProcess;
 
@@ -805,7 +804,6 @@ function setupTestCommon(aAppUpdateAutoEnabled = false) {
 
   gGREDirOrig = getGREDir();
   gGREBinDirOrig = getGREBinDir();
-  gAppDirOrig = getAppBaseDir();
 
   let applyDir = getApplyDirFile().parent;
 
@@ -3836,7 +3834,6 @@ function adjustGeneralPaths() {
   let ds = Services.dirsvc.QueryInterface(Ci.nsIDirectoryService);
   ds.QueryInterface(Ci.nsIProperties).undefine(NS_GRE_DIR);
   ds.QueryInterface(Ci.nsIProperties).undefine(NS_GRE_BIN_DIR);
-  ds.QueryInterface(Ci.nsIProperties).undefine(XRE_EXECUTABLE_FILE);
   ds.registerProvider(dirProvider);
   registerCleanupFunction(function AGP_cleanup() {
     debugDump("start - unregistering directory provider");
diff --git a/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyDirLockedStageFailure_win.js b/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyDirLockedStageFailure_win.js
index 8103d2d1519c..87534720c390 100644
--- a/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyDirLockedStageFailure_win.js
+++ b/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyDirLockedStageFailure_win.js
@@ -17,7 +17,7 @@ function run_test() {
   gTestFiles = gTestFilesCompleteSuccess;
   gTestDirs = gTestDirsCompleteSuccess;
   setTestFilesAndDirsForFailure();
-  createUpdateInProgressLockFile(getAppBaseDir());
+  createUpdateInProgressLockFile(getGREBinDir());
   setupUpdaterTest(FILE_COMPLETE_MAR, false);
 }
 
@@ -32,7 +32,7 @@ function setupUpdaterTestFinished() {
  * Called after the call to stageUpdate finishes.
  */
 function stageUpdateFinished() {
-  removeUpdateInProgressLockFile(getAppBaseDir());
+  removeUpdateInProgressLockFile(getGREBinDir());
   checkPostUpdateRunningFile(false);
   checkFilesAfterUpdateFailure(getApplyDirFile);
   checkUpdateLogContains(PERFORMING_STAGED_UPDATE);
diff --git a/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js b/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js
index fd1a2b2bd5fb..be564dda7ec1 100644
--- a/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js
+++ b/toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js
@@ -33,7 +33,7 @@ function stageUpdateFinished() {
   checkPostUpdateRunningFile(false);
   checkFilesAfterUpdateSuccess(getStageDirFile, true, false);
   checkUpdateLogContents(LOG_COMPLETE_SUCCESS, true);
-  lockDirectory(getAppBaseDir().path);
+  lockDirectory(getGREBinDir().path);
   // Switch the application to the staged application that was updated.
   runUpdateUsingApp(STATE_SUCCEEDED);
 }
diff --git a/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyDirLockedStageFailureSvc_win.js b/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyDirLockedStageFailureSvc_win.js
index 8103d2d1519c..87534720c390 100644
--- a/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyDirLockedStageFailureSvc_win.js
+++ b/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyDirLockedStageFailureSvc_win.js
@@ -17,7 +17,7 @@ function run_test() {
   gTestFiles = gTestFilesCompleteSuccess;
   gTestDirs = gTestDirsCompleteSuccess;
   setTestFilesAndDirsForFailure();
-  createUpdateInProgressLockFile(getAppBaseDir());
+  createUpdateInProgressLockFile(getGREBinDir());
   setupUpdaterTest(FILE_COMPLETE_MAR, false);
 }
 
@@ -32,7 +32,7 @@ function setupUpdaterTestFinished() {
  * Called after the call to stageUpdate finishes.
  */
 function stageUpdateFinished() {
-  removeUpdateInProgressLockFile(getAppBaseDir());
+  removeUpdateInProgressLockFile(getGREBinDir());
   checkPostUpdateRunningFile(false);
   checkFilesAfterUpdateFailure(getApplyDirFile);
   checkUpdateLogContains(PERFORMING_STAGED_UPDATE);
diff --git a/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyUpdateAppBinInUseStageSuccessSvc_win.js b/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyUpdateAppBinInUseStageSuccessSvc_win.js
index fd1a2b2bd5fb..be564dda7ec1 100644
--- a/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyUpdateAppBinInUseStageSuccessSvc_win.js
+++ b/toolkit/mozapps/update/tests/unit_service_updater/marAppApplyUpdateAppBinInUseStageSuccessSvc_win.js
@@ -33,7 +33,7 @@ function stageUpdateFinished() {
   checkPostUpdateRunningFile(false);
   checkFilesAfterUpdateSuccess(getStageDirFile, true, false);
   checkUpdateLogContents(LOG_COMPLETE_SUCCESS, true);
-  lockDirectory(getAppBaseDir().path);
+  lockDirectory(getGREBinDir().path);
   // Switch the application to the staged application that was updated.
   runUpdateUsingApp(STATE_SUCCEEDED);
 }

From efbea58a37b85fee921c720cfcaa3f23870d317c Mon Sep 17 00:00:00 2001
From: Jeff Gilbert <jgilbert@mozilla.com>
Date: Sat, 9 Feb 2019 07:41:49 +0000
Subject: [PATCH 04/15] Bug 1524804 - Some webglsl1/essl1 exts can't be
 implemented on es3 contexts. r=lsalzman

MozReview-Commit-ID: DoyN2kNlL01

Differential Revision: https://phabricator.services.mozilla.com/D19234

--HG--
extra : moz-landing-system : lando
---
 dom/canvas/WebGLExtensionDrawBuffers.cpp | 4 ++++
 dom/canvas/WebGLExtensionFragDepth.cpp   | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/dom/canvas/WebGLExtensionDrawBuffers.cpp b/dom/canvas/WebGLExtensionDrawBuffers.cpp
index 2c8fcdc9da50..27b37a42fb6b 100644
--- a/dom/canvas/WebGLExtensionDrawBuffers.cpp
+++ b/dom/canvas/WebGLExtensionDrawBuffers.cpp
@@ -38,6 +38,10 @@ bool WebGLExtensionDrawBuffers::IsSupported(const WebGLContext* webgl) {
   if (webgl->IsWebGL2()) return false;
 
   gl::GLContext* gl = webgl->GL();
+  if (gl->IsGLES() && gl->Version() >= 300) {
+    // ANGLE's shader translator can't translate ESSL1 exts to ESSL3. (bug 1524804)
+    return false;
+  }
   return gl->IsSupported(gl::GLFeature::draw_buffers);
 }
 
diff --git a/dom/canvas/WebGLExtensionFragDepth.cpp b/dom/canvas/WebGLExtensionFragDepth.cpp
index 63eeab7ef6c4..a1bf661ea9e1 100644
--- a/dom/canvas/WebGLExtensionFragDepth.cpp
+++ b/dom/canvas/WebGLExtensionFragDepth.cpp
@@ -22,6 +22,10 @@ bool WebGLExtensionFragDepth::IsSupported(const WebGLContext* webgl) {
   if (webgl->IsWebGL2()) return false;
 
   gl::GLContext* gl = webgl->GL();
+  if (gl->IsGLES() && gl->Version() >= 300) {
+    // ANGLE's shader translator can't translate ESSL1 exts to ESSL3. (bug 1524804)
+    return false;
+  }
   return gl->IsSupported(gl::GLFeature::frag_depth);
 }
 

From b82d5c7497a6817b3a49bd839dfffebd1381a8aa Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 14:56:50 +0000
Subject: [PATCH 05/15] Bug 1526210 - Remove -Wl,--allow-shlib-undefined from
 LDFLAGS on Android. r=froydnj

This apparently was necessary back when Android support was added 8.5
years ago, but is not desirable as a flag set globally. But it turns out
everything builds just fine without it now.

Differential Revision: https://phabricator.services.mozilla.com/D19128

--HG--
extra : moz-landing-system : lando
---
 build/autoconf/android.m4 | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/build/autoconf/android.m4 b/build/autoconf/android.m4
index 3466bb3c82ab..6bba60e44870 100644
--- a/build/autoconf/android.m4
+++ b/build/autoconf/android.m4
@@ -22,10 +22,7 @@ case "$target" in
     CXXFLAGS="-fno-short-enums -fno-exceptions $CXXFLAGS $stlport_cppflags"
     ASFLAGS="$directory_include_args -DANDROID $ASFLAGS"
 
-    dnl Add --allow-shlib-undefined, because libGLESv2 links to an
-    dnl undefined symbol (present on the hardware, just not in the
-    dnl NDK.)
-    LDFLAGS="-L$android_platform/usr/lib -Wl,-rpath-link=$android_platform/usr/lib --sysroot=$android_platform -Wl,--allow-shlib-undefined $LDFLAGS"
+    LDFLAGS="-L$android_platform/usr/lib -Wl,-rpath-link=$android_platform/usr/lib --sysroot=$android_platform $LDFLAGS"
     ANDROID_PLATFORM="${android_platform}"
 
     AC_DEFINE(ANDROID)

From ef876da54d68525479d1e4ceddca96a117aededa Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 16:56:13 +0000
Subject: [PATCH 06/15] Bug 1526062 - Fix miscellaneous NameErrors lingering in
 the python configure code. r=nalexander

Differential Revision: https://phabricator.services.mozilla.com/D19109

--HG--
extra : moz-landing-system : lando
---
 build/moz.configure/android-ndk.configure | 9 ++++-----
 build/moz.configure/android-sdk.configure | 3 ++-
 build/moz.configure/rust.configure        | 2 +-
 build/moz.configure/util.configure        | 2 +-
 build/moz.configure/windows.configure     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/build/moz.configure/android-ndk.configure b/build/moz.configure/android-ndk.configure
index 66cf3e16ad07..9daf59f2e09c 100644
--- a/build/moz.configure/android-ndk.configure
+++ b/build/moz.configure/android-ndk.configure
@@ -222,7 +222,7 @@ def android_toolchain(target, host, ndk, toolchain):
                                         host.kernel.lower(), host.cpu)
         log.debug('Trying %s' % quote(toolchain))
         if not isdir(toolchain) and host.cpu == 'x86_64':
-            toolchain = toolchain_format % (ndk, target_base, version,
+            toolchain = toolchain_format % (ndk, target_base,
                                             host.kernel.lower(), 'x86')
             log.debug('Trying %s' % quote(toolchain))
         if isdir(toolchain):
@@ -339,11 +339,10 @@ def android_clang_compiler(host, ndk):
     if not ndk:
         return
 
-    llvm_path = '%s/toolchains/llvm/prebuilt/%s-%s/bin' % (ndk,
-                                                           host.kernel.lower(),
-                                                           host.cpu)
+    llvm_format = '%s/toolchains/llvm/prebuilt/%s-%s/bin'
+    llvm_path = llvm_format % (ndk, host.kernel.lower(), host.cpu)
     if not isdir(llvm_path) and host.cpu == 'x86_64':
-        llvm_path = toolchain_format % (ndk, host.kernel.lower(), 'x86')
+        llvm_path = llvm_format % (ndk, host.kernel.lower(), 'x86')
 
     if not isdir(llvm_path):
         die("Couldn't find path to LLVM toolchain at %s" % llvm_path)
diff --git a/build/moz.configure/android-sdk.configure b/build/moz.configure/android-sdk.configure
index bcb762caaf92..a8ecc617850d 100644
--- a/build/moz.configure/android-sdk.configure
+++ b/build/moz.configure/android-sdk.configure
@@ -49,7 +49,8 @@ def android_sdk_version(_):
 @imports(_from='os.path', _import='isdir')
 def android_build_tools(sdk_root, sdk_version):
     android_build_tools_base = os.path.join(sdk_root, 'build-tools')
-    for version in sdk_version.build_tools_versions:
+    versions = sdk_version.build_tools_versions
+    for version in versions:
         if isdir(os.path.join(android_build_tools_base, version)):
             tools = os.path.join(android_build_tools_base, version)
             for zipalign in ('zipalign', 'zipalign.exe'):
diff --git a/build/moz.configure/rust.configure b/build/moz.configure/rust.configure
index 06590ecc185a..5ee7fd59084b 100644
--- a/build/moz.configure/rust.configure
+++ b/build/moz.configure/rust.configure
@@ -241,7 +241,7 @@ rust_host_triple = rust_triple_alias(host)
 def validate_rust_host_triple(host, rust_host, rustc_host):
     if rust_host != rustc_host:
         if host.alias == rust_host:
-            configure_host = host_alias
+            configure_host = host.alias
         else:
             configure_host = '{}/{}'.format(host.alias, rust_host)
         die("The rust compiler host ({}) is not suitable for the configure host ({})."
diff --git a/build/moz.configure/util.configure b/build/moz.configure/util.configure
index 70c06089c78e..322224a0eb14 100644
--- a/build/moz.configure/util.configure
+++ b/build/moz.configure/util.configure
@@ -224,7 +224,7 @@ def try_invoke_compiler(compiler, language, source, flags=None, onerror=None):
 
     if not isinstance(flags, (list, tuple)):
         die("Flags provided to try_compile must be a list of strings, "
-            "not %r", paths)
+            "not %r", flags)
 
     suffix = {
         'C': '.c',
diff --git a/build/moz.configure/windows.configure b/build/moz.configure/windows.configure
index 5fb4e8e481b8..857c84a913ab 100644
--- a/build/moz.configure/windows.configure
+++ b/build/moz.configure/windows.configure
@@ -266,7 +266,7 @@ def vc_path(c_compiler, toolchain_search_path):
         next, p = os.path.split(result)
         if next == result:
             die('Cannot determine the Visual C++ directory the compiler (%s) '
-                'is in' % cl)
+                'is in' % vc_program)
         result = next
         if p.lower() == 'bin':
             break

From 015ffd1905dc704c835a9ddc4344388ac7c0da32 Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 16:56:40 +0000
Subject: [PATCH 07/15] Bug 1526062 - Add missing imports for Exception.
 r=nalexander

Depends on D19109

Differential Revision: https://phabricator.services.mozilla.com/D19110

--HG--
extra : moz-landing-system : lando
---
 build/moz.configure/toolchain.configure | 1 +
 build/moz.configure/windows.configure   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure
index 3fa2bfd4c5a2..0e3126d3fe87 100755
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -442,6 +442,7 @@ def try_preprocess(compiler, language, source):
 @imports(_from='mozbuild.configure.constants',
          _import='OS_preprocessor_checks')
 @imports(_from='textwrap', _import='dedent')
+@imports(_from='__builtin__', _import='Exception')
 def get_compiler_info(compiler, language):
     '''Returns information about the given `compiler` (command line in the
     form of a list or tuple), in the given `language`.
diff --git a/build/moz.configure/windows.configure b/build/moz.configure/windows.configure
index 857c84a913ab..2f268ca0b871 100644
--- a/build/moz.configure/windows.configure
+++ b/build/moz.configure/windows.configure
@@ -97,6 +97,7 @@ def valid_windows_sdk_dir_result(value):
 @depends(c_compiler, windows_sdk_dir, valid_windows_version, 'WINDOWSSDKDIR')
 @checking('for Windows SDK', valid_windows_sdk_dir_result)
 @imports(_from='__builtin__', _import='sorted')
+@imports(_from='__builtin__', _import='Exception')
 @imports(_from='textwrap', _import='dedent')
 def valid_windows_sdk_dir(compiler, windows_sdk_dir, target_version,
                           windows_sdk_dir_env):

From 845718811da76f28b646fa68bf11f29334a65cf2 Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 16:57:55 +0000
Subject: [PATCH 08/15] Bug 1526062 - Add AssertionError to the python
 configure sandbox builtins. r=nalexander

Using the `assert` keyword actually generates bytecode that uses the
`AssertionError` class under the hood. Ideally, we should probably not
use asserts in python configure, but there are currently so many that
it's easier to allow it, until a better strategy is decided upon.

Depends on D19110

Differential Revision: https://phabricator.services.mozilla.com/D19111

--HG--
extra : moz-landing-system : lando
---
 python/mozbuild/mozbuild/configure/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/mozbuild/mozbuild/configure/__init__.py b/python/mozbuild/mozbuild/configure/__init__.py
index a56d870153dd..b53bb07af8b4 100644
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -282,7 +282,7 @@ class ConfigureSandbox(dict):
         b: getattr(__builtin__, b)
         for b in ('None', 'False', 'True', 'int', 'bool', 'any', 'all', 'len',
                   'list', 'tuple', 'set', 'dict', 'isinstance', 'getattr',
-                  'hasattr', 'enumerate', 'range', 'zip')
+                  'hasattr', 'enumerate', 'range', 'zip', 'AssertionError')
     }, __import__=forbidden_import, str=unicode)
 
     # Expose a limited set of functions from os.path

From 83b2adb600bd1cd647b1fc38e97fd2e2bf29cb3e Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 22:47:02 +0000
Subject: [PATCH 09/15] Bug 1526062 - Improve error reporting by the configure
 lint. r=nalexander

Currently, running the configure lint will raise errors with little
context, if any, which can make finding the cause of the errors tedious.

So instead of raising exceptions directly, we use a hack to make the
exception appear as if they had been thrown at the code location an
issue is reported for, which makes the test harness print more useful
context.

With more context, printed out, some exception messages can be made
lighter.

The added _raise_from method does more than is necessary here, but will
be fully used in a new check shortly.

Depends on D19111

Differential Revision: https://phabricator.services.mozilla.com/D19112

--HG--
extra : moz-landing-system : lando
---
 python/mozbuild/mozbuild/configure/lint.py    | 118 ++++++++++++++----
 .../mozbuild/test/configure/test_lint.py      |  86 +++++++------
 2 files changed, 141 insertions(+), 63 deletions(-)

diff --git a/python/mozbuild/mozbuild/configure/lint.py b/python/mozbuild/mozbuild/configure/lint.py
index b9728df54a33..b02ffcba6f86 100644
--- a/python/mozbuild/mozbuild/configure/lint.py
+++ b/python/mozbuild/mozbuild/configure/lint.py
@@ -45,19 +45,82 @@ class LintSandbox(ConfigureSandbox):
         for dep in self._depends.itervalues():
             self._check_dependencies(dep)
 
+    def _raise_from(self, exception, obj, line=0):
+        '''
+        Raises the given exception as if it were emitted from the given
+        location.
+
+        The location is determined from the values of obj and line.
+        - `obj` can be a function or DependsFunction, in which case
+          `line` corresponds to the line within the function the exception
+          will be raised from (as an offset from the function's firstlineno).
+        - `obj` can be a stack frame, in which case `line` is ignored.
+        '''
+        def thrower(e):
+            raise e
+
+        if isinstance(obj, DependsFunction):
+            obj, _ = self.unwrap(obj._func)
+
+        if inspect.isfunction(obj):
+            funcname = obj.__name__
+            filename = obj.func_code.co_filename
+            firstline = obj.func_code.co_firstlineno
+            line += firstline
+        elif inspect.isframe(obj):
+            funcname = obj.f_code.co_name
+            filename = obj.f_code.co_filename
+            firstline = obj.f_code.co_firstlineno
+            line = obj.f_lineno
+        else:
+            # Don't know how to handle the given location, still raise the
+            # exception.
+            raise exception
+
+        # Create a new function from the above thrower that pretends
+        # the `def` line is on the first line of the function given as
+        # argument, and the `raise` line is on the line given as argument.
+
+        offset = line - firstline
+        # co_lnotab is a string where each pair of consecutive character is
+        # (chr(byte_increment), chr(line_increment)), mapping bytes in co_code
+        # to line numbers relative to co_firstlineno.
+        # If the offset we need to encode is larger than 255, we need to split it.
+        co_lnotab = (chr(0) + chr(255)) * (offset / 255) + chr(0) + chr(offset % 255)
+        code = thrower.func_code
+        code = types.CodeType(
+            code.co_argcount,
+            code.co_nlocals,
+            code.co_stacksize,
+            code.co_flags,
+            code.co_code,
+            code.co_consts,
+            code.co_names,
+            code.co_varnames,
+            filename,
+            funcname,
+            firstline,
+            co_lnotab
+        )
+        thrower = types.FunctionType(
+            code,
+            thrower.func_globals,
+            funcname,
+            thrower.func_defaults,
+            thrower.func_closure
+        )
+        thrower(exception)
+
     def _check_dependencies(self, obj):
         if isinstance(obj, CombinedDependsFunction) or obj in (self._always,
                                                                self._never):
             return
         func, glob = self.unwrap(obj._func)
-        loc = '%s:%d' % (func.func_code.co_filename,
-                         func.func_code.co_firstlineno)
         func_args = inspect.getargspec(func)
         if func_args.keywords:
-            raise ConfigureError(
-                '%s: Keyword arguments are not allowed in @depends functions'
-                % loc
-            )
+            e = ConfigureError(
+                    'Keyword arguments are not allowed in @depends functions')
+            self._raise_from(e, func)
 
         all_args = list(func_args.args)
         if func_args.varargs:
@@ -77,10 +140,8 @@ class LintSandbox(ConfigureSandbox):
                         dep = dep.name
                     else:
                         dep = dep.option
-                    raise ConfigureError(
-                        '%s: The dependency on `%s` is unused.'
-                        % (loc, dep)
-                    )
+                    e = ConfigureError('The dependency on `%s` is unused' % dep)
+                    self._raise_from(e, func)
 
     def _need_help_dependency(self, obj):
         if isinstance(obj, (CombinedDependsFunction, TrivialDependsFunction)):
@@ -119,13 +180,13 @@ class LintSandbox(ConfigureSandbox):
         if with_help:
             for arg in obj.dependencies:
                 if self._missing_help_dependency(arg):
-                    raise ConfigureError(
-                        "`%s` depends on '--help' and `%s`. "
-                        "`%s` must depend on '--help'"
-                        % (obj.name, arg.name, arg.name))
+                    e = ConfigureError(
+                        "Missing '--help' dependency because `%s` depends on "
+                        "'--help' and `%s`" % (obj.name, arg.name))
+                    self._raise_from(e, arg)
         elif self._missing_help_dependency(obj):
-            raise ConfigureError("Missing @depends for `%s`: '--help'" %
-                                 obj.name)
+            e = ConfigureError("Missing '--help' dependency")
+            self._raise_from(e, obj)
         return super(LintSandbox, self)._value_for_depends(obj)
 
     def option_impl(self, *args, **kwargs):
@@ -166,14 +227,18 @@ class LintSandbox(ConfigureSandbox):
         }
         for prefix, replacement in table[default].iteritems():
             if name.startswith('--{}-'.format(prefix)):
-                raise ConfigureError(('{} should be used instead of '
-                                      '{} with default={}').format(
-                                          name.replace('--{}-'.format(prefix),
-                                                       '--{}-'.format(replacement)),
-                                          name, default))
+                frame = inspect.currentframe()
+                while frame and frame.f_code.co_name != self.option_impl.__name__:
+                    frame = frame.f_back
+                e = ConfigureError('{} should be used instead of '
+                                   '{} with default={}'.format(
+                                       name.replace('--{}-'.format(prefix),
+                                                    '--{}-'.format(replacement)),
+                                       name, default))
+                self._raise_from(e, frame.f_back if frame else None)
+
 
     def _check_help_for_option_with_func_default(self, option, *args, **kwargs):
-        name = args[0]
         default = kwargs['default']
 
         if not isinstance(default, SandboxDependsFunction):
@@ -196,8 +261,13 @@ class LintSandbox(ConfigureSandbox):
         else:
             rule = '{With|Without}'
 
-        raise ConfigureError(('{} has a non-constant default. '
-                              'Its help should contain "{}"').format(name, rule))
+        frame = inspect.currentframe()
+        while frame and frame.f_code.co_name != self.option_impl.__name__:
+            frame = frame.f_back
+        e = ConfigureError(
+            '`help` should contain "{}" because of non-constant default'
+            .format(rule))
+        self._raise_from(e, frame.f_back if frame else None)
 
     def unwrap(self, func):
         glob = func.func_globals
diff --git a/python/mozbuild/mozbuild/test/configure/test_lint.py b/python/mozbuild/mozbuild/test/configure/test_lint.py
index e0690740fc93..3b2c76fd17d7 100644
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -5,8 +5,11 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 from StringIO import StringIO
+import contextlib
 import os
+import sys
 import textwrap
+import traceback
 import unittest
 
 from mozunit import (
@@ -35,6 +38,16 @@ class TestLint(unittest.TestCase):
                          'moz.configure'): textwrap.dedent(source)
         })
 
+    @contextlib.contextmanager
+    def assertRaisesFromLine(self, exc_type, line):
+        with self.assertRaises(exc_type) as e:
+            yield e
+
+        _, _, tb = sys.exc_info()
+        self.assertEquals(
+            traceback.extract_tb(tb)[-1][:2],
+            (mozpath.join(test_data_path, 'moz.configure'), line))
+
     def test_configure_testcase(self):
         # Lint python/mozbuild/mozbuild/test/configure/data/moz.configure
         self.lint_test()
@@ -53,7 +66,7 @@ class TestLint(unittest.TestCase):
         '''):
             self.lint_test()
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 7) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -67,10 +80,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "%s:7: The dependency on `--help` is unused."
-                          % mozpath.join(test_data_path, 'moz.configure'))
+                          "The dependency on `--help` is unused")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 3) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -85,11 +97,11 @@ class TestLint(unittest.TestCase):
             '''):
                 self.lint_test()
 
-        self.assertEquals(e.exception.message,
-                          "`bar` depends on '--help' and `foo`. "
-                          "`foo` must depend on '--help'")
+        self.assertEquals(
+            e.exception.message,
+            "Missing '--help' dependency because `bar` depends on '--help' and `foo`")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 7) as e:
             with self.moz_configure('''
                 @template
                 def tmpl():
@@ -109,9 +121,9 @@ class TestLint(unittest.TestCase):
             '''):
                 self.lint_test()
 
-        self.assertEquals(e.exception.message,
-                          "`bar` depends on '--help' and `foo`. "
-                          "`foo` must depend on '--help'")
+        self.assertEquals(
+            e.exception.message,
+            "Missing '--help' dependency because `bar` depends on '--help' and `foo`")
 
         with self.moz_configure('''
             option('--foo', help='foo')
@@ -123,7 +135,7 @@ class TestLint(unittest.TestCase):
         '''):
             self.lint_test()
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 3) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -136,9 +148,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "Missing @depends for `foo`: '--help'")
+                          "Missing '--help' dependency")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 3) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -155,9 +167,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "Missing @depends for `foo`: '--help'")
+                          "Missing '--help' dependency")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 3) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -170,9 +182,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "Missing @depends for `foo`: '--help'")
+                          "Missing '--help' dependency")
 
-        # This would have failed with "Missing @depends for `foo`: '--help'"
+        # This would have failed with "Missing '--help' dependency"
         # in the past, because of the reference to the builtin False.
         with self.moz_configure('''
             option('--foo', help='foo')
@@ -186,7 +198,7 @@ class TestLint(unittest.TestCase):
 
         # However, when something that is normally a builtin is overridden,
         # we should still want the dependency on --help.
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 7) as e:
             with self.moz_configure('''
                 @template
                 def tmpl():
@@ -203,7 +215,7 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "Missing @depends for `foo`: '--help'")
+                          "Missing '--help' dependency")
 
         # There is a default restricted `os` module when there is no explicit
         # @imports, and it's fine to use it without a dependency on --help.
@@ -218,7 +230,7 @@ class TestLint(unittest.TestCase):
         '''):
             self.lint_test()
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 3) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
@@ -230,10 +242,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "%s:3: The dependency on `--foo` is unused."
-                          % mozpath.join(test_data_path, 'moz.configure'))
+                          "The dependency on `--foo` is unused")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 5) as e:
             with self.moz_configure('''
                 @depends(when=True)
                 def bar():
@@ -247,10 +258,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "%s:5: The dependency on `bar` is unused."
-                          % mozpath.join(test_data_path, 'moz.configure'))
+                          "The dependency on `bar` is unused")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 2) as e:
             with self.moz_configure('''
                 @depends(depends(when=True)(lambda: None))
                 def foo(value):
@@ -261,10 +271,9 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "%s:2: The dependency on `<lambda>` is unused."
-                          % mozpath.join(test_data_path, 'moz.configure'))
+                          "The dependency on `<lambda>` is unused")
 
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 9) as e:
             with self.moz_configure('''
                 @template
                 def tmpl():
@@ -282,8 +291,7 @@ class TestLint(unittest.TestCase):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
-                          "%s:9: The dependency on `qux` is unused."
-                          % mozpath.join(test_data_path, 'moz.configure'))
+                          "The dependency on `qux` is unused")
 
     def test_default_enable(self):
         # --enable-* with default=True is not allowed.
@@ -291,7 +299,7 @@ class TestLint(unittest.TestCase):
             option('--enable-foo', default=False, help='foo')
         '''):
             self.lint_test()
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 2) as e:
             with self.moz_configure('''
                 option('--enable-foo', default=True, help='foo')
             '''):
@@ -306,7 +314,7 @@ class TestLint(unittest.TestCase):
             option('--disable-foo', default=True, help='foo')
         '''):
             self.lint_test()
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 2) as e:
             with self.moz_configure('''
                 option('--disable-foo', default=False, help='foo')
             '''):
@@ -321,7 +329,7 @@ class TestLint(unittest.TestCase):
             option('--with-foo', default=False, help='foo')
         '''):
             self.lint_test()
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 2) as e:
             with self.moz_configure('''
                 option('--with-foo', default=True, help='foo')
             '''):
@@ -336,7 +344,7 @@ class TestLint(unittest.TestCase):
             option('--without-foo', default=True, help='foo')
         '''):
             self.lint_test()
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 2) as e:
             with self.moz_configure('''
                 option('--without-foo', default=False, help='foo')
             '''):
@@ -354,7 +362,7 @@ class TestLint(unittest.TestCase):
                    help='{Enable|Disable} bar')
         '''):
             self.lint_test()
-        with self.assertRaises(ConfigureError) as e:
+        with self.assertRaisesFromLine(ConfigureError, 4) as e:
             with self.moz_configure('''
                 option(env='FOO', help='foo')
                 option('--enable-bar', default=depends('FOO')(lambda x: bool(x)),
@@ -362,8 +370,8 @@ class TestLint(unittest.TestCase):
             '''):
                 self.lint_test()
         self.assertEquals(e.exception.message,
-                          '--enable-bar has a non-constant default. '
-                          'Its help should contain "{Enable|Disable}"')
+                          '`help` should contain "{Enable|Disable}" because of '
+                          'non-constant default')
 
 if __name__ == '__main__':
     main()

From e3b77d6ed3535435b53c4019e9c5b2cdf2093248 Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 20:38:05 +0000
Subject: [PATCH 10/15] Bug 1526062 - Add a configure lint for undefined
 variables. r=nalexander

There are cases that can be easily detected where an undefined variable
is used, and such mistakes seem to happen more often than they should,
as all the errors fixed in the previous patches (that this new lint
caught).

Depends on D19112

Differential Revision: https://phabricator.services.mozilla.com/D19113

--HG--
extra : moz-landing-system : lando
---
 .../mozbuild/mozbuild/configure/__init__.py   | 29 ++++++++------
 python/mozbuild/mozbuild/configure/lint.py    | 28 ++++++++++++-
 .../mozbuild/mozbuild/configure/lint_util.py  | 40 ++++++++++++++-----
 .../mozbuild/test/configure/test_lint.py      | 16 ++++++++
 4 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/python/mozbuild/mozbuild/configure/__init__.py b/python/mozbuild/mozbuild/configure/__init__.py
index b53bb07af8b4..44ae9aa2ce8d 100644
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -478,7 +478,7 @@ class ConfigureSandbox(dict):
             raise KeyError('Cannot reassign builtins')
 
         if inspect.isfunction(value) and value not in self._templates:
-            value, _ = self._prepare_function(value)
+            value = self._prepare_function(value)
 
         elif (not isinstance(value, SandboxDependsFunction) and
                 value not in self._templates and
@@ -705,7 +705,7 @@ class ConfigureSandbox(dict):
             if inspect.isgeneratorfunction(func):
                 raise ConfigureError(
                     'Cannot decorate generator functions with @depends')
-            func, glob = self._prepare_function(func)
+            func = self._prepare_function(func)
             depends = DependsFunction(self, func, dependencies, when=when)
             return depends.sandboxed
 
@@ -734,12 +734,14 @@ class ConfigureSandbox(dict):
         Templates allow to simplify repetitive constructs, or to implement
         helper decorators and somesuch.
         '''
-        template, glob = self._prepare_function(func)
-        glob.update(
-            (k[:-len('_impl')], getattr(self, k))
-            for k in dir(self) if k.endswith('_impl') and k != 'template_impl'
-        )
-        glob.update((k, v) for k, v in self.iteritems() if k not in glob)
+        def update_globals(glob):
+            glob.update(
+                (k[:-len('_impl')], getattr(self, k))
+                for k in dir(self) if k.endswith('_impl') and k != 'template_impl'
+            )
+            glob.update((k, v) for k, v in self.iteritems() if k not in glob)
+
+        template = self._prepare_function(func, update_globals)
 
         # Any function argument to the template must be prepared to be sandboxed.
         # If the template itself returns a function (in which case, it's very
@@ -750,8 +752,7 @@ class ConfigureSandbox(dict):
 
             def maybe_prepare_function(obj):
                 if isfunction(obj):
-                    func, _ = self._prepare_function(obj)
-                    return func
+                    return self._prepare_function(obj)
                 return obj
 
             # The following function may end up being prepared to be sandboxed,
@@ -1016,14 +1017,14 @@ class ConfigureSandbox(dict):
             when=when,
         ))
 
-    def _prepare_function(self, func):
+    def _prepare_function(self, func, update_globals=None):
         '''Alter the given function global namespace with the common ground
         for @depends, and @template.
         '''
         if not inspect.isfunction(func):
             raise TypeError("Unexpected type: '%s'" % type(func).__name__)
         if func in self._prepared_functions:
-            return func, func.func_globals
+            return func
 
         glob = SandboxedGlobal(
             (k, v) for k, v in func.func_globals.iteritems()
@@ -1037,6 +1038,8 @@ class ConfigureSandbox(dict):
             os=self.OS,
             log=self.log_impl,
         )
+        if update_globals:
+            update_globals(glob)
 
         # The execution model in the sandbox doesn't guarantee the execution
         # order will always be the same for a given function, and if it uses
@@ -1070,4 +1073,4 @@ class ConfigureSandbox(dict):
             return new_func(*args, **kwargs)
 
         self._prepared_functions.add(wrapped)
-        return wrapped, glob
+        return wrapped
diff --git a/python/mozbuild/mozbuild/configure/lint.py b/python/mozbuild/mozbuild/configure/lint.py
index b02ffcba6f86..1a6f5527ff1e 100644
--- a/python/mozbuild/mozbuild/configure/lint.py
+++ b/python/mozbuild/mozbuild/configure/lint.py
@@ -127,7 +127,7 @@ class LintSandbox(ConfigureSandbox):
             all_args.append(func_args.varargs)
         used_args = set()
 
-        for op, arg in disassemble_as_iter(func):
+        for op, arg, _ in disassemble_as_iter(func):
             if op in ('LOAD_FAST', 'LOAD_CLOSURE'):
                 if arg in all_args:
                     used_args.add(arg)
@@ -156,7 +156,7 @@ class LintSandbox(ConfigureSandbox):
             # - don't use global variables
             if func in self._has_imports or func.func_closure:
                 return True
-            for op, arg in disassemble_as_iter(func):
+            for op, arg, _ in disassemble_as_iter(func):
                 if op in ('LOAD_GLOBAL', 'STORE_GLOBAL'):
                     # There is a fake os module when one is not imported,
                     # and it's allowed for functions without a --help
@@ -289,3 +289,27 @@ class LintSandbox(ConfigureSandbox):
             self._has_imports.add(func)
             return wrapper(func)
         return decorator
+
+    def _prepare_function(self, func, update_globals=None):
+        wrapped = super(LintSandbox, self)._prepare_function(func, update_globals)
+        _, glob = self.unwrap(wrapped)
+        imports = set()
+        for _from, _import, _as in self._imports.get(func, ()):
+            if _as:
+                imports.add(_as)
+            else:
+                what = _import.split('.')[0]
+                imports.add(what)
+        for op, arg, line in disassemble_as_iter(func):
+            code = func.func_code
+            if op == 'LOAD_GLOBAL' and \
+                    arg not in glob and \
+                    arg not in imports and \
+                    arg not in glob['__builtins__'] and \
+                    arg not in code.co_varnames[:code.co_argcount]:
+                # Raise the same kind of error as what would happen during
+                # execution.
+                e = NameError("global name '{}' is not defined".format(arg))
+                self._raise_from(e, func, line)
+
+        return wrapped
diff --git a/python/mozbuild/mozbuild/configure/lint_util.py b/python/mozbuild/mozbuild/configure/lint_util.py
index 6e5ea2ae8e63..4d6fea3f63b1 100644
--- a/python/mozbuild/mozbuild/configure/lint_util.py
+++ b/python/mozbuild/mozbuild/configure/lint_util.py
@@ -6,10 +6,19 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 import dis
 import inspect
+import itertools
+
+
+# Like python 3.2's itertools.accumulate
+def accumulate(iterable):
+    t = 0
+    for i in iterable:
+        t += i
+        yield t
 
 
 # dis.dis only outputs to stdout. This is a modified version that
-# returns an iterator.
+# returns an iterator, and includes line numbers.
 def disassemble_as_iter(co):
     if inspect.ismethod(co):
         co = co.im_func
@@ -20,7 +29,20 @@ def disassemble_as_iter(co):
     i = 0
     extended_arg = 0
     free = None
+    line = 0
+    # co_lnotab is a string where each pair of consecutive character is
+    # (chr(byte_increment), chr(line_increment)), mapping bytes in co_code
+    # to line numbers relative to co_firstlineno.
+    # We want to iterate over pairs of (accumulated_byte, accumulated_line).
+    lnotab = itertools.chain(
+        itertools.izip(accumulate(ord(c) for c in co.co_lnotab[0::2]),
+                       accumulate(ord(c) for c in co.co_lnotab[1::2])),
+        (None,))
+    next_byte_line = lnotab.next()
     while i < n:
+        while next_byte_line and i >= next_byte_line[0]:
+            line = next_byte_line[1]
+            next_byte_line = lnotab.next()
         c = code[i]
         op = ord(c)
         opname = dis.opname[op]
@@ -33,20 +55,20 @@ def disassemble_as_iter(co):
                 extended_arg = arg * 65536
                 continue
             if op in dis.hasconst:
-                yield opname, co.co_consts[arg]
+                yield opname, co.co_consts[arg], line
             elif op in dis.hasname:
-                yield opname, co.co_names[arg]
+                yield opname, co.co_names[arg], line
             elif op in dis.hasjrel:
-                yield opname, i + arg
+                yield opname, i + arg, line
             elif op in dis.haslocal:
-                yield opname, co.co_varnames[arg]
+                yield opname, co.co_varnames[arg], line
             elif op in dis.hascompare:
-                yield opname, dis.cmp_op[arg]
+                yield opname, dis.cmp_op[arg], line
             elif op in dis.hasfree:
                 if free is None:
                     free = co.co_cellvars + co.co_freevars
-                yield opname, free[arg]
+                yield opname, free[arg], line
             else:
-                yield opname, None
+                yield opname, None, line
         else:
-            yield opname, None
+            yield opname, None, line
diff --git a/python/mozbuild/mozbuild/test/configure/test_lint.py b/python/mozbuild/mozbuild/test/configure/test_lint.py
index 3b2c76fd17d7..9d7dd425b46d 100644
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -373,5 +373,21 @@ class TestLint(unittest.TestCase):
                           '`help` should contain "{Enable|Disable}" because of '
                           'non-constant default')
 
+    def test_undefined_global(self):
+        with self.assertRaisesFromLine(NameError, 6) as e:
+            with self.moz_configure('''
+                option(env='FOO', help='foo')
+                @depends('FOO')
+                def foo(value):
+                    if value:
+                        return unknown
+                    return value
+            '''):
+                self.lint_test()
+
+        self.assertEquals(e.exception.message,
+                          "global name 'unknown' is not defined")
+
+
 if __name__ == '__main__':
     main()

From e6ab5d959aa5820f9e5c31b6d82827375a996ec4 Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Fri, 8 Feb 2019 13:48:36 +0000
Subject: [PATCH 11/15] Bug 1516642 - Add a function declaration for
 arc4random_buf in expat. r=peterv

The function has been in bionic (Android's libc since the first commit
in the upstream repository), but it's not been in stdlib.h until
recently. As it happens, we have a similar declaration in
xpcom/base/nsUUIDGenerator.cpp.

Differential Revision: https://phabricator.services.mozilla.com/D19120

--HG--
extra : moz-landing-system : lando
---
 parser/expat/lib/xmlparse.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/parser/expat/lib/xmlparse.c b/parser/expat/lib/xmlparse.c
index 209f20e094dc..12a7fc232e5f 100644
--- a/parser/expat/lib/xmlparse.c
+++ b/parser/expat/lib/xmlparse.c
@@ -818,6 +818,13 @@ gather_time_entropy(void)
 # include <bsd/stdlib.h>
 #endif
 
+/* BEGIN MOZILLA CHANGE (not all Android NDK versions have the function
+ * declaration, although the function has been available in bionic forever) */
+#if defined(HAVE_ARC4RANDOM_BUF) && defined(__ANDROID__)
+__attribute__((visibility("default"))) void arc4random_buf(void*, size_t);
+#endif
+/* END MOZILLA CHANGE */
+
 static unsigned long
 ENTROPY_DEBUG(const char * label, unsigned long entropy) {
 /* BEGIN MOZILLA CHANGE (don't getenv every time we set up a hash) */

From d3d50b644736ad05eebf2db38600d2a95634dc89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= <florian@queze.net>
Date: Sat, 9 Feb 2019 11:23:52 +0000
Subject: [PATCH 12/15] Bug 1526351 - avoid needlessly importing
 DownloadUIHelper.jsm and nsPrompter.js during shutdown, r=Felipe.

Differential Revision: https://phabricator.services.mozilla.com/D19187

--HG--
extra : moz-landing-system : lando
---
 .../downloads/DownloadIntegration.jsm         | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/toolkit/components/downloads/DownloadIntegration.jsm b/toolkit/components/downloads/DownloadIntegration.jsm
index ca381cf0c965..c835fc38864e 100644
--- a/toolkit/components/downloads/DownloadIntegration.jsm
+++ b/toolkit/components/downloads/DownloadIntegration.jsm
@@ -879,18 +879,25 @@ var DownloadObserver = {
    *        The type of prompt notification depending on the observer.
    */
   _confirmCancelDownloads: function DO_confirmCancelDownload(
-    aCancel, aDownloadsCount, aPrompter, aPromptType) {
-    // If user has already dismissed the request, then do nothing.
-    if ((aCancel instanceof Ci.nsISupportsPRBool) && aCancel.data) {
-      return;
-    }
+    aCancel, aDownloadsCount, aPromptType) {
     // Handle test mode
     if (gCombinedDownloadIntegration._testPromptDownloads) {
       gCombinedDownloadIntegration._testPromptDownloads = aDownloadsCount;
       return;
     }
 
-    aCancel.data = aPrompter.confirmCancelDownloads(aDownloadsCount, aPromptType);
+    if (!aDownloadsCount) {
+      return;
+    }
+
+    // If user has already dismissed the request, then do nothing.
+    if ((aCancel instanceof Ci.nsISupportsPRBool) && aCancel.data) {
+      return;
+    }
+
+    let prompter = DownloadUIHelper.getPrompter();
+    aCancel.data = prompter.confirmCancelDownloads(aDownloadsCount,
+                                                   prompter[aPromptType]);
   },
 
   /**
@@ -908,22 +915,21 @@ var DownloadObserver = {
   // nsIObserver
   observe: function DO_observe(aSubject, aTopic, aData) {
     let downloadsCount;
-    let p = DownloadUIHelper.getPrompter();
     switch (aTopic) {
       case "quit-application-requested":
         downloadsCount = this._publicInProgressDownloads.size +
                          this._privateInProgressDownloads.size;
-        this._confirmCancelDownloads(aSubject, downloadsCount, p, p.ON_QUIT);
+        this._confirmCancelDownloads(aSubject, downloadsCount, "ON_QUIT");
         break;
       case "offline-requested":
         downloadsCount = this._publicInProgressDownloads.size +
                          this._privateInProgressDownloads.size;
-        this._confirmCancelDownloads(aSubject, downloadsCount, p, p.ON_OFFLINE);
+        this._confirmCancelDownloads(aSubject, downloadsCount, "ON_OFFLINE");
         break;
       case "last-pb-context-exiting":
         downloadsCount = this._privateInProgressDownloads.size;
-        this._confirmCancelDownloads(aSubject, downloadsCount, p,
-                                     p.ON_LEAVE_PRIVATE_BROWSING);
+        this._confirmCancelDownloads(aSubject, downloadsCount,
+                                     "ON_LEAVE_PRIVATE_BROWSING");
         break;
       case "last-pb-context-exited":
         let promise = (async function() {

From 004144fdf9d80decfda439e1a5a77c6570ef46ed Mon Sep 17 00:00:00 2001
From: David Major <dmajor@mozilla.com>
Date: Fri, 8 Feb 2019 09:03:30 +0000
Subject: [PATCH 13/15] Bug 1525957 - Bump clang-cl to 8.0.0rc2 r=glandium

Differential Revision: https://phabricator.services.mozilla.com/D19003

--HG--
extra : moz-landing-system : lando
---
 build/build-clang/clang-win64.json      | 12 ++++++------
 build/moz.configure/toolchain.configure |  2 --
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/build/build-clang/clang-win64.json b/build/build-clang/clang-win64.json
index b46fee917c36..3e772061a409 100644
--- a/build/build-clang/clang-win64.json
+++ b/build/build-clang/clang-win64.json
@@ -1,14 +1,14 @@
 {
-    "llvm_revision": "352000",
+    "llvm_revision": "353414",
     "stages": "3",
     "build_libcxx": false,
     "build_type": "Release",
     "assertions": false,
-    "llvm_repo": "https://llvm.org/svn/llvm-project/llvm/tags/RELEASE_800/rc1",
-    "clang_repo": "https://llvm.org/svn/llvm-project/cfe/tags/RELEASE_800/rc1",
-    "lld_repo": "https://llvm.org/svn/llvm-project/lld/tags/RELEASE_800/rc1",
-    "compiler_repo": "https://llvm.org/svn/llvm-project/compiler-rt/tags/RELEASE_800/rc1",
-    "libcxx_repo": "https://llvm.org/svn/llvm-project/libcxx/tags/RELEASE_800/rc1",
+    "llvm_repo": "https://llvm.org/svn/llvm-project/llvm/tags/RELEASE_800/rc2",
+    "clang_repo": "https://llvm.org/svn/llvm-project/cfe/tags/RELEASE_800/rc2",
+    "lld_repo": "https://llvm.org/svn/llvm-project/lld/tags/RELEASE_800/rc2",
+    "compiler_repo": "https://llvm.org/svn/llvm-project/compiler-rt/tags/RELEASE_800/rc2",
+    "libcxx_repo": "https://llvm.org/svn/llvm-project/libcxx/tags/RELEASE_800/rc2",
     "python_path": "c:/mozilla-build/python/python.exe",
     "cc": "cl.exe",
     "cxx": "cl.exe",
diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure
index 0e3126d3fe87..154621cd8e6e 100755
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -1628,8 +1628,6 @@ def lto(value, pgo, c_compiler):
                 cflags.append("-flto")
             else:
                 cflags.append("-flto=thin")
-                # Workaround for https://bugs.llvm.org/show_bug.cgi?id=40414
-                cflags.append("-fsplit-lto-unit")
             # With clang-cl, -flto can only be used with -c or -fuse-ld=lld.
             # AC_TRY_LINKs during configure don't have -c, so pass -fuse-ld=lld.
             cflags.append("-fuse-ld=lld");

From 12675bb16f61e05dc65eedb02406276cc6fa979e Mon Sep 17 00:00:00 2001
From: Marco Bonardo <mbonardo@mozilla.com>
Date: Sat, 9 Feb 2019 13:29:21 +0000
Subject: [PATCH 14/15] Bug 1519538 - Disable idle-daily notifications in tests
 through user.js files. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D19017

--HG--
extra : moz-landing-system : lando
---
 dom/indexedDB/test/unit/test_idle_maintenance.js |  5 ++---
 testing/mochitest/runtests.py                    |  1 -
 testing/profiles/perf/user.js                    |  3 +++
 testing/profiles/unittest-required/user.js       |  3 +++
 testing/profiles/xpcshell/user.js                |  3 +++
 testing/talos/talos/config.py                    |  6 +-----
 toolkit/modules/Troubleshoot.jsm                 |  1 +
 widget/nsIdleService.cpp                         | 11 ++++++++++-
 8 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/dom/indexedDB/test/unit/test_idle_maintenance.js b/dom/indexedDB/test/unit/test_idle_maintenance.js
index b5f2f63bcd9e..8f627fe1988e 100644
--- a/dom/indexedDB/test/unit/test_idle_maintenance.js
+++ b/dom/indexedDB/test/unit/test_idle_maintenance.js
@@ -15,9 +15,8 @@ function* testSteps()
 
   Services.perms.add(uri, "indexedDB", Ci.nsIPermissionManager.ALLOW_ACTION);
 
-  info("Setting idle preferences to prevent real 'idle-daily' notification");
-
-  Services.prefs.setIntPref("idle.lastDailyNotification", (Date.now() / 1000) - 10);
+  // The idle-daily notification is disabled in xpchsell tests, so we don't
+  // need to do anything special to disable it for this test.
 
   info("Activating real idle service");
 
diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py
index 64bae23b508e..ee89d0d5cf42 100644
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1879,7 +1879,6 @@ toolbar#nav-bar {
         prefs = {
             "browser.tabs.remote.autostart": options.e10s,
             "dom.ipc.tabs.nested.enabled": options.nested_oop,
-            "idle.lastDailyNotification": int(time.time()),
             # Enable tracing output for detailed failures in case of
             # failing connection attempts, and hangs (bug 1397201)
             "marionette.log.level": "Trace",
diff --git a/testing/profiles/perf/user.js b/testing/profiles/perf/user.js
index 3ef5ce2cc810..f1e751e8ceda 100644
--- a/testing/profiles/perf/user.js
+++ b/testing/profiles/perf/user.js
@@ -63,6 +63,9 @@ user_pref("extensions.update.url", "http://127.0.0.1/extensions-dummy/updateURL"
 user_pref("extensions.webservice.discoverURL", "http://127.0.0.1/extensions-dummy/discoveryURL");
 user_pref("identity.fxaccounts.auth.uri", "https://127.0.0.1/fxa-dummy/");
 user_pref("identity.fxaccounts.migrateToDevEdition", false);
+// Avoid idle-daily notifications, to avoid expensive operations that may
+// cause unexpected test timeouts.
+user_pref("idle.lastDailyNotification", -1);
 // Make tests run consistently on DevEdition (which has a lightweight theme
 // selected by default).
 user_pref("lightweightThemes.selectedThemeID", "");
diff --git a/testing/profiles/unittest-required/user.js b/testing/profiles/unittest-required/user.js
index a57df66d5ed5..c104a656581e 100644
--- a/testing/profiles/unittest-required/user.js
+++ b/testing/profiles/unittest-required/user.js
@@ -153,6 +153,9 @@ user_pref("gfx.logging.level", 1);
 user_pref("identity.fxaccounts.auth.uri", "https://{server}/fxa-dummy/");
 // Ditto for all the FxA content root URI.
 user_pref("identity.fxaccounts.remote.root", "https://{server}/");
+// Avoid idle-daily notifications, to avoid expensive operations that may
+// cause unexpected test timeouts.
+user_pref("idle.lastDailyNotification", -1);
 user_pref("javascript.options.showInConsole", true);
 user_pref("layout.accessiblecaret.enabled_on_touch", false);
 // Make sure CSS error reporting is enabled for tests
diff --git a/testing/profiles/xpcshell/user.js b/testing/profiles/xpcshell/user.js
index 5f86f5bc085d..700fddb88c35 100644
--- a/testing/profiles/xpcshell/user.js
+++ b/testing/profiles/xpcshell/user.js
@@ -16,3 +16,6 @@ user_pref("toolkit.telemetry.server", "https://%(server)s/telemetry-dummy");
 // all processes would run at low priority, which is not desirable, so we
 // disable the process priority manager entirely here.
 user_pref("dom.ipc.processPriorityManager.enabled", false);
+// Avoid idle-daily notifications, to avoid expensive operations that may
+// cause unexpected test timeouts.
+user_pref("idle.lastDailyNotification", -1);
diff --git a/testing/talos/talos/config.py b/testing/talos/talos/config.py
index 62792c864c10..241e05e425e5 100644
--- a/testing/talos/talos/config.py
+++ b/testing/talos/talos/config.py
@@ -6,7 +6,6 @@ from __future__ import absolute_import, print_function
 import copy
 import os
 import sys
-import time
 
 from mozlog.commandline import setup_logging
 from talos import utils, test
@@ -142,10 +141,7 @@ def set_webserver(config):
 
 @validator
 def update_prefs(config):
-    config.setdefault('preferences', {}).update({
-        # Bug 1383896 - reduces noise in tests
-        'idle.lastDailyNotification': int(time.time()),
-    })
+    config.setdefault('preferences', {})
 
     # update prefs from command line
     prefs = config.pop('extraPrefs')
diff --git a/toolkit/modules/Troubleshoot.jsm b/toolkit/modules/Troubleshoot.jsm
index f2475b08a397..6163cc1a7234 100644
--- a/toolkit/modules/Troubleshoot.jsm
+++ b/toolkit/modules/Troubleshoot.jsm
@@ -56,6 +56,7 @@ const PREFS_WHITELIST = [
   "general.useragent.",
   "gfx.",
   "html5.",
+  "idle.",
   "image.",
   "javascript.",
   "keyword.",
diff --git a/widget/nsIdleService.cpp b/widget/nsIdleService.cpp
index b8741c795e1a..4e35f3f93cb4 100644
--- a/widget/nsIdleService.cpp
+++ b/widget/nsIdleService.cpp
@@ -156,8 +156,17 @@ void nsIdleServiceDaily::Init() {
   // get ready to send an idle-daily event. Otherwise set a timer targeted
   // at 24 hours past the last idle-daily we sent.
 
-  int32_t nowSec = static_cast<int32_t>(PR_Now() / PR_USEC_PER_SEC);
   int32_t lastDaily = Preferences::GetInt(PREF_LAST_DAILY, 0);
+  // Setting the pref to -1 allows to disable idle-daily, and it's particularly
+  // useful in tests. Normally there should be no need for the user to set
+  // this value.
+  if (lastDaily == -1) {
+    MOZ_LOG(sLog, LogLevel::Debug,
+            ("nsIdleServiceDaily: Init: disabled idle-daily"));
+    return;
+  }
+
+  int32_t nowSec = static_cast<int32_t>(PR_Now() / PR_USEC_PER_SEC);
   if (lastDaily < 0 || lastDaily > nowSec) {
     // The time is bogus, use default.
     lastDaily = 0;

From 5f4b5c30cbf767619a07908010683c884727b6b9 Mon Sep 17 00:00:00 2001
From: Marco Bonardo <mbonardo@mozilla.com>
Date: Sat, 9 Feb 2019 15:39:20 +0000
Subject: [PATCH 15/15] Bug 1512648 - Implement a Quantum Bar event bufferer.
 r=adw

Differential Revision: https://phabricator.services.mozilla.com/D18980

--HG--
extra : moz-landing-system : lando
---
 .../components/urlbar/UrlbarEventBufferer.jsm | 313 ++++++++++++++++++
 browser/components/urlbar/UrlbarInput.jsm     |  26 +-
 .../urlbar/UrlbarMuxerUnifiedComplete.jsm     |   2 +-
 .../urlbar/UrlbarProviderOpenTabs.jsm         |   2 +-
 .../urlbar/UrlbarProviderUnifiedComplete.jsm  |   2 +-
 .../urlbar/UrlbarProvidersManager.jsm         |   2 +-
 browser/components/urlbar/UrlbarTokenizer.jsm |   2 +-
 browser/components/urlbar/UrlbarView.jsm      |  20 +-
 browser/components/urlbar/moz.build           |   1 +
 .../urlbar/tests/browser/browser.ini          |   1 +
 .../tests/browser/browser_canonizeURL.js      |   7 +-
 .../browser/browser_locationBarCommand.js     |  71 ++--
 12 files changed, 402 insertions(+), 47 deletions(-)
 create mode 100644 browser/components/urlbar/UrlbarEventBufferer.jsm

diff --git a/browser/components/urlbar/UrlbarEventBufferer.jsm b/browser/components/urlbar/UrlbarEventBufferer.jsm
new file mode 100644
index 000000000000..4e8b8a9593f1
--- /dev/null
+++ b/browser/components/urlbar/UrlbarEventBufferer.jsm
@@ -0,0 +1,313 @@
+/* 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/. */
+
+"use strict";
+
+var EXPORTED_SYMBOLS = [
+  "UrlbarEventBufferer",
+];
+
+const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  AppConstants: "resource://gre/modules/AppConstants.jsm",
+  clearTimeout: "resource://gre/modules/Timer.jsm",
+  Services: "resource://gre/modules/Services.jsm",
+  setTimeout: "resource://gre/modules/Timer.jsm",
+  Log: "resource://gre/modules/Log.jsm",
+});
+
+XPCOMUtils.defineLazyGetter(this, "logger", () =>
+  Log.repository.getLogger("Urlbar.EventBufferer"));
+
+// Maximum time events can be deferred for.
+const DEFERRING_TIMEOUT_MS = 200;
+
+// Array of keyCodes to defer.
+const DEFERRED_KEY_CODES = new Set([
+  KeyboardEvent.DOM_VK_RETURN,
+  KeyboardEvent.DOM_VK_DOWN,
+  KeyboardEvent.DOM_VK_TAB,
+]);
+
+// Status of the current or last query.
+const QUERY_STATUS = {
+  UKNOWN: 0,
+  RUNNING: 1,
+  COMPLETE: 2,
+};
+
+/**
+ * The UrlbarEventBufferer can queue up events and replay them later, to make
+ * the urlbar results more predictable.
+ *
+ * Search results arrive asynchronously, which means that keydown events may
+ * arrive before results do, and therefore not have the effect the user intends.
+ * That's especially likely to happen with the down arrow and enter keys, due to
+ * the one-off search buttons: if the user very quickly pastes something in the
+ * input, presses the down arrow key, and then hits enter, they are probably
+ * expecting to visit the first result.  But if there are no results, then
+ * pressing down and enter will trigger the first one-off button.
+ * To prevent that undesirable behavior, certain keys are buffered and deferred
+ * until more results arrive, at which time they're replayed.
+ */
+class UrlbarEventBufferer {
+  /**
+   * Initialises the class.
+   * @param {UrlbarInput} input The urlbar input object.
+   */
+  constructor(input) {
+    this.input = input;
+    // A queue of deferred events.
+    this._eventsQueue = [];
+    // If this timer fires, we will unconditionally replay all the deferred
+    // events so that, after a certain point, we don't keep blocking the user's
+    // actions, when nothing else has caused the events to be replayed.
+    // At that point we won't check whether it's safe to replay the events,
+    // because otherwise it may look like we ignored the user's actions.
+    this._deferringTimeout = null;
+
+    // Tracks the current or last query status.
+    this._lastQuery = {
+      // The time at which the current or last search was started. This is used
+      // to check how much time passed while deferring the user's actions. Must
+      // be set using the monotonic Cu.now() helper.
+      startDate: null,
+      // Status of the query; one of QUERY_STATUS.*
+      status: QUERY_STATUS.UKNOWN,
+      // The searchString from the query context.
+      searchString: "",
+      // The currently returned results.
+      results: [],
+    };
+
+    // Start listening for queries.
+    this.input.controller.addQueryListener(this);
+  }
+
+  // UrlbarController listener methods.
+  onQueryStarted(queryContext) {
+    this._lastQuery = {
+      startDate: Cu.now,
+      status: QUERY_STATUS.RUNNING,
+      searchString: queryContext.searchString,
+      results: [],
+    };
+    if (this._deferringTimeout) {
+      clearTimeout(this._deferringTimeout);
+      this._deferringTimeout = null;
+    }
+  }
+
+  onQueryCancelled(queryContext) {
+    this._lastQuery.status = QUERY_STATUS.COMPLETE;
+  }
+
+  onQueryFinished(queryContext) {
+    this._lastQuery.status = QUERY_STATUS.COMPLETE;
+  }
+
+  onQueryResults(queryContext) {
+    this._lastQuery.results = queryContext.results;
+    // Ensure this runs after other results handling code.
+    Services.tm.dispatchToMainThread(this.replaySafeDeferredEvents.bind(this));
+  }
+
+  /**
+   * Receives DOM events, eventually queues them up, and passes them back to the
+   * input object when it's the right time.
+   * @param {Event} event DOM event from the <textbox>.
+   * @returns {boolean}
+   */
+  handleEvent(event) {
+    switch (event.type) {
+      case "keydown":
+        if (this.shouldDeferEvent(event)) {
+          this.deferEvent(event);
+          return false;
+        }
+        break;
+      case "blur":
+        logger.debug("Clearing queue on blur");
+        // The input field was blurred, pending events don't matter anymore.
+        // Clear the timeout and the queue.
+        this._eventsQueue.length = 0;
+        if (this._deferringTimeout) {
+          clearTimeout(this._deferringTimeout);
+          this._deferringTimeout = null;
+        }
+        break;
+    }
+    // Just pass back the event to the input object.
+    return this.input.handleEvent(event);
+  }
+
+  /**
+   * Adds a deferrable event to the deferred event queue.
+   * @param {Event} event The event to defer.
+   */
+  deferEvent(event) {
+    // TODO: once one-off buttons are implemented, figure out if the following
+    // is true for the quantum bar as well: somehow event.defaultPrevented ends
+    // up true for deferred events.  Autocomplete ignores defaultPrevented
+    // events, which means it would ignore replayed deferred events if we didn't
+    // tell it to bypass defaultPrevented through urlbarDeferred.
+    // Check we don't try to defer events more than once.
+    if (event.urlbarDeferred) {
+      throw new Error(`Event ${event.type}:${event.keyCode} already deferred!`);
+    }
+    logger.debug(`Deferring ${event.type}:${event.keyCode} event`);
+    // Mark the event as deferred.
+    event.urlbarDeferred = true;
+    // Also store the current search string, as an added safety check. If the
+    // string will differ later, the event is stale and should be dropped.
+    event.searchString = this._lastQuery.searchString;
+    this._eventsQueue.push(event);
+
+    if (!this._deferringTimeout) {
+      let elapsed = Cu.now() - this._lastQuery.startDate;
+      let remaining = DEFERRING_TIMEOUT_MS - elapsed;
+      this._deferringTimeout = setTimeout(() => {
+        this.replayAllDeferredEvents();
+        this._deferringTimeout = null;
+      }, Math.max(0, remaining));
+    }
+  }
+
+  /**
+   * Unconditionally replays all deferred key events.  This does not check
+   * whether it's safe to replay the events; use replaySafeDeferredEvents
+   * for that.  Use this method when you must replay all events, so that it
+   * does not appear that we ignored the user's input.
+   */
+  replayAllDeferredEvents() {
+    let event = this._eventsQueue.shift();
+    if (!event) {
+      return;
+    }
+    this.replayDeferredEvent(event);
+    Services.tm.dispatchToMainThread(this.replayAllDeferredEvents.bind(this));
+  }
+
+  /**
+   * Replays deferred events if it's a safe time to do it.
+   * @see isSafeToPlayDeferredEvent
+   */
+  replaySafeDeferredEvents() {
+    if (!this._eventsQueue.length) {
+      return;
+    }
+    let event = this._eventsQueue[0];
+    if (!this.isSafeToPlayDeferredEvent(event)) {
+      return;
+    }
+    // Remove the event from the queue and play it.
+    this._eventsQueue.shift();
+    this.replayDeferredEvent(event);
+    // Continue with the next event.
+    Services.tm.dispatchToMainThread(this.replaySafeDeferredEvents.bind(this));
+  }
+
+  /**
+   * Replays a deferred event.
+   * @param {Event} event The deferred event to replay.
+   */
+  replayDeferredEvent(event) {
+    // Safety check: handle only if the search string didn't change.
+    if (event.searchString == this._lastQuery.searchString) {
+      this.input.handleEvent(event);
+    }
+  }
+
+  /**
+   * Checks whether a given event should be deferred
+   * @param {Event} event The event that should maybe be deferred.
+   * @returns {boolean} Whether the event should be deferred.
+   */
+  shouldDeferEvent(event) {
+    // If any event has been deferred for this search, then defer all subsequent
+    // events so that the user does not experience them out of order.
+    // All events will be replayed when _deferringTimeout fires.
+    if (this._eventsQueue.length > 0) {
+      return true;
+    }
+
+    // At this point, no events have been deferred for this search; we must
+    // figure out if this event should be deferred.
+    let isMacNavigation = AppConstants.platform == "macosx" &&
+                          event.ctrlKey &&
+                          this.input.view.isOpen &&
+                          (event.key === "n" || event.key === "p");
+    if (!DEFERRED_KEY_CODES.has(event.keyCode) && !isMacNavigation) {
+      return false;
+    }
+
+    // This is an event that we'd defer, but if enough time has passed since the
+    // start of the search, we don't want to block the user's workflow anymore.
+    if (this._lastQuery.startDate + DEFERRING_TIMEOUT_MS <= Cu.now()) {
+      return false;
+    }
+
+    if (event.keyCode == KeyEvent.DOM_VK_TAB && !this.input.view.isOpen) {
+      // The view is closed and the user pressed the Tab key.  The focus should
+      // move out of the urlbar immediately.
+      return false;
+    }
+
+    return !this.isSafeToPlayDeferredEvent(event);
+  }
+
+  /**
+   * Returns true if the given deferred event can be played now without possibly
+   * surprising the user.  This depends on the state of the view, the results,
+   * and the type of event.
+   * Use this method only after determining that the event should be deferred,
+   * or after it has been deferred and you want to know if it can be played now.
+   * @param {Event} event The event.
+   * @returns {boolean} Whether the event can be played.
+   */
+  isSafeToPlayDeferredEvent(event) {
+    let waitingFirstResult = this._lastQuery.status == QUERY_STATUS.RUNNING &&
+                             !this._lastQuery.results.length;
+    if (event.keyCode == KeyEvent.DOM_VK_RETURN) {
+      // Play a deferred Enter if the heuristic result is not selected, or we
+      // are not waiting for the first results yet.
+      let selectedResult = this.input.view.selectedResult;
+      return (selectedResult && !selectedResult.heuristic) ||
+             !waitingFirstResult;
+    }
+
+    if (waitingFirstResult || !this.input.view.isOpen) {
+      // We're still waiting on the first results, or the popup hasn't opened
+      // yet, so not safe.
+      return false;
+    }
+
+    if (this._lastQuery.status == QUERY_STATUS.COMPLETE) {
+      // The view can't get any more results, so there's no need to further
+      // defer events.
+      return true;
+    }
+
+    let isMacDownNavigation = AppConstants.platform == "macosx" &&
+                              event.ctrlKey &&
+                              this.input.view.isOpen &&
+                              event.key === "n";
+    if (event.keyCode == KeyEvent.DOM_VK_DOWN || isMacDownNavigation) {
+      // Don't play the event if the last result is selected so that the user
+      // doesn't accidentally arrow down into the one-off buttons when they
+      // didn't mean to.
+      return !this.lastResultIsSelected;
+    }
+
+    return true;
+  }
+
+  get lastResultIsSelected() {
+    // TODO: Once one-off buttons are fully implemented, it would be nice to have
+    // a better way to check if the next down will focus one-off buttons.
+    let results = this._lastQuery.results;
+    return results.length &&
+           results[results.length - 1] == this.input.view.selectedResult;
+  }
+}
diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm
index 8eab9c1ef654..445643580adf 100644
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -15,6 +15,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   ReaderMode: "resource://gre/modules/ReaderMode.jsm",
   Services: "resource://gre/modules/Services.jsm",
   UrlbarController: "resource:///modules/UrlbarController.jsm",
+  UrlbarEventBufferer: "resource:///modules/UrlbarEventBufferer.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarQueryContext: "resource:///modules/UrlbarUtils.jsm",
   UrlbarTokenizer: "resource:///modules/UrlbarTokenizer.jsm",
@@ -102,18 +103,21 @@ class UrlbarInput {
       return new UrlbarValueFormatter(this);
     });
 
+    // The event bufferer handles some events, queues them up, and calls back
+    // our handleEvent at the right time.
+    this.eventBufferer = new UrlbarEventBufferer(this);
+    this.inputField.addEventListener("blur", this.eventBufferer);
+    this.inputField.addEventListener("keydown", this.eventBufferer);
+
+    const inputFieldEvents = [
+      "focus", "input", "keyup", "mouseover", "paste", "scrollend", "select",
+      "overflow", "underflow",
+    ];
+    for (let name of inputFieldEvents) {
+      this.inputField.addEventListener(name, this);
+    }
+
     this.addEventListener("mousedown", this);
-    this.inputField.addEventListener("blur", this);
-    this.inputField.addEventListener("focus", this);
-    this.inputField.addEventListener("input", this);
-    this.inputField.addEventListener("mouseover", this);
-    this.inputField.addEventListener("overflow", this);
-    this.inputField.addEventListener("underflow", this);
-    this.inputField.addEventListener("paste", this);
-    this.inputField.addEventListener("scrollend", this);
-    this.inputField.addEventListener("select", this);
-    this.inputField.addEventListener("keydown", this);
-    this.inputField.addEventListener("keyup", this);
     this.view.panel.addEventListener("popupshowing", this);
     this.view.panel.addEventListener("popuphidden", this);
 
diff --git a/browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm b/browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm
index 4c197b850652..ff27077c2ed3 100644
--- a/browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm
+++ b/browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm
@@ -19,7 +19,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 });
 
 XPCOMUtils.defineLazyGetter(this, "logger", () =>
-  Log.repository.getLogger("Places.Urlbar.UrlbarMuxerUnifiedComplete"));
+  Log.repository.getLogger("Urlbar.Muxer.UnifiedComplete"));
 
 const RESULT_TYPE_TO_GROUP = new Map([
   [ UrlbarUtils.RESULT_TYPE.TAB_SWITCH, UrlbarUtils.RESULT_GROUP.GENERAL ],
diff --git a/browser/components/urlbar/UrlbarProviderOpenTabs.jsm b/browser/components/urlbar/UrlbarProviderOpenTabs.jsm
index 4b10dd3978fe..ae38eb5dba40 100644
--- a/browser/components/urlbar/UrlbarProviderOpenTabs.jsm
+++ b/browser/components/urlbar/UrlbarProviderOpenTabs.jsm
@@ -22,7 +22,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 });
 
 XPCOMUtils.defineLazyGetter(this, "logger",
-  () => Log.repository.getLogger("Places.Urlbar.Provider.OpenTabs"));
+  () => Log.repository.getLogger("Urlbar.Provider.OpenTabs"));
 
 /**
  * Class used to create the provider.
diff --git a/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm b/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
index e9fb7d7c56af..fd97fd0389c7 100644
--- a/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
+++ b/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
@@ -27,7 +27,7 @@ XPCOMUtils.defineLazyServiceGetter(this, "unifiedComplete",
   "nsIAutoCompleteSearch");
 
 XPCOMUtils.defineLazyGetter(this, "logger",
-  () => Log.repository.getLogger("Places.Urlbar.Provider.UnifiedComplete"));
+  () => Log.repository.getLogger("Urlbar.Provider.UnifiedComplete"));
 
 // See UnifiedComplete.
 const TITLE_TAGS_SEPARATOR = " \u2013 ";
diff --git a/browser/components/urlbar/UrlbarProvidersManager.jsm b/browser/components/urlbar/UrlbarProvidersManager.jsm
index b9b9b3cbe040..9dd1d28c2a46 100644
--- a/browser/components/urlbar/UrlbarProvidersManager.jsm
+++ b/browser/components/urlbar/UrlbarProvidersManager.jsm
@@ -23,7 +23,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 });
 
 XPCOMUtils.defineLazyGetter(this, "logger", () =>
-  Log.repository.getLogger("Places.Urlbar.ProvidersManager"));
+  Log.repository.getLogger("Urlbar.ProvidersManager"));
 
 // List of available local providers, each is implemented in its own jsm module
 // and will track different queries internally by queryContext.
diff --git a/browser/components/urlbar/UrlbarTokenizer.jsm b/browser/components/urlbar/UrlbarTokenizer.jsm
index 0e167506fa96..e670a6565f53 100644
--- a/browser/components/urlbar/UrlbarTokenizer.jsm
+++ b/browser/components/urlbar/UrlbarTokenizer.jsm
@@ -18,7 +18,7 @@ const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.defineModuleGetter(this, "Log",
                                "resource://gre/modules/Log.jsm");
 XPCOMUtils.defineLazyGetter(this, "logger", () =>
-  Log.repository.getLogger("Places.Urlbar.Tokenizer"));
+  Log.repository.getLogger("Urlbar.Tokenizer"));
 
 var UrlbarTokenizer = {
   // Regex matching on whitespaces.
diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm
index 52f5caef0e3b..870b71d60c3b 100644
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -142,12 +142,22 @@ class UrlbarView {
       fragment.appendChild(this._createRow(resultIndex));
     }
 
-    if (queryContext.preselected) {
-      this._selected = fragment.firstElementChild;
+    if (queryContext.lastResultCount == 0) {
+      if (queryContext.preselected) {
+        this._selected = fragment.firstElementChild;
+        this._selected.toggleAttribute("selected", true);
+      } else if (this._selected) {
+        // Clear the selection when we get a new set of results.
+        this._selected.toggleAttribute("selected", false);
+        this._selected = null;
+      }
+    } else if (this._selected) {
+      // Ensure the selection is stable.
+      // TODO bug 1523602: the selection should stay on the node that had it, if
+      // it's still in the current result set.
+      let resultIndex = this._selected.getAttribute("resultIndex");
+      this._selected = fragment.children[resultIndex];
       this._selected.toggleAttribute("selected", true);
-    } else if (queryContext.lastResultCount == 0) {
-      // Clear the selection when we get a new set of results.
-      this._selected = null;
     }
 
     // TODO bug 1523602: For now, clear the results for each set received.
diff --git a/browser/components/urlbar/moz.build b/browser/components/urlbar/moz.build
index 1c06274e9de0..e80f8b654c99 100644
--- a/browser/components/urlbar/moz.build
+++ b/browser/components/urlbar/moz.build
@@ -7,6 +7,7 @@ with Files("**"):
 
 EXTRA_JS_MODULES += [
     'UrlbarController.jsm',
+    'UrlbarEventBufferer.jsm',
     'UrlbarInput.jsm',
     'UrlbarMuxerUnifiedComplete.jsm',
     'UrlbarPrefs.jsm',
diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini
index 2b863fea4c11..cba03479fe6b 100644
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -15,6 +15,7 @@ support-files =
 [browser_autocomplete_edit_completed.js]
 [browser_autocomplete_no_title.js]
 [browser_canonizeURL.js]
+skip-if = true # Bug 1521702 comment 4, canonization is partially broken.
 [browser_locationBarCommand.js]
 [browser_locationBarExternalLoad.js]
 [browser_moz_action_link.js]
diff --git a/browser/components/urlbar/tests/browser/browser_canonizeURL.js b/browser/components/urlbar/tests/browser/browser_canonizeURL.js
index 9b8dbc9cfc20..8b2569a51629 100644
--- a/browser/components/urlbar/tests/browser/browser_canonizeURL.js
+++ b/browser/components/urlbar/tests/browser/browser_canonizeURL.js
@@ -8,11 +8,11 @@
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
 add_task(async function checkCtrlWorks() {
+  let defaultEngine = await Services.search.getDefault();
   let testcases = [
     ["example", "http://www.example.com/", { ctrlKey: true }],
     // Check that a direct load is not overwritten by a previous canonization.
     ["http://example.com/test/", "http://example.com/test/", {}],
-
     ["ex-ample", "http://www.ex-ample.com/", { ctrlKey: true }],
     ["  example ", "http://www.example.com/", { ctrlKey: true }],
     [" example/foo ", "http://www.example.com/foo", { ctrlKey: true }],
@@ -25,8 +25,8 @@ add_task(async function checkCtrlWorks() {
     ["1.1.1.1", "http://1.1.1.1/", { ctrlKey: true }],
     ["ftp://example", "ftp://example/", { ctrlKey: true }],
     ["ftp.example.bar", "http://ftp.example.bar/", { ctrlKey: true }],
-    ["ex ample", (await Services.search.getDefault()).getSubmission("ex ample", null, "keyword").uri.spec,
-      { ctrlKey: true }],
+    ["ex ample", defaultEngine.getSubmission("ex ample", null, "keyword").uri.spec,
+     { ctrlKey: true }],
   ];
 
   // Disable autoFill for this test, since it could mess up the results.
@@ -36,6 +36,7 @@ add_task(async function checkCtrlWorks() {
   ]});
 
   for (let [inputValue, expectedURL, options] of testcases) {
+    info(`Testing input string: "${inputValue}" - expected: "${expectedURL}"`);
     let promiseLoad =
       BrowserTestUtils.waitForDocLoadAndStopIt(expectedURL, gBrowser.selectedBrowser);
     gURLBar.focus();
diff --git a/browser/components/urlbar/tests/browser/browser_locationBarCommand.js b/browser/components/urlbar/tests/browser/browser_locationBarCommand.js
index a512e15794c3..13494995d93a 100644
--- a/browser/components/urlbar/tests/browser/browser_locationBarCommand.js
+++ b/browser/components/urlbar/tests/browser/browser_locationBarCommand.js
@@ -29,7 +29,7 @@ add_task(async function alt_left_click_test() {
     };
   });
 
-  triggerCommand(true, {altKey: true});
+  triggerCommand("click", {altKey: true});
 
   await saveURLPromise;
   ok(true, "SaveURL was called");
@@ -41,7 +41,7 @@ add_task(async function shift_left_click_test() {
 
   let destinationURL = "http://" + TEST_VALUE + "/";
   let newWindowPromise = BrowserTestUtils.waitForNewWindow({url: destinationURL});
-  triggerCommand(true, {shiftKey: true});
+  triggerCommand("click", {shiftKey: true});
   let win = await newWindowPromise;
 
   info("URL should be loaded in a new window");
@@ -65,7 +65,7 @@ add_task(async function right_click_test() {
   // Add a new tab.
   await promiseOpenNewTab();
 
-  triggerCommand(true, {button: 2});
+  triggerCommand("click", {button: 2});
 
   // Right click should do nothing (context menu will be shown).
   is(gURLBar.value, TEST_VALUE, "Urlbar still has the value we entered");
@@ -81,7 +81,7 @@ add_task(async function shift_accel_left_click_test() {
   let tab = await promiseOpenNewTab();
 
   let loadStartedPromise = promiseLoadStarted();
-  triggerCommand(true, {accelKey: true, shiftKey: true});
+  triggerCommand("click", {accelKey: true, shiftKey: true});
   await loadStartedPromise;
 
   // Check the load occurred in a new background tab.
@@ -101,25 +101,39 @@ add_task(async function shift_accel_left_click_test() {
 
 add_task(async function load_in_current_tab_test() {
   let tests = [
-    {desc: "Simple return keypress"},
-    {desc: "Left click on go button", click: true},
-    {desc: "Ctrl/Cmd+Return keypress", event: {accelKey: true}},
-    {desc: "Alt+Return keypress in a blank tab", event: {altKey: true}},
+    {
+      desc: "Simple return keypress",
+      type: "keypress",
+    },
+    {
+      desc: "Left click on go button",
+      type: "click",
+    },
+    {
+      desc: "Ctrl/Cmd+Return keypress",
+      type: "keypress",
+      details: {accelKey: true},
+    },
+    {
+      desc: "Alt+Return keypress in a blank tab",
+      type: "keypress",
+      details: {altKey: true},
+    },
   ];
 
-  for (let test of tests) {
-    info(`Running test: ${test.desc}`);
+  for (let {desc, type, details} of tests) {
+    info(`Running test: ${desc}`);
 
     // Add a new tab.
     let tab = await promiseOpenNewTab();
 
     // Trigger a load and check it occurs in the current tab.
     let loadStartedPromise = promiseLoadStarted();
-    triggerCommand(test.click || false, test.event || {});
+    triggerCommand(type, details);
     await loadStartedPromise;
 
     info("URL should be loaded in the current tab");
-    is(gURLBar.value, TEST_VALUE, "Urlbar still has the value we entered");
+    is(gURLBar.textValue, TEST_VALUE, "Urlbar still has the value we entered");
     await promiseCheckChildNoFocusedElement(gBrowser.selectedBrowser);
     is(document.activeElement, gBrowser.selectedBrowser, "Content window should be focused");
     is(gBrowser.selectedTab, tab, "New URL was loaded in the current tab");
@@ -131,19 +145,29 @@ add_task(async function load_in_current_tab_test() {
 
 add_task(async function load_in_new_tab_test() {
   let tests = [
-    {desc: "Ctrl/Cmd left click on go button", click: true, event: {accelKey: true}},
-    {desc: "Alt+Return keypress in a dirty tab", event: {altKey: true}, url: START_VALUE},
+    {
+      desc: "Ctrl/Cmd left click on go button",
+      type: "click",
+      details: {accelKey: true},
+      url: null,
+    },
+    {
+      desc: "Alt+Return keypress in a dirty tab",
+      type: "keypress",
+      details: {altKey: true},
+      url: START_VALUE,
+    },
   ];
 
-  for (let test of tests) {
-    info(`Running test: ${test.desc}`);
+  for (let {desc, type, details, url} of tests) {
+    info(`Running test: ${desc}`);
 
     // Add a new tab.
-    let tab = await promiseOpenNewTab(test.url || "about:blank");
+    let tab = await promiseOpenNewTab(url || "about:blank");
 
     // Trigger a load and check it occurs in the current tab.
     let tabSwitchedPromise = promiseNewTabSwitched();
-    triggerCommand(test.click || false, test.event || {});
+    triggerCommand(type, details);
     await tabSwitchedPromise;
 
     // Check the load occurred in a new tab.
@@ -159,18 +183,19 @@ add_task(async function load_in_new_tab_test() {
   }
 });
 
-function triggerCommand(shouldClick, event) {
+function triggerCommand(type, details = {}) {
   gURLBar.focus();
   gURLBar.value = "";
   EventUtils.sendString(TEST_VALUE);
 
-  if (shouldClick) {
+  if (type == "click") {
     ok(gURLBar.hasAttribute("usertyping"),
        "usertyping attribute must be set for the go button to be visible");
-
-    EventUtils.synthesizeMouseAtCenter(gURLBar.goButton, event);
+    EventUtils.synthesizeMouseAtCenter(gURLBar.goButton, details);
+  } else if (type == "keypress") {
+    EventUtils.synthesizeKey("KEY_Enter", details);
   } else {
-    EventUtils.synthesizeKey("KEY_Enter", event);
+    throw new Error("Unsupported event type");
   }
 }