Bug 1356103 - Part 8: Use PostTraversalTasks to deal with FontFaceSet's Promise and DOM event dispatch during Servo traversal. r=bholley
☠☠ backed out by 8f8cb62a8749 ☠ ☠
authorCameron McCormack <cam@mcc.id.au>
Sun, 30 Apr 2017 14:55:22 +0800
changeset 404586 a03112e1c9d5597e91b48d94a49d62890a33d298
parent 404585 c60b4c9cbd83e774eca3014e1e0d78f67b055b62
child 404587 2f383d89184b02533e9afbfa4d614db064e0b2c6
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1356103
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1356103 - Part 8: Use PostTraversalTasks to deal with FontFaceSet's Promise and DOM event dispatch during Servo traversal. r=bholley The PostTraversalTask does not take a strong reference to the FontFaceSet object, since as a DOM object, we can't call AddRef/Release on it from the Servo style worker threads. The FontFaceSet is held on to by the nsIDocument, and only cleared during cycle collection, which should not occur between the font metrics request and the PostTraversalTask running. MozReview-Commit-ID: 5aKIg4DIQ4w
layout/style/FontFaceSet.cpp
layout/style/FontFaceSet.h
layout/style/PostTraversalTask.cpp
layout/style/PostTraversalTask.h
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -12,16 +12,18 @@
 #include "mozilla/dom/FontFaceSetBinding.h"
 #include "mozilla/dom/FontFaceSetIterator.h"
 #include "mozilla/dom/FontFaceSetLoadEvent.h"
 #include "mozilla/dom/FontFaceSetLoadEventBinding.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/AsyncEventDispatcher.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/ServoStyleSet.h"
+#include "mozilla/ServoUtils.h"
 #include "mozilla/SizePrintfMacros.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/Telemetry.h"
 #include "nsAutoPtr.h"
 #include "nsContentPolicyUtils.h"
 #include "nsCSSParser.h"
 #include "nsDeviceContext.h"
 #include "nsFontFaceLoader.h"
@@ -123,16 +125,20 @@ FontFaceSet::FontFaceSet(nsPIDOMWindowIn
 
   mDocument->CSSLoader()->AddObserver(this);
 
   mUserFontSet = new UserFontSet(this);
 }
 
 FontFaceSet::~FontFaceSet()
 {
+  // Assert that we don't drop any FontFaceSet objects during a Servo traversal,
+  // since PostTraversalTask objects can hold raw pointers to FontFaceSets.
+  MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal());
+
   Disconnect();
   for (auto it = mLoaders.Iter(); !it.Done(); it.Next()) {
     it.Get()->GetKey()->Cancel();
   }
 }
 
 JSObject*
 FontFaceSet::WrapObject(JSContext* aContext, JS::Handle<JSObject*> aGivenProto)
@@ -375,16 +381,18 @@ FontFaceSet::Check(const nsAString& aFon
   }
 
   return true;
 }
 
 Promise*
 FontFaceSet::GetReady(ErrorResult& aRv)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!mReady) {
     nsCOMPtr<nsIGlobalObject> global = GetParentObject();
     mReady = Promise::Create(global, aRv);
     if (!mReady) {
       aRv.Throw(NS_ERROR_FAILURE);
       return nullptr;
     }
     if (mResolveLazilyCreatedReadyPromise) {
@@ -1457,16 +1465,18 @@ FontFaceSet::GetPrivateBrowsing()
 {
   nsCOMPtr<nsILoadContext> loadContext = mDocument->GetLoadContext();
   return loadContext && loadContext->UsePrivateBrowsing();
 }
 
 void
 FontFaceSet::OnFontFaceStatusChanged(FontFace* aFontFace)
 {
+  AssertIsMainThreadOrServoFontMetricsLocked();
+
   MOZ_ASSERT(HasAvailableFontFace(aFontFace));
 
   mHasLoadingFontFacesIsDirty = true;
 
   if (aFontFace->Status() == FontFaceLoadStatus::Loading) {
     CheckLoadingStarted();
   } else {
     MOZ_ASSERT(aFontFace->Status() == FontFaceLoadStatus::Loaded ||
@@ -1475,51 +1485,93 @@ FontFaceSet::OnFontFaceStatusChanged(Fon
     // will be called immediately afterwards to request a reflow of the
     // relevant elements in the document.  We want to wait until the reflow
     // request has been done before the FontFaceSet is marked as Loaded so
     // that we don't briefly set the FontFaceSet to Loaded and then Loading
     // again once the reflow is pending.  So we go around the event loop
     // and call CheckLoadingFinished() after the reflow has been queued.
     if (!mDelayedLoadCheck) {
       mDelayedLoadCheck = true;
-      nsCOMPtr<nsIRunnable> checkTask =
-        NewRunnableMethod(this, &FontFaceSet::CheckLoadingFinishedAfterDelay);
-      mDocument->Dispatch("FontFaceSet::CheckLoadingFinishedAfterDelay",
-                          TaskCategory::Other, checkTask.forget());
+      DispatchCheckLoadingFinishedAfterDelay();
     }
   }
 }
 
 void
+FontFaceSet::DispatchCheckLoadingFinishedAfterDelay()
+{
+  AssertIsMainThreadOrServoFontMetricsLocked();
+
+  if (ServoStyleSet* set = ServoStyleSet::Current()) {
+    // See comments in Gecko_GetFontMetrics.
+    //
+    // We can't just dispatch the runnable below if we're not on the main
+    // thread, since it needs to take a strong reference to the FontFaceSet,
+    // and being a DOM object, FontFaceSet doesn't support thread-safe
+    // refcounting.
+    set->AppendTask(PostTraversalTask::DispatchFontFaceSetCheckLoadingFinishedAfterDelay(this));
+    return;
+  }
+
+  nsCOMPtr<nsIRunnable> checkTask =
+    NewRunnableMethod(this, &FontFaceSet::CheckLoadingFinishedAfterDelay);
+  mDocument->Dispatch("FontFaceSet::CheckLoadingFinishedAfterDelay",
+                      TaskCategory::Other, checkTask.forget());
+}
+
+void
 FontFaceSet::DidRefresh()
 {
   CheckLoadingFinished();
 }
 
 void
 FontFaceSet::CheckLoadingFinishedAfterDelay()
 {
   mDelayedLoadCheck = false;
   CheckLoadingFinished();
 }
 
 void
 FontFaceSet::CheckLoadingStarted()
 {
+  AssertIsMainThreadOrServoFontMetricsLocked();
+
   if (!HasLoadingFontFaces()) {
     return;
   }
 
   if (mStatus == FontFaceSetLoadStatus::Loading) {
     // We have already dispatched a loading event and replaced mReady
     // with a fresh, unresolved promise.
     return;
   }
 
   mStatus = FontFaceSetLoadStatus::Loading;
+  DispatchLoadingEventAndReplaceReadyPromise();
+}
+
+void
+FontFaceSet::DispatchLoadingEventAndReplaceReadyPromise()
+{
+  AssertIsMainThreadOrServoFontMetricsLocked();
+
+  if (ServoStyleSet* set = ServoStyleSet::Current()) {
+    // See comments in Gecko_GetFontMetrics.
+    //
+    // We can't just dispatch the runnable below if we're not on the main
+    // thread, since it needs to take a strong reference to the FontFaceSet,
+    // and being a DOM object, FontFaceSet doesn't support thread-safe
+    // refcounting.  (Also, the Promise object creation must be done on
+    // the main thread.)
+    set->AppendTask(
+      PostTraversalTask::DispatchLoadingEventAndReplaceReadyPromise(this));
+    return;
+  }
+
   (new AsyncEventDispatcher(this, NS_LITERAL_STRING("loading"),
                             false))->PostDOMEvent();
 
   if (PrefEnabled()) {
     if (mReady) {
       if (GetParentObject()) {
         ErrorResult rv;
         mReady = Promise::Create(GetParentObject(), rv);
@@ -1589,16 +1641,18 @@ FontFaceSet::MightHavePendingFontLoads()
   }
 
   return false;
 }
 
 void
 FontFaceSet::CheckLoadingFinished()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mDelayedLoadCheck) {
     // Wait until the runnable posted in OnFontFaceStatusChanged calls us.
     return;
   }
 
   if (mStatus == FontFaceSetLoadStatus::Loaded) {
     // We've already resolved mReady and dispatched the loadingdone/loadingerror
     // events.
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -15,32 +15,34 @@
 
 struct gfxFontFaceSrc;
 class gfxUserFontEntry;
 class nsFontFaceLoader;
 class nsIPrincipal;
 class nsPIDOMWindowInner;
 
 namespace mozilla {
+class PostTraversalTask;
 namespace css {
 class FontFamilyListRefCnt;
 } // namespace css
 namespace dom {
 class FontFace;
 class Promise;
 } // namespace dom
 } // namespace mozilla
 
 namespace mozilla {
 namespace dom {
 
 class FontFaceSet final : public DOMEventTargetHelper
                         , public nsIDOMEventListener
                         , public nsICSSLoaderObserver
 {
+  friend class mozilla::PostTraversalTask;
   friend class UserFontSet;
 
 public:
   /**
    * A gfxUserFontSet that integrates with the layout and style systems to
    * manage @font-face rules and handle network requests for font loading.
    *
    * We would combine this class and FontFaceSet into the one class if it were
@@ -298,16 +300,19 @@ private:
               int32_t& aStretch,
               uint8_t& aStyle,
               ErrorResult& aRv);
   void FindMatchingFontFaces(const nsAString& aFont,
                              const nsAString& aText,
                              nsTArray<FontFace*>& aFontFaces,
                              mozilla::ErrorResult& aRv);
 
+  void DispatchLoadingEventAndReplaceReadyPromise();
+  void DispatchCheckLoadingFinishedAfterDelay();
+
   TimeStamp GetNavigationStartTimeStamp();
 
   RefPtr<UserFontSet> mUserFontSet;
 
   // The document this is a FontFaceSet for.
   nsCOMPtr<nsIDocument> mDocument;
 
   // A Promise that is fulfilled once all of the FontFace objects
--- a/layout/style/PostTraversalTask.cpp
+++ b/layout/style/PostTraversalTask.cpp
@@ -2,28 +2,39 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "PostTraversalTask.h"
 
 #include "mozilla/dom/FontFace.h"
+#include "mozilla/dom/FontFaceSet.h"
 
 namespace mozilla {
 
 using namespace dom;
 
 void
 PostTraversalTask::Run()
 {
   switch (mType) {
     case Type::ResolveFontFaceLoadedPromise:
       static_cast<FontFace*>(mTarget)->DoResolve();
       break;
 
     case Type::RejectFontFaceLoadedPromise:
       static_cast<FontFace*>(mTarget)->DoReject(mResult);
       break;
+
+    case Type::DispatchLoadingEventAndReplaceReadyPromise:
+      static_cast<FontFaceSet*>(mTarget)->
+        DispatchLoadingEventAndReplaceReadyPromise();
+      break;
+
+    case Type::DispatchFontFaceSetCheckLoadingFinishedAfterDelay:
+      static_cast<FontFaceSet*>(mTarget)->
+        DispatchCheckLoadingFinishedAfterDelay();
+      break;
   }
 }
 
 } // namespace mozilla
--- a/layout/style/PostTraversalTask.h
+++ b/layout/style/PostTraversalTask.h
@@ -7,16 +7,17 @@
 #ifndef mozilla_PostTraversalTask_h
 #define mozilla_PostTraversalTask_h
 
 /* a task to be performed immediately after a Servo traversal */
 
 namespace mozilla {
 namespace dom {
 class FontFace;
+class FontFaceSet;
 } // namespace dom
 } // namespace mozilla
 
 namespace mozilla {
 
 /**
  * A PostTraversalTask is a task to be performed immediately after a Servo
  * traversal.  There are just a few tasks we need to perform, so we use this
@@ -39,30 +40,54 @@ public:
                                                        nsresult aResult)
   {
     auto task = PostTraversalTask(Type::ResolveFontFaceLoadedPromise);
     task.mTarget = aFontFace;
     task.mResult = aResult;
     return task;
   }
 
+  static PostTraversalTask DispatchLoadingEventAndReplaceReadyPromise(
+    dom::FontFaceSet* aFontFaceSet)
+  {
+    auto task =
+      PostTraversalTask(Type::DispatchLoadingEventAndReplaceReadyPromise);
+    task.mTarget = aFontFaceSet;
+    return task;
+  }
+
+  static PostTraversalTask DispatchFontFaceSetCheckLoadingFinishedAfterDelay(
+    dom::FontFaceSet* aFontFaceSet)
+  {
+    auto task =
+      PostTraversalTask(Type::DispatchFontFaceSetCheckLoadingFinishedAfterDelay);
+    task.mTarget = aFontFaceSet;
+    return task;
+  }
+
   void Run();
 
 private:
   // For any new raw pointer type that we need to store in a PostTraversalTask,
   // please add an assertion that class' destructor that we are not in a Servo
   // traversal, to protect against the possibility of having dangling pointers.
   enum class Type
   {
     // mTarget (FontFace*)
     ResolveFontFaceLoadedPromise,
 
     // mTarget (FontFace*)
     // mResult
     RejectFontFaceLoadedPromise,
+
+    // mTarget (FontFaceSet*)
+    DispatchLoadingEventAndReplaceReadyPromise,
+
+    // mTarget (FontFaceSet*)
+    DispatchFontFaceSetCheckLoadingFinishedAfterDelay,
   };
 
   PostTraversalTask(Type aType)
     : mType(aType)
     , mTarget(nullptr)
     , mResult(NS_OK)
   {
   }