Merge m-c to a CLOSED TREE f-t

browser/base/content/browser.css

@@ -242,6 +242,17 @@ toolbar[customizing] > .overflow-button {
 .titlebar-placeholder[type="fullscreen-button"]:-moz-locale-dir(rtl) {
   -moz-box-ordinal-group: 0;
 }
+
+/* In private windows, the #titlebar-content is higher because of the
+ * private browsing indicator. With the margin-top the titlebar buttons
+ * align to the top correctly in that case, but only if we don't stretch
+ * the box they're in because the container is too high, so we override
+ * the default alignment value (stretch).
+ */
+#main-window[tabsintitlebar] > #titlebar > #titlebar-content > #titlebar-buttonbox-container {
+  -moz-box-align: start;
+}
+
 %else
 /* On non-OSX, these should be start-aligned */
 #titlebar-buttonbox-container {

browser/components/nsBrowserContentHandler.js

@@ -715,12 +715,12 @@ nsDefaultCommandLineHandler.prototype = {
     // instances where users explicitly decide to "open with" the browser.
     // Note that users who launch firefox manually with the -url flag will
     // get erroneously counted.
-    if (cmdLine.findFlag("url", false) &&
-        ShellService.isDefaultBrowser(false, false)) {
-      try {
+    try {
+      if (cmdLine.findFlag("url", false) &&
+          ShellService.isDefaultBrowser(false, false)) {
         Services.telemetry.getHistogramById("FX_STARTUP_EXTERNAL_CONTENT_HANDLER").add();
-      } catch (e) {}
-    }
+      }
+    } catch (e) {}
 
     var urilist = [];
 

browser/components/shell/ShellService.jsm

@@ -101,7 +101,13 @@ XPCOMUtils.defineLazyServiceGetter(ShellServiceInternal, "shellService",
  */
 this.ShellService = new Proxy(ShellServiceInternal, {
   get(target, name) {
-    return name in target ? target[name] :
-                            target.shellService[name];
+    if (name in target) {
+      return target[name];
+    }
+    if (target.shellService) {
+      return target.shellService[name];
+    }
+    Services.console.logStringMessage(`${name} not found in ShellService: ${target.shellService}`);
+    return undefined;
   }
 });

browser/themes/osx/devedition.css

@@ -32,16 +32,6 @@
   margin-top: 6px;
 }
 
-/* In private windows, the #titlebar-content is higher because of the
- * private browsing indicator. With the margin-top the titlebar buttons
- * align to the top correctly in that case, but only if we don't stretch
- * the box they're in because the container is too high, so we override
- * the default alignment value (stretch).
- */
-#main-window[tabsintitlebar] > #titlebar > #titlebar-content > #titlebar-buttonbox-container {
-  -moz-box-align: start;
-}
-
 /* Square back and forward buttons.  Need !important on these because there
    are a lot of more specific selectors sprinkled around elsewhere for changing
    background / shadows for different states */

caps/BasePrincipal.h

@@ -11,6 +11,7 @@
 #include "nsIScriptSecurityManager.h"
 #include "nsJSPrincipals.h"
 
+#include "mozilla/Attributes.h"
 #include "mozilla/dom/ChromeUtilsBinding.h"
 
 class nsIContentSecurityPolicy;
@@ -45,11 +46,11 @@ public:
   // |!key1=value1&key2=value2|. If there are no non-default attributes, this
   // returns an empty string.
   void CreateSuffix(nsACString& aStr) const;
-  bool PopulateFromSuffix(const nsACString& aStr);
+  MOZ_MUST_USE bool PopulateFromSuffix(const nsACString& aStr);
 
   // Populates the attributes from a string like
   // |uri!key1=value1&key2=value2| and returns the uri without the suffix.
-  bool PopulateFromOrigin(const nsACString& aOrigin,
+  MOZ_MUST_USE bool PopulateFromOrigin(const nsACString& aOrigin,
                           nsACString& aOriginNoSuffix);
 
   // Helper function to match mIsPrivateBrowsing to existing private browsing

caps/nsJSPrincipals.cpp

@@ -139,7 +139,9 @@ ReadSuffixAndSpec(JSStructuredCloneReader* aReader,
         return false;
     }
 
-    aAttrs.PopulateFromSuffix(suffix);
+    if (!aAttrs.PopulateFromSuffix(suffix)) {
+        return false;
+    }
 
     aSpec.SetLength(specLength);
     if (!JS_ReadBytes(aReader, aSpec.BeginWriting(), specLength)) {

caps/tests/gtest/TestOriginAttributes.cpp

@@ -13,7 +13,8 @@ TestSuffix(const PrincipalOriginAttributes& attrs)
   attrs.CreateSuffix(suffix);
 
   PrincipalOriginAttributes attrsFromSuffix;
-  attrsFromSuffix.PopulateFromSuffix(suffix);
+  bool success = attrsFromSuffix.PopulateFromSuffix(suffix);
+  EXPECT_TRUE(success);
 
   EXPECT_EQ(attrs, attrsFromSuffix);
 }

devtools/client/shared/shim/Services.js

@@ -426,9 +426,13 @@ PrefBranch.prototype = {
    * Helper function to initialize the root PrefBranch.
    */
   _initializeRoot: function () {
-    if (localStorage.length === 0) {
-      // FIXME - this is where we'll load devtools.js to install the
-      // default prefs.
+    if (localStorage.length === 0 && Services._defaultPrefsEnabled) {
+      /* eslint-disable no-eval */
+      let devtools = require("raw!prefs!devtools/client/preferences/devtools");
+      eval(devtools);
+      let all = require("raw!prefs!modules/libpref/init/all");
+      eval(all);
+      /* eslint-enable no-eval */
     }
 
     // Read the prefs from local storage and create the local
@@ -449,12 +453,25 @@ PrefBranch.prototype = {
 };
 
 const Services = {
+  _prefs: null,
+
+  // For use by tests.  If set to false before Services.prefs is used,
+  // this will disable the reading of the default prefs.
+  _defaultPrefsEnabled: true,
+
   /**
    * An implementation of nsIPrefService that is based on local
    * storage.  Only the subset of nsIPrefService that is actually used
-   * by devtools is implemented here.
+   * by devtools is implemented here.  This is lazily instantiated so
+   * that the tests have a chance to disable the loading of default
+   * prefs.
    */
-  prefs: new PrefBranch(null, "", ""),
+  get prefs() {
+    if (!this._prefs) {
+      this._prefs = new PrefBranch(null, "", "");
+    }
+    return this._prefs;
+  },
 
   /**
    * An implementation of Services.appinfo that holds just the
@@ -582,7 +599,5 @@ function pref(name, value) {
 }
 
 module.exports = Services;
-// This is exported to silence eslint and, at some point, perhaps to
-// provide it when loading devtools.js in order to install the default
-// preferences.
+// This is exported to silence eslint.
 exports.pref = pref;

devtools/client/shared/shim/test/test_service_prefs.html

@@ -52,6 +52,9 @@ localStorage.setItem("Services.prefs:devtools.branch2.someint", JSON.stringify({
 "use strict";
 
 function do_tests() {
+  // We can't load the defaults in this context.
+  Services._defaultPrefsEnabled = false;
+
   is(Services.prefs.getBoolPref("devtools.branch1.somebool"), false,
     "bool pref value");
   Services.prefs.setBoolPref("devtools.branch1.somebool", true);

docshell/base/SerializedLoadContext.h

@@ -87,9 +87,7 @@ struct ParamTraits<SerializedLoadContext>
         !ReadParam(aMsg, aIter, &suffix)) {
       return false;
     }
-    aResult->mOriginAttributes.PopulateFromSuffix(suffix);
-
-    return true;
+    return aResult->mOriginAttributes.PopulateFromSuffix(suffix);
   }
 };
 

dom/ipc/ContentChild.cpp

@@ -2436,7 +2436,8 @@ ContentChild::RecvAddPermission(const IPC::Permission& permission)
   // the permission manager does that internally.
   nsAutoCString originNoSuffix;
   PrincipalOriginAttributes attrs;
-  attrs.PopulateFromOrigin(permission.origin, originNoSuffix);
+  bool success = attrs.PopulateFromOrigin(permission.origin, originNoSuffix);
+  NS_ENSURE_TRUE(success, false);
 
   nsCOMPtr<nsIURI> uri;
   nsresult rv = NS_NewURI(getter_AddRefs(uri), originNoSuffix);

dom/ipc/TabContext.cpp

@@ -369,7 +369,10 @@ MaybeInvalidTabContext::MaybeInvalidTabContext(const IPCTabContext& aParams)
       showAccelerators = ipcContext.showAccelerators();
       showFocusRings = ipcContext.showFocusRings();
       originSuffix = ipcContext.originSuffix();
-      originAttributes.PopulateFromSuffix(originSuffix);
+      if (!originAttributes.PopulateFromSuffix(originSuffix)) {
+        mInvalidReason = "Populate originAttributes from originSuffix failed.";
+        return;
+      }
       break;
     }
     case IPCTabContext::TUnsafeIPCTabContext: {

dom/storage/DOMStorageDBThread.cpp

@@ -55,7 +55,8 @@ Scheme0Scope(DOMStorageCacheBridge* aCache)
 
   PrincipalOriginAttributes oa;
   if (!suffix.IsEmpty()) {
-    oa.PopulateFromSuffix(suffix);
+    DebugOnly<bool> success = oa.PopulateFromSuffix(suffix);
+    MOZ_ASSERT(success);
   }
 
   if (oa.mAppId != nsIScriptSecurityManager::NO_APP_ID || oa.mInIsolatedMozBrowser) {
@@ -763,7 +764,8 @@ OriginAttrsPatternMatchSQLFunction::OnFunctionCall(
   NS_ENSURE_SUCCESS(rv, rv);
 
   PrincipalOriginAttributes oa;
-  oa.PopulateFromSuffix(suffix);
+  bool success = oa.PopulateFromSuffix(suffix);
+  NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
   bool result = mPattern.Matches(oa);
 
   RefPtr<nsVariant> outVar(new nsVariant());

gfx/2d/ScaledFontBase.cpp

@@ -235,6 +235,7 @@ ScaledFontBase::GetGlyphDesignMetrics(const uint16_t* aGlyphs, uint32_t aNumGlyp
         }
 #endif
       }
+      cairo_font_options_destroy(options);
     }
 
   }

image/DynamicImage.cpp

@@ -195,8 +195,10 @@ DynamicImage::GetFrameAtSize(const IntSize& aSize,
 }
 
 NS_IMETHODIMP_(bool)
-DynamicImage::WillDrawOpaqueNow()
+DynamicImage::IsOpaque()
 {
+  // XXX(seth): For performance reasons it'd be better to return true here, but
+  // I'm not sure how we can guarantee it for an arbitrary gfxDrawable.
   return false;
 }
 

image/ImageWrapper.cpp

@@ -185,9 +185,9 @@ ImageWrapper::GetFrameAtSize(const IntSize& aSize,
 }
 
 NS_IMETHODIMP_(bool)
-ImageWrapper::WillDrawOpaqueNow()
+ImageWrapper::IsOpaque()
 {
-  return mInnerImage->WillDrawOpaqueNow();
+  return mInnerImage->IsOpaque();
 }
 
 NS_IMETHODIMP_(bool)

image/OrientedImage.cpp

@@ -89,7 +89,7 @@ OrientedImage::GetFrame(uint32_t aWhichFrame,
 
   // Determine an appropriate format for the surface.
   gfx::SurfaceFormat surfaceFormat;
-  if (InnerImage()->WillDrawOpaqueNow()) {
+  if (InnerImage()->IsOpaque()) {
     surfaceFormat = gfx::SurfaceFormat::B8G8R8X8;
   } else {
     surfaceFormat = gfx::SurfaceFormat::B8G8R8A8;

image/RasterImage.cpp

@@ -377,7 +377,7 @@ RasterImage::LookupFrame(const IntSize& aSize,
   return Move(result.Surface());
 }
 
-bool
+NS_IMETHODIMP_(bool)
 RasterImage::IsOpaque()
 {
   if (mError) {
@@ -395,39 +395,6 @@ RasterImage::IsOpaque()
   return !(progress & FLAG_HAS_TRANSPARENCY);
 }
 
-NS_IMETHODIMP_(bool)
-RasterImage::WillDrawOpaqueNow()
-{
-  if (!IsOpaque()) {
-    return false;
-  }
-
-  if (mAnimationState) {
-    // We never discard frames of animated images.
-    return true;
-  }
-
-  // If we are not locked our decoded data could get discard at any time (ie
-  // between the call to this function and when we are asked to draw), so we
-  // have to return false if we are unlocked.
-  if (IsUnlocked()) {
-    return false;
-  }
-
-  LookupResult result =
-    SurfaceCache::LookupBestMatch(ImageKey(this),
-                                  RasterSurfaceKey(mSize,
-                                                   DefaultSurfaceFlags(),
-                                                   PlaybackType::eStatic));
-  MatchType matchType = result.Type();
-  if (matchType == MatchType::NOT_FOUND || matchType == MatchType::PENDING ||
-      !result.Surface()->IsFinished()) {
-    return false;
-  }
-
-  return true;
-}
-
 void
 RasterImage::OnSurfaceDiscarded()
 {

image/RasterImage.h

@@ -484,8 +484,6 @@ private: // data
   // Helpers
   bool CanDiscard();
 
-  bool IsOpaque();
-
 protected:
   explicit RasterImage(ImageURL* aURI = nullptr);
 

image/VectorImage.cpp

@@ -678,7 +678,7 @@ VectorImage::GetFirstFrameDelay()
 }
 
 NS_IMETHODIMP_(bool)
-VectorImage::WillDrawOpaqueNow()
+VectorImage::IsOpaque()
 {
   return false; // In general, SVG content is not opaque.
 }

image/VectorImage.h

@@ -49,7 +49,7 @@ public:
                                        nsresult aResult,
                                        bool aLastPart) override;
 
-  virtual void OnSurfaceDiscarded() override;
+  void OnSurfaceDiscarded() override;
 
   /**
    * Callback for SVGRootRenderingObserver.

image/imgIContainer.idl

@@ -255,12 +255,9 @@ interface imgIContainer : nsISupports
                                                            in uint32_t aFlags);
 
   /**
-   * Returns true if this image will draw opaquely right now if asked to draw
-   * with FLAG_HIGH_QUALITY_SCALING and otherwise default flags. If this image
-   * (when decoded) is opaque but no decoded frames are available then
-   * willDrawOpaqueNow will return false.
+   * Whether this image is opaque (i.e., needs a background painted behind it).
    */
-  [noscript, notxpcom] boolean willDrawOpaqueNow();
+  [notxpcom] boolean isOpaque();
 
   /**
    * @return true if getImageContainer() is expected to return a valid

layout/generic/nsImageFrame.cpp

@@ -1634,7 +1634,7 @@ nsDisplayImage::GetOpaqueRegion(nsDisplayListBuilder* aBuilder,
                                 bool* aSnap)
 {
   *aSnap = false;
-  if (mImage && mImage->WillDrawOpaqueNow()) {
+  if (mImage && mImage->IsOpaque()) {
     const nsRect frameContentBox = GetBounds(aSnap);
     return GetDestRect().Intersect(frameContentBox);
   }

layout/style/nsStyleStruct.cpp

@@ -2218,7 +2218,7 @@ nsStyleImage::IsOpaque() const
   MOZ_ASSERT(imageContainer, "IsComplete() said image container is ready");
 
   // Check if the crop region of the image is opaque.
-  if (imageContainer->WillDrawOpaqueNow()) {
+  if (imageContainer->IsOpaque()) {
     if (!mCropRect) {
       return true;
     }

media/webrtc/trunk/webrtc/modules/video_capture/android/java/src/org/webrtc/videoengine/VideoCaptureAndroid.java

@@ -106,6 +106,7 @@ public class VideoCaptureAndroid implements PreviewCallback, Callback, AppStateL
     if (camera != null) {
       mResumeCapture = true;
       stopCapture();
+      GeckoAppShell.notifyObservers("VideoCapture:Paused", null);
     }
   }
 
@@ -114,6 +115,7 @@ public class VideoCaptureAndroid implements PreviewCallback, Callback, AppStateL
     if (mResumeCapture) {
       startCapture(mCaptureWidth, mCaptureHeight, mCaptureMinFPS, mCaptureMaxFPS);
       mResumeCapture = false;
+      GeckoAppShell.notifyObservers("VideoCapture:Resumed", null);
     }
   }
 

mobile/android/app/mobile.js

@@ -472,7 +472,10 @@ pref("plugin.default.state", 1);
 // product URLs
 // The breakpad report server to link to in about:crashes
 pref("breakpad.reportURL", "https://crash-stats.mozilla.com/report/index/");
+
 pref("app.support.baseURL", "https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/");
+pref("app.supportURL", "https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/mobile-help");
+pref("app.faqURL", "https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/faq");
 
 // URL for feedback page
 // This should be kept in sync with the "feedback_link" string defined in strings.xml.in
@@ -489,8 +492,6 @@ pref("app.releaseNotesURL", "https://www.mozilla.com/%LOCALE%/mobile/%VERSION%be
 pref("app.releaseNotesURL", "https://www.mozilla.com/%LOCALE%/mobile/%VERSION%/releasenotes/");
 #endif
 
-pref("app.faqURL", "https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/faq");
-
 // Name of alternate about: page for certificate errors (when undefined, defaults to about:neterror)
 pref("security.alternate_certificate_error_page", "certerror");
 

mobile/android/base/strings.xml.in

@@ -85,8 +85,8 @@
 
   <string name="url_bar_default_text">&url_bar_default_text2;</string>
 
-  <!-- https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/ -->
-  <string name="help_link">https://support.mozilla.org/1/mobile/&formatS1;/&formatS2;/&formatS3;/</string>
+  <!-- https://support.mozilla.org/1/mobile/%VERSION%/%OS%/%LOCALE%/mobile-help -->
+  <string name="help_link">https://support.mozilla.org/1/mobile/&formatS1;/&formatS2;/&formatS3;/mobile-help</string>
   <string name="help_menu">&help_menu;</string>
 
   <string name="quit">&quit;</string>

mobile/android/chrome/content/WebrtcUI.js

@@ -46,6 +46,13 @@ var WebrtcUI = {
           this.notify();
           break;
       }
+    } else if (aTopic === "VideoCapture:Paused") {
+      if (this._notificationId) {
+        Notifications.cancel(this._notificationId);
+        this._notificationId = null;
+      }
+    } else if (aTopic === "VideoCapture:Resumed") {
+      this.notify();
     }
   },
 

mobile/android/chrome/content/about.js

@@ -45,7 +45,7 @@ function init() {
 
     let links = [
       {id: "releaseNotesURL", pref: "app.releaseNotesURL"},
-      {id: "supportURL",      pref: "app.support.baseURL"},
+      {id: "supportURL",      pref: "app.supportURL"},
       {id: "faqURL",          pref: "app.faqURL"},
       {id: "privacyURL",      pref: "app.privacyURL"},
       {id: "creditsURL",      pref: "app.creditsURL"},

mobile/android/chrome/content/browser.js

@@ -166,7 +166,9 @@ if (AppConstants.MOZ_WEBRTC) {
   lazilyLoadedObserverScripts.push(
     ["WebrtcUI", ["getUserMedia:request",
                   "PeerConnection:request",
-                  "recording-device-events"], "chrome://browser/content/WebrtcUI.js"])
+                  "recording-device-events",
+                  "VideoCapture:Paused",
+                  "VideoCapture:Resumed"], "chrome://browser/content/WebrtcUI.js"])
 }
 
 lazilyLoadedObserverScripts.forEach(function (aScript) {

netwerk/cookie/nsCookieService.cpp

@@ -7,6 +7,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
+#include "mozilla/unused.h"
 
 #include "mozilla/net/CookieServiceChild.h"
 #include "mozilla/net/NeckoCommon.h"
@@ -491,7 +492,8 @@ public:
 
       nsAutoCString suffix;
       row->GetUTF8String(IDX_ORIGIN_ATTRIBUTES, suffix);
-      tuple->key.mOriginAttributes.PopulateFromSuffix(suffix);
+      DebugOnly<bool> success = tuple->key.mOriginAttributes.PopulateFromSuffix(suffix);
+      MOZ_ASSERT(success);
 
       tuple->cookie =
         gCookieService->GetCookieFromRow(row, tuple->key.mOriginAttributes);
@@ -864,7 +866,8 @@ SetAppIdFromOriginAttributesSQLFunction::OnFunctionCall(
 
   rv = aFunctionArguments->GetUTF8String(0, suffix);
   NS_ENSURE_SUCCESS(rv, rv);
-  attrs.PopulateFromSuffix(suffix);
+  bool success = attrs.PopulateFromSuffix(suffix);
+  NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
 
   RefPtr<nsVariant> outVar(new nsVariant());
   rv = outVar->SetAsInt32(attrs.mAppId);
@@ -896,7 +899,8 @@ SetInBrowserFromOriginAttributesSQLFunction::OnFunctionCall(
 
   rv = aFunctionArguments->GetUTF8String(0, suffix);
   NS_ENSURE_SUCCESS(rv, rv);
-  attrs.PopulateFromSuffix(suffix);
+  bool success = attrs.PopulateFromSuffix(suffix);
+  NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
 
   RefPtr<nsVariant> outVar(new nsVariant());
   rv = outVar->SetAsInt32(attrs.mInIsolatedMozBrowser);
@@ -2798,7 +2802,9 @@ nsCookieService::EnsureReadComplete()
     nsAutoCString suffix;
     NeckoOriginAttributes attrs;
     stmt->GetUTF8String(IDX_ORIGIN_ATTRIBUTES, suffix);
-    attrs.PopulateFromSuffix(suffix);
+    // If PopulateFromSuffix failed we just ignore the OA attributes
+    // that we don't support
+    Unused << attrs.PopulateFromSuffix(suffix);
 
     nsCookieKey key(baseDomain, attrs);
     if (mDefaultDBState->readSet.GetEntry(key))

netwerk/protocol/http/PackagedAppVerifier.cpp

@@ -82,7 +82,8 @@ NS_IMETHODIMP PackagedAppVerifier::Init(nsIPackagedAppVerifierListener* aListene
   mIsFirstResource = true;
   mManifest = EmptyCString();
 
-  NeckoOriginAttributes().PopulateFromOrigin(aPackageOrigin, mPackageOrigin);
+  bool success = NeckoOriginAttributes().PopulateFromOrigin(aPackageOrigin, mPackageOrigin);
+  NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
   mBypassVerification = (mPackageOrigin ==
       Preferences::GetCString("network.http.signed-packages.trusted-origin"));
 

other-licenses/bsdiff/moz.build

@@ -21,6 +21,7 @@ if CONFIG['HOST_OS_ARCH'] == 'WINNT':
     HOST_OS_LIBS += [
         'ws2_32',
     ]
+    USE_STATIC_LIBS = True
 
 LOCAL_INCLUDES += [
     '/toolkit/mozapps/update/updater',

taskcluster/scripts/builder/build-l10n.sh

@@ -4,6 +4,8 @@ set -x -e
 
 echo "running as" $(id)
 
+. /home/worker/scripts/xvfb.sh
+
 ####
 # Taskcluster friendly wrapper for performing fx desktop l10n repacks via mozharness.
 # Based on ./build-linux.sh
@@ -43,37 +45,14 @@ if [[ -z ${MOZHARNESS_CONFIG} ]]; then fail "MOZHARNESS_CONFIG is not set"; fi
 
 cleanup() {
     local rv=$?
-    if [ -n "$xvfb_pid" ]; then
-        kill $xvfb_pid || true
-    fi
+    cleanup_xvfb
     exit $rv
 }
 trap cleanup EXIT INT
 
-# run mozharness in XVfb, if necessary; this is an array to maintain the quoting in the -s argument
+# run XVfb in the background, if necessary
 if $NEED_XVFB; then
-    # Some mozharness scripts set DISPLAY=:2
-    Xvfb :2 -screen 0 1024x768x24 &
-    export DISPLAY=:2
-    xvfb_pid=$!
-    # Only error code 255 matters, because it signifies that no
-    # display could be opened. As long as we can open the display
-    # tests should work. We'll retry a few times with a sleep before
-    # failing.
-    retry_count=0
-    max_retries=2
-    xvfb_test=0
-    until [ $retry_count -gt $max_retries ]; do
-        xvinfo || xvfb_test=$?
-        if [ $xvfb_test != 255 ]; then
-            retry_count=$(($max_retries + 1))
-        else
-            retry_count=$(($retry_count + 1))
-            echo "Failed to start Xvfb, retry: $retry_count"
-            sleep 2
-        fi
-    done
-    if [ $xvfb_test == 255 ]; then fail "xvfb did not start properly"; fi
+    start_xvfb '1024x768x24' 2
 fi
 
 # set up mozharness configuration, via command line, env, etc.

taskcluster/scripts/builder/build-linux.sh

@@ -4,6 +4,8 @@ set -x -e
 
 echo "running as" $(id)
 
+. /home/worker/scripts/xvfb.sh
+
 ####
 # Taskcluster friendly wrapper for performing fx desktop builds via mozharness.
 ####
@@ -54,37 +56,14 @@ if [[ -z ${MOZHARNESS_CONFIG} ]]; then fail "MOZHARNESS_CONFIG is not set"; fi
 
 cleanup() {
     local rv=$?
-    if [ -n "$xvfb_pid" ]; then
-        kill $xvfb_pid || true
-    fi
+    cleanup_xvfb
     exit $rv
 }
 trap cleanup EXIT INT
 
-# run mozharness in XVfb, if necessary; this is an array to maintain the quoting in the -s argument
+# run XVfb in the background, if necessary
 if $NEED_XVFB; then
-    # Some mozharness scripts set DISPLAY=:2
-    Xvfb :2 -screen 0 1024x768x24 &
-    export DISPLAY=:2
-    xvfb_pid=$!
-    # Only error code 255 matters, because it signifies that no
-    # display could be opened. As long as we can open the display
-    # tests should work. We'll retry a few times with a sleep before
-    # failing.
-    retry_count=0
-    max_retries=2
-    xvfb_test=0
-    until [ $retry_count -gt $max_retries ]; do
-        xvinfo || xvfb_test=$?
-        if [ $xvfb_test != 255 ]; then
-            retry_count=$(($max_retries + 1))
-        else
-            retry_count=$(($retry_count + 1))
-            echo "Failed to start Xvfb, retry: $retry_count"
-            sleep 2
-        fi
-    done
-    if [ $xvfb_test == 255 ]; then fail "xvfb did not start properly"; fi
+    start_xvfb '1024x768x24' 2
 fi
 
 # set up mozharness configuration, via command line, env, etc.

taskcluster/scripts/tester/test-ubuntu1204.sh

@@ -4,6 +4,8 @@ set -x -e
 
 echo "running as" $(id)
 
+. /home/worker/scripts/xvfb.sh
+
 ####
 # Taskcluster friendly wrapper for performing fx desktop tests via mozharness.
 ####
@@ -43,13 +45,7 @@ cleanup() {
       # To share X issues
       cp /home/worker/.xsession-errors ~/artifacts/public/xsession-errors.log
     fi
-    # When you call this script with START_VNC or TASKCLUSTER_INTERACTIVE
-    # we make sure we do not kill xvfb so you do not lose your connection
-    xvfb_pid=`pidof Xvfb`
-    if [ -n "$xvfb_pid" ] && [ $START_VNC == false ] && [ $TASKCLUSTER_INTERACTIVE == false ] ; then
-        kill $xvfb_pid || true
-        screen -XS xvfb quit || true
-    fi
+    cleanup_xvfb
     exit $rv
 }
 trap cleanup EXIT INT
@@ -73,29 +69,9 @@ if $NEED_PULSEAUDIO; then
     pactl load-module module-null-sink
 fi
 
-# run Xvfb in the background, if necessary
+# run XVfb in the background, if necessary
 if $NEED_XVFB; then
-    screen -dmS xvfb Xvfb :0 -nolisten tcp -screen 0 1600x1200x24 \
-       > ~/artifacts/public/xvfb.log 2>&1
-    export DISPLAY=:0
-    # Only error code 255 matters, because it signifies that no
-    # display could be opened. As long as we can open the display
-    # tests should work. We'll retry a few times with a sleep before
-    # failing.
-    retry_count=0
-    max_retries=2
-    xvfb_test=0
-    until [ $retry_count -gt $max_retries ]; do
-        xvinfo || xvfb_test=$?
-        if [ $xvfb_test != 255 ]; then
-            retry_count=$(($max_retries + 1))
-        else
-            retry_count=$(($retry_count + 1))
-            echo "Failed to start Xvfb, retry: $retry_count"
-            sleep 2
-        fi
-    done
-    if [ $xvfb_test == 255 ]; then fail "xvfb did not start properly"; fi
+    start_xvfb '1600x1200x24' 0
 fi
 
 if $START_VNC; then

taskcluster/scripts/tester/test-ubuntu1604.sh

@@ -4,6 +4,8 @@ set -x -e
 
 echo "running as" $(id)
 
+. /home/worker/scripts/xvfb.sh
+
 ####
 # Taskcluster friendly wrapper for performing fx desktop tests via mozharness.
 ####
@@ -43,13 +45,7 @@ cleanup() {
       # To share X issues
       cp /home/worker/.xsession-errors ~/artifacts/public/xsession-errors.log
     fi
-    # When you call this script with START_VNC or TASKCLUSTER_INTERACTIVE
-    # we make sure we do not kill xvfb so you do not lose your connection
-    xvfb_pid=`pidof Xvfb`
-    if [ -n "$xvfb_pid" ] && [ $START_VNC == false ] && [ $TASKCLUSTER_INTERACTIVE == false ] ; then
-        kill $xvfb_pid || true
-        screen -XS xvfb quit || true
-    fi
+    cleanup_xvfb
     exit $rv
 }
 trap cleanup EXIT INT
@@ -66,30 +62,9 @@ if ! [ -d mozharness ]; then
     fail "mozharness zip did not contain mozharness/"
 fi
 
-# run Xvfb in the background, if necessary
+# run XVfb in the background, if necessary
 if $NEED_XVFB; then
-    screen -dmS xvfb Xvfb :0 -nolisten tcp -screen 0 1600x1200x24 \
-       > ~/artifacts/public/xvfb.log 2>&1
-    export DISPLAY=:0
-
-    # Only error code 255 matters, because it signifies that no
-    # display could be opened. As long as we can open the display
-    # tests should work. We'll retry a few times with a sleep before
-    # failing.
-    retry_count=0
-    max_retries=2
-    xvfb_test=0
-    until [ $retry_count -gt $max_retries ]; do
-        xvinfo || xvfb_test=$?
-        if [ $xvfb_test != 255 ]; then
-            retry_count=$(($max_retries + 1))
-        else
-            retry_count=$(($retry_count + 1))
-            echo "Failed to start Xvfb, retry: $retry_count"
-            sleep 2
-        fi
-    done
-    if [ $xvfb_test == 255 ]; then fail "xvfb did not start properly"; fi
+    start_xvfb '1600x1200x24' 0
 fi
 
 if $START_VNC; then

testing/docker/desktop-build/Dockerfile

@@ -11,6 +11,10 @@ VOLUME /home/worker/tooltool-cache
 ADD             bin /home/worker/bin
 RUN             chmod +x /home/worker/bin/*
 
+# Add wrapper scripts for xvfb allowing tasks to easily retry starting up xvfb
+# %include testing/docker/recipes/xvfb.sh
+ADD topsrcdir/testing/docker/recipes/xvfb.sh /home/worker/scripts/xvfb.sh
+
 # Add configuration
 COPY            dot-config                    /home/worker/.config
 
@@ -33,5 +37,9 @@ ADD           buildprops.json /home/worker/
 RUN wget -O /builds/tooltool.py https://raw.githubusercontent.com/mozilla/build-tooltool/master/tooltool.py
 RUN chmod +x /builds/tooltool.py
 
+# Move installation to base centos6-build image once Bug 1272629 is fixed
+# Install the screen package here to use with xvfb.
+RUN yum install -y screen
+
 # Set a default command useful for debugging
 CMD ["/bin/bash", "--login"]

testing/docker/desktop-test/Dockerfile

@@ -10,6 +10,10 @@ ADD topsrcdir/testing/docker/recipes/tooltool.py /setup/tooltool.py
 # %include testing/docker/recipes/install-mercurial.sh
 ADD topsrcdir/testing/docker/recipes/install-mercurial.sh /tmp/install-mercurial.sh
 
+# Add wrapper scripts for xvfb allowing tasks to easily retry starting up xvfb
+# %include testing/docker/recipes/xvfb.sh
+ADD topsrcdir/testing/docker/recipes/xvfb.sh /home/worker/scripts/xvfb.sh
+
 # %include testing/docker/recipes/ubuntu1204-test-system-setup.sh
 ADD topsrcdir/testing/docker/recipes/ubuntu1204-test-system-setup.sh /setup/system-setup.sh
 RUN bash /setup/system-setup.sh

testing/docker/desktop1604-test/Dockerfile

@@ -14,6 +14,10 @@ ADD topsrcdir/testing/docker/recipes/install-mercurial.sh /setup/install-mercuri
 ADD topsrcdir/testing/docker/recipes/ubuntu1604-test-system-setup.sh /setup/system-setup.sh
 RUN           bash /setup/system-setup.sh
 
+# Add wrapper scripts for xvfb allowing tasks to easily retry starting up xvfb
+# %include testing/docker/recipes/xvfb.sh
+ADD topsrcdir/testing/docker/recipes/xvfb.sh /home/worker/scripts/xvfb.sh
+
 # %include testing/docker/recipes/run-task
 ADD topsrcdir/testing/docker/recipes/run-task /home/worker/bin/run-task
 

testing/docker/recipes/xvfb.sh

@@ -0,0 +1,75 @@
+#! /bin/bash -x
+
+set -x
+
+fail() {
+    echo # make sure error message is on a new line
+    echo "[xvfb.sh:error]" "${@}"
+    exit 1
+}
+
+cleanup_xvfb() {
+    # When you call this script with START_VNC or TASKCLUSTER_INTERACTIVE
+    # we make sure we do not kill xvfb so you do not lose your connection
+    local xvfb_pid=`pidof Xvfb`
+    local vnc=${START_VNC:-false}
+    local interactive=${TASKCLUSTER_INTERACTIVE:-false}
+    if [ -n "$xvfb_pid" ] && [[ $vnc == false ]] && [[ $interactive == false ]] ; then
+        kill $xvfb_pid || true
+        screen -XS xvfb quit || true
+    fi
+}
+
+# Attempt to start xvfb in a screen session with the given resolution and display
+# number.  Up to 5 attempts will be made to start xvfb with a short delay
+# between retries
+try_xvfb() {
+    screen -dmS xvfb Xvfb :$2 -nolisten tcp -screen 0 $1 \
+       > ~/artifacts/xvfb/xvfb.log 2>&1
+    export DISPLAY=:$2
+
+    # Only error code 255 matters, because it signifies that no
+    # display could be opened. As long as we can open the display
+    # tests should work. We'll retry a few times with a sleep before
+    # failing.
+    local retry_count=0
+    local max_retries=5
+    xvfb_test=0
+    until [ $retry_count -gt $max_retries ]; do
+        xvinfo || xvfb_test=$?
+        if [ $xvfb_test != 255 ]; then
+            retry_count=$(($max_retries + 1))
+        else
+            retry_count=$(($retry_count + 1))
+            echo "Failed to start Xvfb, retry: $retry_count"
+            sleep 2
+        fi
+    done
+    if [ $xvfb_test == 255 ]; then
+        return 1
+    else
+        return 0
+    fi
+}
+
+start_xvfb() {
+    set +e
+    mkdir -p ~/artifacts/xvfb
+    local retry_count=0
+    local max_retries=2
+    local success=1
+    until [ $retry_count -gt $max_retries ]; do
+        try_xvfb $1 $2
+        success=$?
+        if [ $success -eq 0 ]; then
+            retry_count=$(($max_retries + 1))
+        else
+            retry_count=$(($retry_count + 1))
+            sleep 10
+        fi
+    done
+    set -e
+    if [ $success -eq 1 ]; then
+        fail "Could not start xvfb after ${max_retries} attempts"
+    fi
+}

testing/mozharness/configs/unittests/mac_unittest.py

@@ -114,7 +114,9 @@ config = {
                 "--appname=%(binary_path)s",
                 "--utility-path=tests/bin",
                 "--extra-profile-file=tests/bin/plugins",
-                "--symbols-path=%(symbols_path)s"
+                "--symbols-path=%(symbols_path)s",
+                "--log-raw=%(raw_log_file)s",
+                "--log-errorsummary=%(error_summary_file)s",
                 "--cleanup-crashes",
             ],
             "run_filename": "runreftest.py",