Bug 1356103 - Part 8: Use PostTraversalTasks to deal with FontFaceSet's Promise and DOM event dispatch during Servo traversal. r=bholley draft
authorCameron McCormack <cam@mcc.id.au>
Sun, 30 Apr 2017 14:55:22 +0800
changeset 572580 dff7c804d7e320b8cdad31ed542d3114ea23344f
parent 572579 ce4d984030ee4cf01f2c8b2dff25d349c6eb8444
child 572581 e3ff05a6108d4709a21e836d9ab43defdad2ad33
push id57115
push userbmo:cam@mcc.id.au
push dateThu, 04 May 2017 10:01:25 +0000
reviewersbholley
bugs1356103
milestone55.0a1
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,
   };
 
   explicit PostTraversalTask(Type aType)
     : mType(aType)
     , mTarget(nullptr)
     , mResult(NS_OK)
   {
   }