Merge branch 'master' into release

36 files changed, 439 insertions, 424 deletions

diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp
index 3965d40f83..5ea9c9cea1 100644
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -5752,6 +5752,18 @@ inline uint8_t PoisonValue(uint8_t v)
   return v + rand() %3 -1;
 }
 
+static IntRect ClipImageDataTransfer(IntRect& aSrc,
+                                     const IntPoint& aDestOffset,
+                                     const IntSize& aDestBounds)
+{
+  IntRect dest = aSrc;
+  dest.SafeMoveBy(aDestOffset);
+  dest = IntRect(IntPoint(0, 0), aDestBounds).SafeIntersect(dest);
+
+  aSrc = aSrc.SafeIntersect(dest - aDestOffset);
+  return aSrc + aDestOffset;
+}
+
 nsresult
 CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx,
                                             int32_t aX,
@@ -5792,9 +5804,11 @@ CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx,
     return NS_OK;
   }
 
-  IntRect srcRect(0, 0, mWidth, mHeight);
-  IntRect destRect(aX, aY, aWidth, aHeight);
-  IntRect srcReadRect = srcRect.Intersect(destRect);
+  IntRect dstWriteRect(0, 0, aWidth, aHeight);
+  IntRect srcReadRect = ClipImageDataTransfer(dstWriteRect,
+                                              IntPoint(aX, aY),
+                                              IntSize(mWidth, mHeight));
+
   RefPtr<DataSourceSurface> readback;
   DataSourceSurface::MappedSurface rawData;
   if (!srcReadRect.IsEmpty()) {
@@ -5822,9 +5836,6 @@ CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx,
     }
   }
 
-  IntRect dstWriteRect = srcReadRect;
-  dstWriteRect.MoveBy(-aX, -aY);
-
   JS::AutoCheckCannotGC nogc;
   bool isShared;
   uint8_t* data = JS_GetUint8ClampedArrayData(darray, &isShared, nogc);
@@ -6025,10 +6036,10 @@ CanvasRenderingContext2D::PutImageData_explicit(int32_t aX, int32_t aY, uint32_t
     dirtyRect = imageDataRect;
   }
 
-  dirtyRect.MoveBy(IntPoint(aX, aY));
-  dirtyRect = IntRect(0, 0, mWidth, mHeight).Intersect(dirtyRect);
-
-  if (dirtyRect.Width() <= 0 || dirtyRect.Height() <= 0) {
+  IntRect srcRect = dirtyRect;
+  dirtyRect = ClipImageDataTransfer(srcRect, IntPoint(aX, aY),
+                                    IntSize(mWidth, mHeight));
+  if (dirtyRect.IsEmpty()) {
     return NS_OK;
   }
 
diff --git a/dom/events/DataTransfer.cpp b/dom/events/DataTransfer.cpp
index 5e7d477dfb..3a3f5464d2 100644
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -639,16 +639,11 @@ DataTransfer::PrincipalMaySetData(const nsAString& aType,
       return false;
     }
 
-    if (aType.EqualsASCII(kFileMime) ||
-        aType.EqualsASCII(kFilePromiseMime)) {
-      NS_WARNING("Disallowing adding x-moz-file or x-moz-file-promize types to DataTransfer");
-      return false;
-    }
-    
-    // Disallow content from creating x-moz-place flavors, so that it cannot
-    // create fake Places smart queries exposing user data.
-    if (StringBeginsWith(aType, NS_LITERAL_STRING("text/x-moz-place"))) {
-      NS_WARNING("Disallowing adding moz-place types to DataTransfer");
+    // Don't allow adding internal types of the form */x-moz-*, but
+    // special-case the url types as they are simple variations of urls.
+    if (FindInReadable(NS_LITERAL_STRING(kInternal_Mimetype_Prefix), aType) &&
+        !StringBeginsWith(aType, NS_LITERAL_STRING("text/x-moz-url"))) {
+      NS_WARNING("Disallowing adding requested internal type to DataTransfer");
       return false;
     }
   }
diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp
index 4671f89f88..221b464773 100644
--- a/dom/events/EventListenerManager.cpp
+++ b/dom/events/EventListenerManager.cpp
@@ -751,6 +751,8 @@ EventListenerManager::SetEventHandler(nsIAtom* aName,
                                       bool aPermitUntrustedEvents,
                                       Element* aElement)
 {
+  auto removeEventHandler = MakeScopeExit([&] { RemoveEventHandler(aName, EmptyString()); });
+
   nsCOMPtr<nsIDocument> doc;
   nsCOMPtr<nsIScriptGlobalObject> global =
     GetScriptGlobalAndDocument(getter_AddRefs(doc));
@@ -825,6 +827,8 @@ EventListenerManager::SetEventHandler(nsIAtom* aName,
   NS_ENSURE_TRUE(context, NS_ERROR_FAILURE);
   NS_ENSURE_STATE(global->GetGlobalJSObject());
 
+  removeEventHandler.release();
+
   Listener* listener = SetEventHandlerInternal(aName,
                                                EmptyString(),
                                                TypedEventHandler(),
diff --git a/dom/media/mediasource/MediaSourceDemuxer.cpp b/dom/media/mediasource/MediaSourceDemuxer.cpp
index 73950e1a87..616693720a 100644
--- a/dom/media/mediasource/MediaSourceDemuxer.cpp
+++ b/dom/media/mediasource/MediaSourceDemuxer.cpp
@@ -465,10 +465,12 @@ MediaSourceTrackDemuxer::DoGetSamples(int32_t aNumSamples)
   }
   RefPtr<SamplesHolder> samples = new SamplesHolder;
   samples->mSamples.AppendElement(sample);
-  if (mNextRandomAccessPoint.ToMicroseconds() <= sample->mTime) {
+  {
     MonitorAutoLock mon(mMonitor);
-    mNextRandomAccessPoint =
-      mManager->GetNextRandomAccessPoint(mType, MediaSourceDemuxer::EOS_FUZZ);
+    if (mNextRandomAccessPoint.ToMicroseconds() <= sample->mTime) {
+      mNextRandomAccessPoint =
+        mManager->GetNextRandomAccessPoint(mType, MediaSourceDemuxer::EOS_FUZZ);
+    }
   }
   return SamplesPromise::CreateAndResolve(samples, __func__);
 }
diff --git a/dom/media/wave/WaveDemuxer.cpp b/dom/media/wave/WaveDemuxer.cpp
index 387a756759..5243909dec 100644
--- a/dom/media/wave/WaveDemuxer.cpp
+++ b/dom/media/wave/WaveDemuxer.cpp
@@ -611,7 +611,8 @@ WAVTrackDemuxer::Read(uint8_t* aBuffer, int64_t aOffset, int32_t aSize)
 {
   const int64_t streamLen = StreamLength();
   if (mInfo && streamLen > 0) {
-    aSize = std::min<int64_t>(aSize, streamLen - aOffset);
+    int64_t max = streamLen > aOffset ? streamLen - aOffset : 0;
+    aSize = std::min<int64_t>(aSize, max);
   }
   uint32_t read = 0;
   const nsresult rv = mSource.ReadAt(aOffset,
diff --git a/dom/media/webm/WebMBufferedParser.cpp b/dom/media/webm/WebMBufferedParser.cpp
index 979308cf0c..c3f1266cca 100644
--- a/dom/media/webm/WebMBufferedParser.cpp
+++ b/dom/media/webm/WebMBufferedParser.cpp
@@ -425,6 +425,7 @@ void WebMBufferedState::NotifyDataArrived(const unsigned char* aBuffer, uint32_t
 }
 
 void WebMBufferedState::Reset() {
+  ReentrantMonitorAutoEnter mon(mReentrantMonitor);
   mRangeParsers.Clear();
   mTimeMapping.Clear();
 }
diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp
index 4b9736d79c..38db74b020 100644
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -730,12 +730,17 @@ PromiseWorkerProxy::Create(WorkerPrivate* aWorkerPrivate,
   RefPtr<PromiseWorkerProxy> proxy =
     new PromiseWorkerProxy(aWorkerPrivate, aWorkerPromise, aCb);
 
+  // Maintain a reference so that we have a valid object to clean up when
+  // removing the feature.
+  proxy.get()->AddRef();
+
   // We do this to make sure the worker thread won't shut down before the
   // promise is resolved/rejected on the worker thread.
   if (!proxy->AddRefObject()) {
     // Probably the worker is terminating. We cannot complete the operation
-    // and we have to release all the resources.
-    proxy->CleanProperties();
+    // and we have to release all the resources. CleanUp releases the extra
+    // ref, too
+    proxy->CleanUp();
     return nullptr;
   }
 
@@ -763,24 +768,6 @@ PromiseWorkerProxy::~PromiseWorkerProxy()
   MOZ_ASSERT(!mWorkerPrivate);
 }
 
-void
-PromiseWorkerProxy::CleanProperties()
-{
-#ifdef DEBUG
-  WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
-  MOZ_ASSERT(worker);
-  worker->AssertIsOnWorkerThread();
-#endif
-  // Ok to do this unprotected from Create().
-  // CleanUp() holds the lock before calling this.
-  mCleanedUp = true;
-  mWorkerPromise = nullptr;
-  mWorkerPrivate = nullptr;
-
-  // Clear the StructuredCloneHolderBase class.
-  Clear();
-}
-
 bool
 PromiseWorkerProxy::AddRefObject()
 {
@@ -881,8 +868,9 @@ PromiseWorkerProxy::CleanUp()
       return;
     }
 
-    MOZ_ASSERT(mWorkerPrivate);
-    mWorkerPrivate->AssertIsOnWorkerThread();
+    if (mWorkerPrivate) {
+      mWorkerPrivate->AssertIsOnWorkerThread();
+    }
 
     // Release the Promise and remove the PromiseWorkerProxy from the holders of
     // the worker thread since the Promise has been resolved/rejected or the
@@ -890,7 +878,12 @@ PromiseWorkerProxy::CleanUp()
     MOZ_ASSERT(mWorkerHolder);
     mWorkerHolder = nullptr;
 
-    CleanProperties();
+    mCleanedUp = true;
+    mWorkerPromise = nullptr;
+    mWorkerPrivate = nullptr;
+
+    // Clear the StructuredCloneHolderBase class.
+    Clear();
   }
   Release();
 }
diff --git a/dom/promise/PromiseWorkerProxy.h b/dom/promise/PromiseWorkerProxy.h
index 3b0e443a46..1c70528602 100644
--- a/dom/promise/PromiseWorkerProxy.h
+++ b/dom/promise/PromiseWorkerProxy.h
@@ -190,9 +190,6 @@ private:
 
   bool AddRefObject();
 
-  // If not called from Create(), be sure to hold Lock().
-  void CleanProperties();
-
   // Function pointer for calling Promise::{ResolveInternal,RejectInternal}.
   typedef void (Promise::*RunCallbackFunc)(JSContext*,
                                            JS::Handle<JS::Value>);
diff --git a/dom/storage/DOMStorageIPC.cpp b/dom/storage/DOMStorageIPC.cpp
index 442941a7a0..1cf3463459 100644
--- a/dom/storage/DOMStorageIPC.cpp
+++ b/dom/storage/DOMStorageIPC.cpp
@@ -213,8 +213,10 @@ DOMStorageDBChild::RecvObserve(const nsCString& aTopic,
                                const nsString& aOriginAttributesPattern,
                                const nsCString& aOriginScope)
 {
-  DOMStorageObserver::Self()->Notify(
-    aTopic.get(), aOriginAttributesPattern, aOriginScope);
+  DOMStorageObserver* observer = DOMStorageObserver::Self();
+  if (observer) {
+    observer->Notify(aTopic.get(), aOriginAttributesPattern, aOriginScope);
+  }
   return true;
 }
 
diff --git a/dom/storage/DOMStorageObserver.cpp b/dom/storage/DOMStorageObserver.cpp
index 295cee4ab0..9de18738da 100644
--- a/dom/storage/DOMStorageObserver.cpp
+++ b/dom/storage/DOMStorageObserver.cpp
@@ -90,6 +90,8 @@ DOMStorageObserver::Shutdown()
   if (!sSelf) {
     return NS_ERROR_NOT_INITIALIZED;
   }
+  
+  sSelf->mSinks.Clear();
 
   NS_RELEASE(sSelf);
   return NS_OK;
@@ -123,6 +125,10 @@ DOMStorageObserver::Observe(nsISupports* aSubject,
                             const char* aTopic,
                             const char16_t* aData)
 {
+  if (NS_WARN_IF(!sSelf)) {  // Shutdown took place, so bail.
+    return NS_OK;
+  }
+
   nsresult rv;
 
   // Start the thread that opens the database.
diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp
index 56381a0c8d..5aaac7cfae 100644
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -2232,6 +2232,10 @@ void ReportLoadError(ErrorResult& aRv, nsresult aLoadResult,
       aLoadResult = NS_ERROR_DOM_SECURITY_ERR;
       break;
 
+    case NS_ERROR_CORRUPTED_CONTENT:
+      aLoadResult = NS_ERROR_DOM_NETWORK_ERR;
+      break;
+
     default:
       // For lack of anything better, go ahead and throw a NetworkError here.
       // We don't want to throw a JS exception, because for toplevel script
diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
index 1d9459cd07..cf270703e0 100644
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -4646,9 +4646,14 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
   AssertIsOnWorkerThread();
   MOZ_ASSERT(mThread);
 
+  RefPtr<WorkerThread> thread;
   {
     MutexAutoLock lock(mMutex);
     mJSContext = aCx;
+    // mThread is set before we enter, and is never changed during DoRunLoop.
+    // copy to local so we don't trigger mutex analysis
+    MOZ_ASSERT(mThread);
+    thread = mThread;
 
     MOZ_ASSERT(mStatus == Pending);
     mStatus = Running;
@@ -4678,7 +4683,7 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
 
       while (mControlQueue.IsEmpty() &&
              !(debuggerRunnablesPending = !mDebuggerQueue.IsEmpty()) &&
-             !(normalRunnablesPending = NS_HasPendingEvents(mThread))) {
+             !(normalRunnablesPending = NS_HasPendingEvents(thread))) {
         WaitForWorkerEvents();
       }
 
@@ -4688,7 +4693,7 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
         // irrelevant
 
         // The state of the world may have changed, recheck it.
-        normalRunnablesPending = NS_HasPendingEvents(mThread);
+        normalRunnablesPending = NS_HasPendingEvents(thread);
         // The debugger queue doesn't get cleared, so we can ignore that.
       }
 
@@ -4781,9 +4786,9 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
       }
     } else if (normalRunnablesPending) {
       // Process a single runnable from the main queue.
-      NS_ProcessNextEvent(mThread, false);
+      NS_ProcessNextEvent(thread, false);
 
-      normalRunnablesPending = NS_HasPendingEvents(mThread);
+      normalRunnablesPending = NS_HasPendingEvents(thread);
       if (normalRunnablesPending && GlobalScope()) {
         // Now *might* be a good time to GC. Let the JS engine make the decision.
         JSAutoCompartment ac(aCx, GlobalScope()->GetGlobalJSObject());
@@ -5185,10 +5190,14 @@ WorkerPrivate::ClearMainEventQueue(WorkerRanOrNot aRanOrNot)
   mCancelAllPendingRunnables = true;
 
   if (WorkerNeverRan == aRanOrNot) {
-    for (uint32_t count = mPreStartRunnables.Length(), index = 0;
-         index < count;
+    nsTArray<RefPtr<WorkerRunnable>> prestart;
+    {
+      MutexAutoLock lock(mMutex);
+      mPreStartRunnables.SwapElements(prestart);
+    }
+    for (uint32_t count = prestart.Length(), index = 0; index < count;
          index++) {
-      RefPtr<WorkerRunnable> runnable = mPreStartRunnables[index].forget();
+      RefPtr<WorkerRunnable> runnable = prestart[index].forget();
       static_cast<nsIRunnable*>(runnable.get())->Run();
     }
   } else {
@@ -5434,20 +5443,20 @@ WorkerPrivate::CreateNewSyncLoop(Status aFailStatus)
 {
   AssertIsOnWorkerThread();
 
+  nsCOMPtr<nsIEventTarget> realEventTarget;
   {
     MutexAutoLock lock(mMutex);
 
     if (mStatus >= aFailStatus) {
       return nullptr;
     }
+    
+    nsCOMPtr<nsIThreadInternal> thread = do_QueryInterface(NS_GetCurrentThread());
+    MOZ_ASSERT(thread);
+    
+    MOZ_ALWAYS_SUCCEEDS(thread->PushEventQueue(getter_AddRefs(realEventTarget)));
   }
 
-  nsCOMPtr<nsIThreadInternal> thread = do_QueryInterface(NS_GetCurrentThread());
-  MOZ_ASSERT(thread);
-
-  nsCOMPtr<nsIEventTarget> realEventTarget;
-  MOZ_ALWAYS_SUCCEEDS(thread->PushEventQueue(getter_AddRefs(realEventTarget)));
-
   RefPtr<EventTarget> workerEventTarget =
     new EventTarget(this, realEventTarget);
 
@@ -5469,9 +5478,18 @@ WorkerPrivate::RunCurrentSyncLoop()
 {
   AssertIsOnWorkerThread();
 
+  RefPtr<WorkerThread> thread;
   JSContext* cx = GetJSContext();
   MOZ_ASSERT(cx);
 
+  // mThread is set before we enter, and is never changed during
+  // RunCurrentSyncLoop.
+  {
+    MutexAutoLock lock(mMutex);
+    // Copy to local
+    thread = mThread;
+  }
+
   // This should not change between now and the time we finish running this sync
   // loop.
   uint32_t currentLoopIndex = mSyncLoopStack.Length() - 1;
@@ -5490,7 +5508,7 @@ WorkerPrivate::RunCurrentSyncLoop()
     bool normalRunnablesPending = false;
 
     // Don't block with the periodic GC timer running.
-    if (!NS_HasPendingEvents(mThread)) {
+    if (!NS_HasPendingEvents(thread)) {
       SetGCTimerMode(IdleTimer);
     }
 
@@ -5501,7 +5519,7 @@ WorkerPrivate::RunCurrentSyncLoop()
       for (;;) {
         while (mControlQueue.IsEmpty() &&
                !normalRunnablesPending &&
-               !(normalRunnablesPending = NS_HasPendingEvents(mThread))) {
+               !(normalRunnablesPending = NS_HasPendingEvents(thread))) {
           WaitForWorkerEvents();
         }
 
@@ -5510,7 +5528,7 @@ WorkerPrivate::RunCurrentSyncLoop()
           // XXXkhuey how should we handle Abort here? See Bug 1003730.
 
           // The state of the world may have changed. Recheck it.
-          normalRunnablesPending = NS_HasPendingEvents(mThread);
+          normalRunnablesPending = NS_HasPendingEvents(thread);
 
           // NB: If we processed a NotifyRunnable, we might have run
           // non-control runnables, one of which may have shut down the
@@ -5533,7 +5551,7 @@ WorkerPrivate::RunCurrentSyncLoop()
       // Make sure the periodic timer is running before we continue.
       SetGCTimerMode(PeriodicTimer);
 
-      MOZ_ALWAYS_TRUE(NS_ProcessNextEvent(mThread, false));
+      MOZ_ALWAYS_TRUE(NS_ProcessNextEvent(thread, false));
 
       // Now *might* be a good time to GC. Let the JS engine make the decision.
       if (JS::CurrentGlobalOrNull(cx)) {
@@ -5542,7 +5560,7 @@ WorkerPrivate::RunCurrentSyncLoop()
     }
   }
 
-  // Make sure that the stack didn't change underneath us.
+  // Make sure that the stack didn't change under us.
   MOZ_ASSERT(mSyncLoopStack[currentLoopIndex] == loopInfo);
 
   return DestroySyncLoop(currentLoopIndex);
diff --git a/gfx/2d/BaseRect.h b/gfx/2d/BaseRect.h
index b1eed9ddb9..a4e5e4c1d0 100644
--- a/gfx/2d/BaseRect.h
+++ b/gfx/2d/BaseRect.h
@@ -107,6 +107,10 @@ struct BaseRect {
   // (including edges) of *this and aRect. If there are no points in that
   // intersection, returns an empty rectangle with x/y set to the std::max of the x/y
   // of *this and aRect.
+  //
+  // Intersection with an empty Rect may not produce an empty Rect if overflow
+  // occurs. e.g. {INT_MIN, 0, 0, 20} Intersect { 5000, 0, 500, 20 } gives:
+  // the non-emtpy {5000, 0, 500, 20 } instead of {5000, 0, 0, 0}
   MOZ_MUST_USE Sub Intersect(const Sub& aRect) const
   {
     Sub result;
@@ -119,6 +123,26 @@ struct BaseRect {
     }
     return result;
   }
+  // Gives the same results as Intersect() but handles integer overflow
+  // better. This comes at a tiny cost in performance.
+  // e.g. {INT_MIN, 0, 0, 20} Intersect { 5000, 0, 500, 20 } gives:
+  // {5000, 0, 0, 0}
+  MOZ_MUST_USE Sub SafeIntersect(const Sub& aRect) const
+  {
+    Sub result;
+    result.x = std::max<T>(x, aRect.x);
+    result.y = std::max<T>(y, aRect.y);
+    T right = std::min<T>(x + width, aRect.x + aRect.width);
+    T bottom = std::min<T>(y + height, aRect.y + aRect.height);
+    if (right < result.x || bottom < result.y) {
+      result.width = 0;
+      result.height = 0;
+    } else {
+      result.width = right - result.x;
+      result.height = bottom - result.y;
+    }
+    return result;
+  }
   // Sets *this to be the rectangle containing the intersection of the points
   // (including edges) of *this and aRect. If there are no points in that
   // intersection, sets *this to be an empty rectangle with x/y set to the std::max
@@ -211,6 +235,40 @@ struct BaseRect {
   void MoveTo(const Point& aPoint) { x = aPoint.x; y = aPoint.y; }
   void MoveBy(T aDx, T aDy) { x += aDx; y += aDy; }
   void MoveBy(const Point& aPoint) { x += aPoint.x; y += aPoint.y; }
+
+  // Variant of MoveBy that ensures that even after translation by a point that
+  // the rectangle coordinates will still fit within numeric limits. The origin
+  // and size will be clipped within numeric limits to ensure this.
+  void SafeMoveByX(T aDx) {
+    T x2 = XMost();
+    if (aDx >= T(0)) {
+      T limit = std::numeric_limits<T>::max();
+      x = limit - aDx < x ? limit : x + aDx;
+      width = (limit - aDx < x2 ? limit : x2 + aDx) - x;
+    } else {
+      T limit = std::numeric_limits<T>::min();
+      x = limit - aDx > x ? limit : x + aDx;
+      width = (limit - aDx > x2 ? limit : x2 + aDx) - x;
+    }
+  }
+  void SafeMoveByY(T aDy) {
+    T y2 = YMost();
+    if (aDy >= T(0)) {
+      T limit = std::numeric_limits<T>::max();
+      y = limit - aDy < y ? limit : y + aDy;
+      height = (limit - aDy < y2 ? limit : y2 + aDy) - y;
+    } else {
+      T limit = std::numeric_limits<T>::min();
+      y = limit - aDy > y ? limit : y + aDy;
+      height = (limit - aDy > y2 ? limit : y2 + aDy) - y;
+    }
+  }
+  void SafeMoveBy(T aDx, T aDy) {
+    SafeMoveByX(aDx);
+    SafeMoveByY(aDy);
+  }
+  void SafeMoveBy(const Point& aPoint) { SafeMoveBy(aPoint.x, aPoint.y); }
+
   void SizeTo(T aWidth, T aHeight) { width = aWidth; height = aHeight; }
   void SizeTo(const SizeT& aSize) { width = aSize.width; height = aSize.height; }
 
diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp
index 9a0846bce2..88cef68bf9 100644
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -123,7 +123,7 @@ AllowedImageSize(int32_t aWidth, int32_t aHeight)
     return false;
   }
 
-  // check to make sure we don't overflow a 32-bit
+  // check to make sure we don't overflow 32-bit size for RGBA
   CheckedInt32 requiredBytes = CheckedInt32(aWidth) * CheckedInt32(aHeight) * 4;
   if (MOZ_UNLIKELY(!requiredBytes.isValid())) {
     NS_WARNING("width or height too large");
@@ -198,6 +198,7 @@ imgFrame::InitForDecoder(const nsIntSize& aImageSize,
   // warn for properties related to bad content.
   if (!AllowedImageAndFrameDimensions(aImageSize, aRect)) {
     NS_WARNING("Should have legal image size");
+    MonitorAutoLock lock(mMonitor);
     mAborted = true;
     return NS_ERROR_FAILURE;
   }
@@ -287,6 +288,7 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
   // warn for properties related to bad content.
   if (!AllowedImageSize(aSize.width, aSize.height)) {
     NS_WARNING("Should have legal image size");
+    MonitorAutoLock lock(mMonitor);
     mAborted = true;
     return NS_ERROR_FAILURE;
   }
@@ -303,6 +305,7 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
     gfxPlatform::GetPlatform()->CanRenderContentToDataSurface();
 
   if (canUseDataSurface) {
+    MonitorAutoLock lock(mMonitor);
     // It's safe to use data surfaces for content on this platform, so we can
     // get away with using volatile buffers.
     MOZ_ASSERT(!mImageSurface, "Called imgFrame::InitWithDrawable() twice?");
@@ -344,7 +347,12 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
     // surface instead.  This means if someone later calls RawAccessRef(), we
     // may have to do an expensive readback, but we warned callers about that in
     // the documentation for this method.
-    MOZ_ASSERT(!mOptSurface, "Called imgFrame::InitWithDrawable() twice?");
+#ifdef DEBUG
+    {
+      MonitorAutoLock lock(mMonitor);
+      MOZ_ASSERT(!mOptSurface, "Called imgFrame::InitWithDrawable() twice?");
+    }
+#endif
 
     if (gfxPlatform::GetPlatform()->SupportsAzureContentForType(aBackend)) {
       target = gfxPlatform::GetPlatform()->
@@ -356,6 +364,7 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
   }
 
   if (!target || !target->IsValid()) {
+    MonitorAutoLock lock(mMonitor);
     mAborted = true;
     return NS_ERROR_OUT_OF_MEMORY;
   }
@@ -367,6 +376,7 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
                              ImageRegion::Create(ThebesRect(mFrameRect)),
                              mFormat, aSamplingFilter, aImageFlags);
 
+  MonitorAutoLock lock(mMonitor);
   if (canUseDataSurface && !mImageSurface) {
     NS_WARNING("Failed to create VolatileDataSourceSurface");
     mAborted = true;
@@ -383,10 +393,7 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
   mDecoded = GetRect();
   mFinished = true;
 
-#ifdef DEBUG
-  MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(AreAllPixelsWritten());
-#endif
 
   return NS_OK;
 }
diff --git a/image/imgRequest.cpp b/image/imgRequest.cpp
index ba99779d30..2a8af98cf4 100644
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -96,6 +96,8 @@ imgRequest::Init(nsIURI *aURI,
                  ReferrerPolicy aReferrerPolicy)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Cannot use nsIURI off main thread!");
+  // Init() can only be called once, and that's before it can be used off
+  // mainthread
 
   LOG_FUNC(gImgLog, "imgRequest::Init");
 
@@ -800,7 +802,14 @@ imgRequest::OnStopRequest(nsIRequest* aRequest,
 
   RefPtr<imgRequest> strongThis = this;
 
-  if (mIsMultiPartChannel && mNewPartPending) {
+  bool isMultipart = false;
+  bool newPartPending = false;
+  {
+    MutexAutoLock lock(mMutex);
+    isMultipart = mIsMultiPartChannel;
+    newPartPending = mNewPartPending;
+  }
+  if (isMultipart && newPartPending) {
     OnDataAvailable(aRequest, ctxt, nullptr, 0, 0);
   }
 
diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp
index c1011b1053..ff5fc3070d 100644
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -930,7 +930,6 @@ nsresult
 nsSocketTransport::InitWithConnectedSocket(PRFileDesc *fd, const NetAddr *addr)
 {
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "wrong thread");
-    NS_ASSERTION(!mFD.IsInitialized(), "already initialized");
 
     char buf[kNetAddrMaxCStrBufSize];
     NetAddrToString(addr, buf, sizeof(buf));
@@ -956,6 +955,7 @@ nsSocketTransport::InitWithConnectedSocket(PRFileDesc *fd, const NetAddr *addr)
     {
         MutexAutoLock lock(mLock);
 
+        NS_ASSERTION(!mFD.IsInitialized(), "already initialized");
         mFD = fd;
         mFDref = 1;
         mFDconnected = 1;
@@ -1319,11 +1319,14 @@ nsSocketTransport::InitiateSocket()
     //
     // if we already have a connected socket, then just attach and return.
     //
-    if (mFD.IsInitialized()) {
+    {
+      MutexAutoLock lock(mLock);
+      if (mFD.IsInitialized()) {
         rv = mSocketTransportService->AttachSocket(mFD, this);
         if (NS_SUCCEEDED(rv))
-            mAttached = true;
+          mAttached = true;
         return rv;
+      }
     }
 
     //
@@ -1389,18 +1392,18 @@ nsSocketTransport::InitiateSocket()
     PR_SetSocketOption(fd, &opt);
 #endif
 
-    // inform socket transport about this newly created socket...
-    rv = mSocketTransportService->AttachSocket(fd, this);
-    if (NS_FAILED(rv)) {
-        CloseSocket(fd);
-        return rv;
-    }
-    mAttached = true;
-
     // assign mFD so that we can properly handle OnSocketDetached before we've
     // established a connection.
     {
         MutexAutoLock lock(mLock);
+        // inform socket transport about this newly created socket...
+        rv = mSocketTransportService->AttachSocket(fd, this);
+        if (NS_FAILED(rv)) {
+            CloseSocket(fd);
+            return rv;
+        }
+        mAttached = true;
+
         mFD = fd;
         mFDref = 1;
         mFDconnected = false;
@@ -1545,8 +1548,12 @@ nsSocketTransport::RecoverFromError()
 
     nsresult rv;
 
-    // OK to check this outside mLock
-    NS_ASSERTION(!mFDconnected, "socket should not be connected");
+#ifdef DEBUG
+    {
+      MutexAutoLock lock(mLock);
+      NS_ASSERTION(!mFDconnected, "socket should not be connected");
+    }
+#endif
 
     // all connection failures need to be reported to DNS so that the next
     // time we will use a different address if available.
diff --git a/netwerk/base/nsSocketTransport2.h b/netwerk/base/nsSocketTransport2.h
index cb107e6e79..89b75efa57 100644
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@@ -350,11 +350,13 @@ private:
 
     void OnMsgInputPending()
     {
+        MOZ_ASSERT(OnSocketThread(), "not on socket thread");
         if (mState == STATE_TRANSFERRING)
             mPollFlags |= (PR_POLL_READ | PR_POLL_EXCEPT);
     }
     void OnMsgOutputPending()
     {
+        MOZ_ASSERT(OnSocketThread(), "not on socket thread");
         if (mState == STATE_TRANSFERRING)
             mPollFlags |= (PR_POLL_WRITE | PR_POLL_EXCEPT);
     }
diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
index 34bf9e58f3..70d051f747 100644
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -2630,6 +2630,7 @@ nsHttpConnectionMgr::OnMsgCompleteUpgrade(int32_t, ARefBase *param)
 void
 nsHttpConnectionMgr::OnMsgUpdateParam(int32_t inParam, ARefBase *)
 {
+    MOZ_ASSERT(OnSocketThread(), "not on socket thread");
     uint32_t param = static_cast<uint32_t>(inParam);
     uint16_t name  = ((param) & 0xFFFF0000) >> 16;
     uint16_t value =  param & 0x0000FFFF;
@@ -2701,9 +2702,18 @@ nsHttpConnectionMgr::ActivateTimeoutTick()
             NS_WARNING("failed to create timer for http timeout management");
             return;
         }
+        ReentrantMonitorAutoEnter mon(mReentrantMonitor);
+        if (!mSocketThreadTarget) {
+            NS_WARNING("HTTP Connection Manager: Cannot activate timout if not initialized or shutdown");
+          return;
+        }
         mTimeoutTick->SetTarget(mSocketThreadTarget);
     }
-
+    if (mIsShuttingDown) {  // Atomic
+        // don't set a timer to generate an event if we're shutting down
+        // (mSocketThreadTarget might be null or garbage anyway if we're shutting down)
+        return;
+    }
     MOZ_ASSERT(!mTimeoutTickArmed, "timer tick armed");
     mTimeoutTickArmed = true;
     mTimeoutTick->Init(this, 1000, nsITimer::TYPE_REPEATING_SLACK);
diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.h b/netwerk/protocol/http/nsHttpConnectionMgr.h
index b94761a018..10df91a75a 100644
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -483,8 +483,15 @@ private:
     //-------------------------------------------------------------------------
 
     ReentrantMonitor    mReentrantMonitor;
+    // This is used as a flag that we're shut down, and no new events should be
+    // dispatched.
     nsCOMPtr<nsIEventTarget>     mSocketThreadTarget;
 
+    Atomic<bool, mozilla::Relaxed> mIsShuttingDown;
+
+    //-------------------------------------------------------------------------
+    // NOTE: these members are only accessed on the socket transport thread
+    //-------------------------------------------------------------------------
     // connection limits
     uint16_t mMaxConns;
     uint16_t mMaxPersistConnsPerHost;
@@ -492,11 +499,6 @@ private:
     uint16_t mMaxRequestDelay; // in seconds
     uint16_t mMaxPipelinedRequests;
     uint16_t mMaxOptimisticPipelinedRequests;
-    Atomic<bool, mozilla::Relaxed> mIsShuttingDown;
-
-    //-------------------------------------------------------------------------
-    // NOTE: these members are only accessed on the socket transport thread
-    //-------------------------------------------------------------------------
 
     bool     ProcessPendingQForEntry(nsConnectionEntry *, bool considerAll);
     bool     IsUnderPressure(nsConnectionEntry *ent,
diff --git a/netwerk/protocol/websocket/WebSocketChannel.cpp b/netwerk/protocol/websocket/WebSocketChannel.cpp
index 85a822806b..f94c1d9ca9 100644
--- a/netwerk/protocol/websocket/WebSocketChannel.cpp
+++ b/netwerk/protocol/websocket/WebSocketChannel.cpp
@@ -429,23 +429,46 @@ public:
       }
     }
 
-    if (aChannel->mConnecting) {
-      MOZ_ASSERT(NS_IsMainThread(), "not main thread");
-
-      // Only way a connecting channel may get here w/o failing is if it was
-      // closed with GOING_AWAY (1001) because of navigation, tab close, etc.
-      MOZ_ASSERT(NS_FAILED(aReason) ||
-                 aChannel->mScriptCloseCode == CLOSE_GOING_AWAY,
-                 "websocket closed while connecting w/o failing?");
-
-      sManager->RemoveFromQueue(aChannel);
-
-      bool wasNotQueued = (aChannel->mConnecting != CONNECTING_QUEUED);
-      LOG(("Websocket: changing state to NOT_CONNECTING"));
-      aChannel->mConnecting = NOT_CONNECTING;
-      if (wasNotQueued) {
-        sManager->ConnectNext(aChannel->mAddress);
-      }
+    if (NS_IsMainThread()) {
+      ContinueOnStopSession(aChannel, aReason);
+    } else {
+      RefPtr<WebSocketChannel> threadChannel = aChannel;
+      NS_DispatchToMainThread(NS_NewRunnableFunction(
+          [channel = threadChannel, reason = aReason]() {
+            StaticMutexAutoLock lock(sLock);
+            if (!sManager) {
+              return;
+            }
+
+            nsWSAdmissionManager::ContinueOnStopSession(channel, reason);
+          }));
+    }
+  }
+
+  static void ContinueOnStopSession(WebSocketChannel* aChannel,
+                                    nsresult aReason) {
+    sLock.AssertCurrentThreadOwns();
+    MOZ_ASSERT(NS_IsMainThread(), "not main thread");
+
+    if (!aChannel->mConnecting) {
+      return;
+    }
+
+    // Only way a connecting channel may get here w/o failing is if it
+    // was closed with GOING_AWAY (1001) because of navigation, tab
+    // close, etc.
+    MOZ_ASSERT(
+        NS_FAILED(aReason) || aChannel->mScriptCloseCode == CLOSE_GOING_AWAY,
+        "websocket closed while connecting w/o failing?");
+    Unused << aReason;
+
+    sManager->RemoveFromQueue(aChannel);
+
+    bool wasNotQueued = (aChannel->mConnecting != CONNECTING_QUEUED);
+    LOG(("Websocket: changing state to NOT_CONNECTING"));
+    aChannel->mConnecting = NOT_CONNECTING;
+    if (wasNotQueued) {
+      sManager->ConnectNext(aChannel->mAddress);
     }
   }
 
diff --git a/security/nss/cmd/smimetools/cmsutil.c b/security/nss/cmd/smimetools/cmsutil.c
index 9106d9955b..4343695ede 100644
--- a/security/nss/cmd/smimetools/cmsutil.c
+++ b/security/nss/cmd/smimetools/cmsutil.c
@@ -219,7 +219,7 @@ decode(FILE *out, SECItem *input, const struct decodeOptionsStr *decodeOptions)
         switch (typetag) {
             case SEC_OID_PKCS7_SIGNED_DATA: {
                 NSSCMSSignedData *sigd = NULL;
-                SECItem **digests;
+                SECItem **digests = NULL;
                 int nsigners;
                 int j;
 
diff --git a/security/nss/coreconf/coreconf.dep b/security/nss/coreconf/coreconf.dep
index 5182f75552..590d1bfaee 100644
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -10,3 +10,4 @@
  */
 
 #error "Do not include this header file."
+
diff --git a/security/nss/lib/nss/nss.h b/security/nss/lib/nss/nss.h
index f5886189ad..609a69f2ef 100644
--- a/security/nss/lib/nss/nss.h
+++ b/security/nss/lib/nss/nss.h
@@ -22,10 +22,10 @@
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define NSS_VERSION "3.52.5" _NSS_CUSTOMIZED
+#define NSS_VERSION "3.52.6" _NSS_CUSTOMIZED
 #define NSS_VMAJOR 3
 #define NSS_VMINOR 52
-#define NSS_VPATCH 5
+#define NSS_VPATCH 6
 #define NSS_VBUILD 0
 #define NSS_BETA PR_FALSE
 
diff --git a/security/nss/lib/smime/cmsdigest.c b/security/nss/lib/smime/cmsdigest.c
index bd14740682..1eb88f0b63 100644
--- a/security/nss/lib/smime/cmsdigest.c
+++ b/security/nss/lib/smime/cmsdigest.c
@@ -239,7 +239,7 @@ NSS_CMSDigestContext_FinishSingle(NSSCMSDigestContext *cmsdigcx,
                                   SECItem *digest)
 {
     SECStatus rv = SECFailure;
-    SECItem **dp;
+    SECItem **dp = NULL;
     PLArenaPool *arena = NULL;
 
     if ((arena = PORT_NewArena(1024)) == NULL)
@@ -247,7 +247,7 @@ NSS_CMSDigestContext_FinishSingle(NSSCMSDigestContext *cmsdigcx,
 
     /* get the digests into arena, then copy the first digest into poolp */
     rv = NSS_CMSDigestContext_FinishMultiple(cmsdigcx, arena, &dp);
-    if (rv == SECSuccess) {
+    if (rv == SECSuccess && dp) {
         /* now copy it into poolp */
         rv = SECITEM_CopyItem(poolp, digest, dp[0]);
     }
diff --git a/security/nss/lib/softoken/softkver.h b/security/nss/lib/softoken/softkver.h
index 93fc93ff35..e458f77ccc 100644
--- a/security/nss/lib/softoken/softkver.h
+++ b/security/nss/lib/softoken/softkver.h
@@ -17,10 +17,10 @@
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define SOFTOKEN_VERSION "3.52.5" SOFTOKEN_ECC_STRING
+#define SOFTOKEN_VERSION "3.52.6" SOFTOKEN_ECC_STRING
 #define SOFTOKEN_VMAJOR 3
 #define SOFTOKEN_VMINOR 52
-#define SOFTOKEN_VPATCH 5
+#define SOFTOKEN_VPATCH 6
 #define SOFTOKEN_VBUILD 0
 #define SOFTOKEN_BETA PR_FALSE
 
diff --git a/security/nss/lib/util/nssutil.h b/security/nss/lib/util/nssutil.h
index a94b7e6e9b..c8ff5eac2c 100644
--- a/security/nss/lib/util/nssutil.h
+++ b/security/nss/lib/util/nssutil.h
@@ -19,10 +19,10 @@
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <Beta>]"
  */
-#define NSSUTIL_VERSION "3.52.5"
+#define NSSUTIL_VERSION "3.52.6"
 #define NSSUTIL_VMAJOR 3
 #define NSSUTIL_VMINOR 52
-#define NSSUTIL_VPATCH 5
+#define NSSUTIL_VPATCH 6
 #define NSSUTIL_VBUILD 0
 #define NSSUTIL_BETA PR_FALSE
 
diff --git a/toolkit/components/places/BookmarkHTMLUtils.jsm b/toolkit/components/places/BookmarkHTMLUtils.jsm
index 6b4ea79340..8a54135a7d 100644
--- a/toolkit/components/places/BookmarkHTMLUtils.jsm
+++ b/toolkit/components/places/BookmarkHTMLUtils.jsm
@@ -1052,8 +1052,9 @@ BookmarkExporter.prototype = {
     this._writeLine("<!-- This is an automatically generated file.");
     this._writeLine("     It will be read and overwritten.");
     this._writeLine("     DO NOT EDIT! -->");
-    this._writeLine('<META HTTP-EQUIV="Content-Type" CONTENT="text/html; ' +
-                    'charset=UTF-8">');
+    this._writeLine('<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=UTF-8">');
+    this._writeLine(`<META HTTP-EQUIV="Content-Security-Policy"
+    CONTENT="default-src 'self'; script-src 'none'; img-src data: *; object-src 'none'"></META>`);
     this._writeLine("<TITLE>Bookmarks</TITLE>");
   },
 
diff --git a/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm b/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
index 0dfb7cd109..7eb8c9b9d5 100644
--- a/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
@@ -611,6 +611,14 @@ function UpdateParser(aId, aUpdateKey, aUrl, aObserver) {
   let requireBuiltIn = Services.prefs.getBoolPref(PREF_UPDATE_REQUIREBUILTINCERTS, true);
 
   logger.debug("Requesting " + aUrl);
+
+  if (!aUrl) {
+    logger.warn("Request failed: empty update manifest URL");
+    this._doneAt = new Error("UP_emptyManifestURL");
+    this.notifyError(AddonUpdateChecker.ERROR_DOWNLOAD_ERROR);
+    return;
+  }
+
   try {
     this.request = new ServiceRequest();
     this.request.open("GET", this.url, true);
diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
index dbb1a18dd1..d266ab6fac 100644
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -6129,7 +6129,11 @@ function UpdateChecker(aAddon, aListener, aReason, aAppVersion, aPlatformVersion
   if ("onUpdateAvailable" in this.listener)
     aReason |= UPDATE_TYPE_NEWVERSION;
 
-  let url = escapeAddonURI(aAddon, updateURL, aReason, aAppVersion);
+  // Don't perform substitutions on the update URL if we still don't
+  // have one at this point.
+  let url = updateURL ?
+            escapeAddonURI(aAddon, url, aReason, aAppVersion) :
+            updateURL;
   this._parser = AddonUpdateChecker.checkForUpdates(aAddon.id, aAddon.updateKey,
                                                     url, this);
 }
diff --git a/widget/nsITransferable.idl b/widget/nsITransferable.idl
index b128586dd1..e580673f5e 100644
--- a/widget/nsITransferable.idl
+++ b/widget/nsITransferable.idl
@@ -13,12 +13,17 @@ interface nsIPrincipal;
 
 %{ C++
 
+// Internal formats must have their second part starting with 'x-moz-',
+// for example text/x-moz-internaltype. These cannot be assigned by
+// unprivileged content but all other types can.
+#define kInternal_Mimetype_Prefix   "/x-moz-"
+
 // these probably shouldn't live here, but in some central repository shared
 // by the entire app.
 #define kTextMime                   "text/plain"
 #define kRTFMime                    "text/rtf"
 #define kUnicodeMime                "text/unicode"
-#define kMozTextInternal          "text/x-moz-text-internal"  // text data which isn't suppoed to be parsed by other apps.
+#define kMozTextInternal            "text/x-moz-text-internal"  // text data which isn't suppoed to be parsed by other apps.
 #define kHTMLMime                   "text/html"
 #define kAOLMailMime                "AOLMAIL"
 #define kPNGImageMime               "image/png"
diff --git a/widget/windows/nsFilePicker.cpp b/widget/windows/nsFilePicker.cpp
index 59ae152ec6..3d733fcb7f 100644
--- a/widget/windows/nsFilePicker.cpp
+++ b/widget/windows/nsFilePicker.cpp
@@ -35,12 +35,14 @@ using mozilla::mscom::EnsureMTA;
 using mozilla::UniquePtr;
 using namespace mozilla::widget;
 
-char16_t *nsFilePicker::mLastUsedUnicodeDirectory;
+UniquePtr<char16_t[], nsFilePicker::FreeDeleter>
+    nsFilePicker::sLastUsedUnicodeDirectory;
+
 char nsFilePicker::mLastUsedDirectory[MAX_PATH+1] = { 0 };
 
 static const wchar_t kDialogPtrProp[] = L"DialogPtrProperty";
 static const DWORD kDialogTimerID = 9999;
-static const unsigned long kDialogTimerTimeout = 300;
+
 
 #define MAX_EXTENSION_LENGTH 10
 #define FILE_BUFFER_SIZE     4096 
@@ -71,31 +73,6 @@ private:
   RefPtr<nsWindow> mWindow;
 };
 
-// Manages the current working path.
-class AutoRestoreWorkingPath
-{
-public:
-  AutoRestoreWorkingPath() {
-    DWORD bufferLength = GetCurrentDirectoryW(0, nullptr);
-    mWorkingPath = MakeUnique<wchar_t[]>(bufferLength);
-    if (GetCurrentDirectoryW(bufferLength, mWorkingPath.get()) == 0) {
-      mWorkingPath = nullptr;
-    }
-  }
-
-  ~AutoRestoreWorkingPath() {
-    if (HasWorkingPath()) {
-      ::SetCurrentDirectoryW(mWorkingPath.get());
-    }
-  }
-
-  inline bool HasWorkingPath() const {
-    return mWorkingPath != nullptr;
-  }
-private:
-  UniquePtr<wchar_t[]> mWorkingPath;
-};
-
 // Manages NS_NATIVE_TMP_WINDOW child windows. NS_NATIVE_TMP_WINDOWs are
 // temporary child windows of mParentWidget created to address RTL issues
 // in picker dialogs. We are responsible for destroying these.
@@ -140,57 +117,11 @@ private:
   RefPtr<nsWindow> mWindow;
 };
 
-// Manages a simple callback timer
-class AutoTimerCallbackCancel
-{
-public:
-  AutoTimerCallbackCancel(nsFilePicker* aTarget,
-                          nsTimerCallbackFunc aCallbackFunc) {
-    Init(aTarget, aCallbackFunc);
-  }
-
-  ~AutoTimerCallbackCancel() {
-    if (mPickerCallbackTimer) {
-      mPickerCallbackTimer->Cancel();
-    }
-  }
-
-private:
-  void Init(nsFilePicker* aTarget,
-            nsTimerCallbackFunc aCallbackFunc) {
-    mPickerCallbackTimer = do_CreateInstance("@mozilla.org/timer;1");
-    if (!mPickerCallbackTimer) {
-      NS_WARNING("do_CreateInstance for timer failed??");
-      return;
-    }
-    mPickerCallbackTimer->InitWithFuncCallback(aCallbackFunc,
-                                               aTarget,
-                                               kDialogTimerTimeout,
-                                               nsITimer::TYPE_REPEATING_SLACK);
-  }
-  nsCOMPtr<nsITimer> mPickerCallbackTimer;
-    
-};
-
 ///////////////////////////////////////////////////////////////////////////////
 // nsIFilePicker
 
-nsFilePicker::nsFilePicker() :
-  mSelectedType(1)
-  , mDlgWnd(nullptr)
-  , mFDECookie(0)
-{
-   CoInitialize(nullptr);
-}
-
-nsFilePicker::~nsFilePicker()
-{
-  if (mLastUsedUnicodeDirectory) {
-    free(mLastUsedUnicodeDirectory);
-    mLastUsedUnicodeDirectory = nullptr;
-  }
-  CoUninitialize();
-}
+nsFilePicker::nsFilePicker()
+    : mSelectedType(1) {}
 
 NS_IMPL_ISUPPORTS(nsFilePicker, nsIFilePicker)
 
@@ -205,130 +136,6 @@ NS_IMETHODIMP nsFilePicker::Init(mozIDOMWindowProxy *aParent, const nsAString& a
   return nsBaseFilePicker::Init(aParent, aTitle, aMode);
 }
 
-STDMETHODIMP nsFilePicker::QueryInterface(REFIID refiid, void** ppvResult)
-{
-  *ppvResult = nullptr;
-  if (IID_IUnknown == refiid ||
-      refiid == IID_IFileDialogEvents) {
-    *ppvResult = this;
-  }
-
-  if (nullptr != *ppvResult) {
-    ((LPUNKNOWN)*ppvResult)->AddRef();
-    return S_OK;
-  }
-
-  return E_NOINTERFACE;
-}
-
-
-/*
- * Callbacks
- */
-
-HRESULT
-nsFilePicker::OnFileOk(IFileDialog *pfd)
-{
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnFolderChanging(IFileDialog *pfd,
-                               IShellItem *psiFolder)
-{
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnFolderChange(IFileDialog *pfd)
-{
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnSelectionChange(IFileDialog *pfd)
-{
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnShareViolation(IFileDialog *pfd,
-                               IShellItem *psi,
-                               FDE_SHAREVIOLATION_RESPONSE *pResponse)
-{
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnTypeChange(IFileDialog *pfd)
-{
-  // Failures here result in errors due to security concerns.
-  RefPtr<IOleWindow> win;
-  pfd->QueryInterface(IID_IOleWindow, getter_AddRefs(win));
-  if (!win) {
-    NS_ERROR("Could not retrieve the IOleWindow interface for IFileDialog.");
-    return S_OK;
-  }
-  HWND hwnd = nullptr;
-  win->GetWindow(&hwnd);
-  if (!hwnd) {
-    NS_ERROR("Could not retrieve the HWND for IFileDialog.");
-    return S_OK;
-  }
-  
-  SetDialogHandle(hwnd);
-  return S_OK;
-}
-
-HRESULT
-nsFilePicker::OnOverwrite(IFileDialog *pfd,
-                          IShellItem *psi,
-                          FDE_OVERWRITE_RESPONSE *pResponse)
-{
-  return S_OK;
-}
-
-/*
- * Close on parent close logic
- */
-
-bool
-nsFilePicker::ClosePickerIfNeeded()
-{
-  if (!mParentWidget || !mDlgWnd)
-    return false;
-
-  nsWindow *win = static_cast<nsWindow *>(mParentWidget.get());
-  if (IsWindow(mDlgWnd) && IsWindowVisible(mDlgWnd) && win->DestroyCalled()) {
-    wchar_t className[64];
-    // Make sure we have the right window
-    if (GetClassNameW(mDlgWnd, className, mozilla::ArrayLength(className)) &&
-        !wcscmp(className, L"#32770") &&
-        DestroyWindow(mDlgWnd)) {
-      mDlgWnd = nullptr;
-      return true;
-    }
-  }
-  return false;
-}
-
-void
-nsFilePicker::PickerCallbackTimerFunc(nsITimer *aTimer, void *aCtx)
-{
-  nsFilePicker* picker = (nsFilePicker*)aCtx;
-  if (picker->ClosePickerIfNeeded()) {
-    aTimer->Cancel();
-  }
-}
-
-void
-nsFilePicker::SetDialogHandle(HWND aWnd)
-{
-  if (!aWnd || mDlgWnd)
-    return;
-  mDlgWnd = aWnd;
-}
-
 /*
  * Folder picker invocation
  */
@@ -353,15 +160,14 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir)
   }
 
   RefPtr<IFileOpenDialog> dialog;
-  if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC,
+  if (FAILED(CoCreateInstance(CLSID_FileOpenDialog,
+                              nullptr,
+                              CLSCTX_INPROC_SERVER,
                               IID_IFileOpenDialog,
                               getter_AddRefs(dialog)))) {
     return false;
   }
 
-  // hook up event callbacks
-  dialog->Advise(this, &mFDECookie);
-
   // options
   FILEOPENDIALOGOPTIONS fos = FOS_PICKFOLDERS;
   // Require interaction if the folder picker is triggered by an element that
@@ -371,13 +177,22 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir)
     fos |= FOS_OKBUTTONNEEDSINTERACTION;
   }
 
-  dialog->SetOptions(fos);
+  HRESULT hr = dialog->SetOptions(fos);
+  if (FAILED(hr)) {
+    return false;
+  }
  
   // initial strings
-  dialog->SetTitle(mTitle.get());
+  hr = dialog->SetTitle(mTitle.get());
+  if (FAILED(hr)) {
+    return false;
+  }
 
   if (!mOkButtonLabel.IsEmpty()) {
-    dialog->SetOkButtonLabel(mOkButtonLabel.get());
+    hr = dialog->SetOkButtonLabel(mOkButtonLabel.get());
+    if (FAILED(hr)) {
+      return false;
+    }
   }
 
   if (!aInitialDir.IsEmpty()) {
@@ -386,7 +201,10 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir)
           WinUtils::SHCreateItemFromParsingName(aInitialDir.get(), nullptr,
                                                 IID_IShellItem,
                                                 getter_AddRefs(folder)))) {
-      dialog->SetFolder(folder);
+      hr = dialog->SetFolder(folder);
+      if (FAILED(hr)) {
+        return false;
+      }
     }
   }
 
@@ -398,10 +216,8 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir)
   if (FAILED(dialog->Show(adtw.get())) ||
       FAILED(dialog->GetResult(getter_AddRefs(item))) ||
       !item) {
-    dialog->Unadvise(mFDECookie);
     return false;
   }
-  dialog->Unadvise(mFDECookie);
 
   // results
 
@@ -409,8 +225,14 @@ nsFilePicker::ShowFolderPicker(const nsString& aInitialDir)
   // default save folder.
   RefPtr<IShellItem> folderPath;
   RefPtr<IShellLibrary> shellLib;
-  CoCreateInstance(CLSID_ShellLibrary, nullptr, CLSCTX_INPROC,
-                   IID_IShellLibrary, getter_AddRefs(shellLib));
+  if (FAILED(CoCreateInstance(CLSID_ShellLibrary,
+                              nullptr,
+                              CLSCTX_INPROC_SERVER,
+                              IID_IShellLibrary,
+                              getter_AddRefs(shellLib)))) {
+    return false;
+  }
+
   if (shellLib &&
       SUCCEEDED(shellLib->LoadLibraryFromItem(item, STGM_READ)) &&
       SUCCEEDED(shellLib->GetDefaultSaveFolder(DSFT_DETECT, IID_IShellItem,
@@ -449,22 +271,23 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
 
   RefPtr<IFileDialog> dialog;
   if (mMode != modeSave) {
-    if (FAILED(CoCreateInstance(CLSID_FileOpenDialog, nullptr, CLSCTX_INPROC,
+    if (FAILED(CoCreateInstance(CLSID_FileOpenDialog,
+                                nullptr,
+                                CLSCTX_INPROC_SERVER,
                                 IID_IFileOpenDialog,
                                 getter_AddRefs(dialog)))) {
       return false;
     }
   } else {
-    if (FAILED(CoCreateInstance(CLSID_FileSaveDialog, nullptr, CLSCTX_INPROC,
+    if (FAILED(CoCreateInstance(CLSID_FileSaveDialog,
+                                nullptr,
+                                CLSCTX_INPROC_SERVER,
                                 IID_IFileSaveDialog,
                                 getter_AddRefs(dialog)))) {
       return false;
     }
   }
 
-  // hook up event callbacks
-  dialog->Advise(this, &mFDECookie);
-
   // options
 
   FILEOPENDIALOGOPTIONS fos = 0;
@@ -483,10 +306,6 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
     fos |= FOS_DONTADDTORECENT;
   }
 
-  // Msdn claims FOS_NOCHANGEDIR is not needed. We'll add this
-  // just in case.
-  AutoRestoreWorkingPath arw;
-
   // mode specific
   switch(mMode) {
     case modeOpen:
@@ -506,25 +325,45 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
       break;
   }
 
-  dialog->SetOptions(fos);
+  HRESULT hr = dialog->SetOptions(fos);
+  if (FAILED(hr)) {
+    return false;
+  }
 
   // initial strings
 
   // title
-  dialog->SetTitle(mTitle.get());
+  hr = dialog->SetTitle(mTitle.get());
+  if (FAILED(hr)) {
+    return false;
+  }
 
   // default filename
   if (!mDefaultFilename.IsEmpty()) {
-    dialog->SetFileName(mDefaultFilename.get());
+    // Prevent the shell from expanding environment variables by removing
+    // the % characters that are used to delimit them.
+    nsAutoString sanitizedFilename(mDefaultFilename);
+    sanitizedFilename.ReplaceChar('%', '_');
+
+    hr = dialog->SetFileName(sanitizedFilename.get());
+    if (FAILED(hr)) {
+      return false;
+    }
   }
   
   NS_NAMED_LITERAL_STRING(htmExt, "html");
 
   // default extension to append to new files
   if (!mDefaultExtension.IsEmpty()) {
-    dialog->SetDefaultExtension(mDefaultExtension.get());
+    hr = dialog->SetDefaultExtension(mDefaultExtension.get());
+    if (FAILED(hr)) {
+      return false;
+    }
   } else if (IsDefaultPathHtml()) {
-    dialog->SetDefaultExtension(htmExt.get());
+    hr = dialog->SetDefaultExtension(htmExt.get());
+    if (FAILED(hr)) {
+      return false;
+    }
   }
 
   // initial location
@@ -534,14 +373,24 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
           WinUtils::SHCreateItemFromParsingName(aInitialDir.get(), nullptr,
                                                 IID_IShellItem,
                                                 getter_AddRefs(folder)))) {
-      dialog->SetFolder(folder);
+      hr = dialog->SetFolder(folder);
+      if (FAILED(hr)) {
+        return false;
+      }
     }
   }
 
   // filter types and the default index
   if (!mComFilterList.IsEmpty()) {
-    dialog->SetFileTypes(mComFilterList.Length(), mComFilterList.get());
-    dialog->SetFileTypeIndex(mSelectedType);
+    hr = dialog->SetFileTypes(mComFilterList.Length(), mComFilterList.get());
+    if (FAILED(hr)) {
+      return false;
+    }
+
+    hr = dialog->SetFileTypeIndex(mSelectedType);
+    if (FAILED(hr)) {
+      return false;
+    }
   }
 
   // display
@@ -549,14 +398,11 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
   {
     AutoDestroyTmpWindow adtw((HWND)(mParentWidget.get() ?
       mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
-    AutoTimerCallbackCancel atcc(this, PickerCallbackTimerFunc);
     AutoWidgetPickerState awps(mParentWidget);
 
     if (FAILED(dialog->Show(adtw.get()))) {
-      dialog->Unadvise(mFDECookie);
       return false;
     }
-    dialog->Unadvise(mFDECookie);
   }
 
   // results
@@ -596,9 +442,10 @@ nsFilePicker::ShowFilePicker(const nsString& aInitialDir)
     if (SUCCEEDED(items->GetItemAt(idx, getter_AddRefs(item)))) {
       if (!WinUtils::GetShellItemPath(item, str))
         continue;
-      nsCOMPtr<nsIFile> file = do_CreateInstance("@mozilla.org/file/local;1");
-      if (file && NS_SUCCEEDED(file->InitWithPath(str)))
+      nsCOMPtr<nsIFile> file;
+      if (NS_SUCCEEDED(NS_NewLocalFile(str, false, getter_AddRefs(file)))) {
         mFiles.AppendObject(file);
+      }
     }
   }
   return true;
@@ -623,7 +470,7 @@ nsFilePicker::ShowW(int16_t *aReturnVal)
   // If no display directory, re-use the last one.
   if(initialDir.IsEmpty()) {
     // Allocate copy of last used dir.
-    initialDir = mLastUsedUnicodeDirectory;
+    initialDir = sLastUsedUnicodeDirectory.get();
   }
 
   // Clear previous file selections
@@ -651,10 +498,11 @@ nsFilePicker::ShowW(int16_t *aReturnVal)
   if (mMode == modeSave) {
     // Windows does not return resultReplace, we must check if file
     // already exists.
-    nsCOMPtr<nsIFile> file(do_CreateInstance("@mozilla.org/file/local;1"));
+    nsCOMPtr<nsIFile> file;
+    nsresult rv = NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file));
+
     bool flag = false;
-    if (file && NS_SUCCEEDED(file->InitWithPath(mUnicodeFile)) &&
-        NS_SUCCEEDED(file->Exists(&flag)) && flag) {
+    if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(file->Exists(&flag)) && flag) {
       retValue = returnReplace;
     }
   }
@@ -678,14 +526,13 @@ nsFilePicker::GetFile(nsIFile **aFile)
   if (mUnicodeFile.IsEmpty())
       return NS_OK;
 
-  nsCOMPtr<nsIFile> file(do_CreateInstance("@mozilla.org/file/local;1"));
-    
-  NS_ENSURE_TRUE(file, NS_ERROR_FAILURE);
-
-  file->InitWithPath(mUnicodeFile);
-
-  NS_ADDREF(*aFile = file);
+  nsCOMPtr<nsIFile> file;
+  nsresult rv = NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
+  file.forget(aFile);
   return NS_OK;
 }
 
@@ -800,8 +647,13 @@ nsFilePicker::AppendFilter(const nsAString& aTitle, const nsAString& aFilter)
 void
 nsFilePicker::RememberLastUsedDirectory()
 {
-  nsCOMPtr<nsIFile> file(do_CreateInstance("@mozilla.org/file/local;1"));
-  if (!file || NS_FAILED(file->InitWithPath(mUnicodeFile))) {
+  if (IsPrivacyModeEnabled()) {
+    // Don't remember the directory if private browsing was in effect
+    return;
+  }
+
+  nsCOMPtr<nsIFile> file;
+  if (NS_FAILED(NS_NewLocalFile(mUnicodeFile, false, getter_AddRefs(file)))) {
     NS_WARNING("RememberLastUsedDirectory failed to init file path.");
     return;
   }
@@ -816,11 +668,7 @@ nsFilePicker::RememberLastUsedDirectory()
     return;
   }
 
-  if (mLastUsedUnicodeDirectory) {
-    free(mLastUsedUnicodeDirectory);
-    mLastUsedUnicodeDirectory = nullptr;
-  }
-  mLastUsedUnicodeDirectory = ToNewUnicode(newDir);
+  sLastUsedUnicodeDirectory.reset(ToNewUnicode(newDir));
 }
 
 bool
diff --git a/widget/windows/nsFilePicker.h b/widget/windows/nsFilePicker.h
index 7a3fbbe073..050b6acafa 100644
--- a/widget/windows/nsFilePicker.h
+++ b/widget/windows/nsFilePicker.h
@@ -10,7 +10,6 @@
 #include <windows.h>
 
 #include "nsIFile.h"
-#include "nsITimer.h"
 #include "nsISimpleEnumerator.h"
 #include "nsCOMArray.h"
 #include "nsBaseFilePicker.h"
@@ -41,11 +40,9 @@ protected:
  * Native Windows FileSelector wrapper
  */
 
-class nsFilePicker :
-  public IFileDialogEvents,
-  public nsBaseWinFilePicker
-{
-  virtual ~nsFilePicker();
+class nsFilePicker : public nsBaseWinFilePicker {
+  virtual ~nsFilePicker() = default;
+
 public:
   nsFilePicker();
 
@@ -54,9 +51,6 @@ public:
 
   NS_DECL_ISUPPORTS
 
-  // IUnknown's QueryInterface
-  STDMETHODIMP QueryInterface(REFIID refiid, void** ppvResult);
-
   // nsIFilePicker (less what's in nsBaseFilePicker and nsBaseWinFilePicker)
   NS_IMETHOD GetFilterIndex(int32_t *aFilterIndex);
   NS_IMETHOD SetFilterIndex(int32_t aFilterIndex);
@@ -67,15 +61,6 @@ public:
   NS_IMETHOD ShowW(int16_t *aReturnVal);
   NS_IMETHOD AppendFilter(const nsAString& aTitle, const nsAString& aFilter);
 
-  // IFileDialogEvents
-  HRESULT STDMETHODCALLTYPE OnFileOk(IFileDialog *pfd);
-  HRESULT STDMETHODCALLTYPE OnFolderChanging(IFileDialog *pfd, IShellItem *psiFolder);
-  HRESULT STDMETHODCALLTYPE OnFolderChange(IFileDialog *pfd);
-  HRESULT STDMETHODCALLTYPE OnSelectionChange(IFileDialog *pfd);
-  HRESULT STDMETHODCALLTYPE OnShareViolation(IFileDialog *pfd, IShellItem *psi, FDE_SHAREVIOLATION_RESPONSE *pResponse);
-  HRESULT STDMETHODCALLTYPE OnTypeChange(IFileDialog *pfd);
-  HRESULT STDMETHODCALLTYPE OnOverwrite(IFileDialog *pfd, IShellItem *psi, FDE_OVERWRITE_RESPONSE *pResponse);
-
 protected:
   /* method from nsBaseFilePicker */
   virtual void InitNative(nsIWidget *aParent,
@@ -87,9 +72,6 @@ protected:
   bool IsPrivacyModeEnabled();
   bool IsDefaultPathLink();
   bool IsDefaultPathHtml();
-  void SetDialogHandle(HWND aWnd);
-  bool ClosePickerIfNeeded();
-  static void PickerCallbackTimerFunc(nsITimer *aTimer, void *aPicker);
 
   nsCOMPtr<nsILoadContext> mLoadContext;
   nsCOMPtr<nsIWidget>    mParentWidget;
@@ -100,10 +82,13 @@ protected:
   nsCOMArray<nsIFile>    mFiles;
   static char            mLastUsedDirectory[];
   nsString               mUnicodeFile;
-  static char16_t      *mLastUsedUnicodeDirectory;
-  HWND                   mDlgWnd;
   bool                   mRequireInteraction;
 
+  struct FreeDeleter {
+    void operator()(void* aPtr) { ::free(aPtr); }
+  };
+  static mozilla::UniquePtr<char16_t[], FreeDeleter> sLastUsedUnicodeDirectory;
+
   class ComDlgFilterSpec
   {
   public:
@@ -129,7 +114,6 @@ protected:
   };
 
   ComDlgFilterSpec       mComFilterList;
-  DWORD                  mFDECookie;
 };
 
 #endif // nsFilePicker_h__
diff --git a/xpcom/threads/TimerThread.cpp b/xpcom/threads/TimerThread.cpp
index db7c924623..a2e40c3880 100644
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -140,11 +140,7 @@ public:
 
   nsresult Cancel() override
   {
-    // Since nsTimerImpl is not thread-safe, we should release |mTimer|
-    // here in the target thread to avoid race condition. Otherwise,
-    // ~nsTimerEvent() which calls nsTimerImpl::Release() could run in the
-    // timer thread and result in race condition.
-    mTimer = nullptr;
+    mTimer->Cancel();
     return NS_OK;
   }
 
@@ -269,11 +265,6 @@ nsTimerEvent::DeleteAllocatorIfNeeded()
 NS_IMETHODIMP
 nsTimerEvent::Run()
 {
-  if (!mTimer) {
-    MOZ_ASSERT(false);
-    return NS_OK;
-  }
-
   if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) {
     TimeStamp now = TimeStamp::Now();
     MOZ_LOG(GetTimerLog(), LogLevel::Debug,
@@ -283,9 +274,7 @@ nsTimerEvent::Run()
 
   mTimer->Fire(mGeneration);
 
-  // We call Cancel() to correctly release mTimer.
-  // Read more in the Cancel() implementation.
-  return Cancel();
+  return NS_OK;
 }
 
 nsresult
@@ -634,10 +623,13 @@ TimerThread::AddTimerInternal(nsTimerImpl* aTimer)
   return insertSlot - mTimers.Elements();
 }
 
+// This function must be called from within a lock.
+// Also: we hold the mutex for the nsTimerImpl.
 bool
 TimerThread::RemoveTimerInternal(nsTimerImpl* aTimer)
 {
   mMonitor.AssertCurrentThreadOwns();
+  aTimer->mMutex.AssertCurrentThreadOwns();
   if (!mTimers.RemoveElement(aTimer)) {
     return false;
   }
@@ -701,12 +693,17 @@ TimerThread::PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef)
     // at the TimerThread we'll deadlock.
     MonitorAutoUnlock unlock(mMonitor);
     rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
-  }
-
-  if (NS_FAILED(rv)) {
-    timer = event->ForgetTimer();
-    RemoveTimerInternal(timer);
-    return timer.forget();
+    if (NS_FAILED(rv)) {
+      timer = event->ForgetTimer();
+      // We do this to avoid possible deadlock by taking the two locks in a
+      // different order than is used in RemoveTimer(). RemoveTimer() has
+      // aTimer->mMutex first. We use timer.get() to keep static analysis
+      // happy.
+      MutexAutoLock lock1(timer.get()->mMutex);
+      MonitorAutoLock lock2(mMonitor);
+      RemoveTimerInternal(timer.get());
+      return timer.forget();
+    }
   }
 
   return nullptr;
diff --git a/xpcom/threads/TimerThread.h b/xpcom/threads/TimerThread.h
index 4d4b7be77d..c82f3ec7f5 100644
--- a/xpcom/threads/TimerThread.h
+++ b/xpcom/threads/TimerThread.h
@@ -69,6 +69,10 @@ private:
   already_AddRefed<nsTimerImpl> PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef);
 
   nsCOMPtr<nsIThread> mThread;
+  // Lock ordering requirements:
+  // (optional) ThreadWrapper::sMutex ->
+  // (optional) nsTimerImpl::mMutex   ->
+  // TimerThread::mMonitor
   Monitor mMonitor;
 
   bool mShutdown;
diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp
index 830315cefa..2a3531a858 100644
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -433,12 +433,16 @@ nsTimerImpl::Fire(int32_t aGeneration)
   uint8_t oldType;
   uint32_t oldDelay;
   TimeStamp oldTimeout;
+  nsCOMPtr<nsITimer> timer;
 
   {
+    MutexAutoLock lock(mMutex);
+
     // Don't fire callbacks or fiddle with refcounts when the mutex is locked.
     // If some other thread Cancels/Inits after this, they're just too late.
-    MutexAutoLock lock(mMutex);
     if (aGeneration != mGeneration) {
+      // This timer got rescheduled or cancelled before we fired, so ignore this
+      // firing
       return;
     }
 
@@ -446,6 +450,10 @@ nsTimerImpl::Fire(int32_t aGeneration)
     oldType = mType;
     oldDelay = mDelay;
     oldTimeout = mTimeout;
+    // Ensure that the nsITimer does not unhook from the nsTimerImpl during
+    // Fire; this will cause null pointer crashes if the user of the timer drops
+    // its reference, and then uses the nsITimer* passed in the callback.
+    timer = mITimer;    
   }
 
   PROFILER_LABEL("Timer", "Fire",
@@ -475,13 +483,13 @@ nsTimerImpl::Fire(int32_t aGeneration)
 
   switch (mCallbackDuringFire.mType) {
     case Callback::Type::Function:
-      mCallbackDuringFire.mCallback.c(mITimer, mCallbackDuringFire.mClosure);
+      mCallbackDuringFire.mCallback.c(timer, mCallbackDuringFire.mClosure);
       break;
     case Callback::Type::Interface:
-      mCallbackDuringFire.mCallback.i->Notify(mITimer);
+      mCallbackDuringFire.mCallback.i->Notify(timer);
       break;
     case Callback::Type::Observer:
-      mCallbackDuringFire.mCallback.o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC,
+      mCallbackDuringFire.mCallback.o->Observe(timer, NS_TIMER_CALLBACK_TOPIC,
                                                nullptr);
       break;
     default:
diff --git a/xpcom/threads/nsTimerImpl.h b/xpcom/threads/nsTimerImpl.h
index 9d59b4e672..039294995d 100644
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -166,7 +166,10 @@ public:
   int32_t               mGeneration;
 
   uint32_t              mDelay;
-  // Updated only after this timer has been removed from the timer thread.
+  // Never updated while in the TimerThread's timer list. Only updated
+  // before adding to that list or during nsTimerImpl::Fire(), when it has
+  // been removed from the TimerThread's list. TimerThread can safely access
+  // mTimeout of any timer in the list.
   TimeStamp             mTimeout;
 
 #ifdef MOZ_TASK_TRACER