Bug 1309867 - Part 2: Lazily create FontFace::mLoaded. r=heycam, a=gchang
authorBill McCloskey <billm@mozilla.com>
Thu, 13 Oct 2016 16:01:52 -0700
changeset 356293 561f07df81008724f1928748627bed4ad37d77b7
parent 356292 01c159415a042dbbd0bc792762384b1632796393
child 356294 9544b06de18051cde204e6de25617cba4d9f0628
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, gchang
bugs1309867
milestone51.0a2
Bug 1309867 - Part 2: Lazily create FontFace::mLoaded. r=heycam, a=gchang
layout/style/FontFace.cpp
layout/style/FontFace.h
--- a/layout/style/FontFace.cpp
+++ b/layout/style/FontFace.cpp
@@ -96,34 +96,25 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(FontFace)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(FontFace)
 
 FontFace::FontFace(nsISupports* aParent, FontFaceSet* aFontFaceSet)
   : mParent(aParent)
+  , mLoadedRejection(NS_OK)
   , mStatus(FontFaceLoadStatus::Unloaded)
   , mSourceType(SourceType(0))
   , mSourceBuffer(nullptr)
   , mSourceBufferLength(0)
   , mFontFaceSet(aFontFaceSet)
   , mInFontFaceSet(false)
 {
   MOZ_COUNT_CTOR(FontFace);
-
-  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aParent);
-
-  // If the pref is not set, don't create the Promise (which the page wouldn't
-  // be able to get to anyway) as it causes the window.FontFace constructor
-  // to be created.
-  if (global && FontFaceSet::PrefEnabled()) {
-    ErrorResult rv;
-    mLoaded = Promise::Create(global, rv);
-  }
 }
 
 FontFace::~FontFace()
 {
   MOZ_COUNT_DTOR(FontFace);
 
   SetUserFontEntry(nullptr);
 
@@ -193,23 +184,17 @@ FontFace::Constructor(const GlobalObject
 
 void
 FontFace::InitializeSource(const StringOrArrayBufferOrArrayBufferView& aSource)
 {
   if (aSource.IsString()) {
     if (!ParseDescriptor(eCSSFontDesc_Src,
                          aSource.GetAsString(),
                          mDescriptors->mSrc)) {
-      if (mLoaded) {
-        // The SetStatus call we are about to do assumes that for
-        // FontFace objects with sources other than ArrayBuffer(View)s, that the
-        // mLoaded Promise is rejected with a network error.  We get
-        // in here beforehand to set it to the required syntax error.
-        mLoaded->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
-      }
+      Reject(NS_ERROR_DOM_SYNTAX_ERR);
 
       SetStatus(FontFaceLoadStatus::Error);
       return;
     }
 
     mSourceType = eSourceType_URLs;
     return;
   }
@@ -373,16 +358,18 @@ FontFace::Status()
   return mStatus;
 }
 
 Promise*
 FontFace::Load(ErrorResult& aRv)
 {
   mFontFaceSet->FlushUserFontSet();
 
+  EnsurePromise();
+
   if (!mLoaded) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   // Calling Load on a FontFace constructed with an ArrayBuffer data source,
   // or on one that is already loading (or has finished loading), has no
   // effect.
@@ -428,16 +415,18 @@ FontFace::DoLoad()
   mUserFontEntry->Load();
 }
 
 Promise*
 FontFace::GetLoaded(ErrorResult& aRv)
 {
   mFontFaceSet->FlushUserFontSet();
 
+  EnsurePromise();
+
   if (!mLoaded) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   return mLoaded;
 }
 
@@ -462,27 +451,25 @@ FontFace::SetStatus(FontFaceLoadStatus a
   if (mInFontFaceSet) {
     mFontFaceSet->OnFontFaceStatusChanged(this);
   }
 
   for (FontFaceSet* otherSet : mOtherFontFaceSets) {
     otherSet->OnFontFaceStatusChanged(this);
   }
 
-  if (!mLoaded) {
-    return;
-  }
-
   if (mStatus == FontFaceLoadStatus::Loaded) {
-    mLoaded->MaybeResolve(this);
+    if (mLoaded) {
+      mLoaded->MaybeResolve(this);
+    }
   } else if (mStatus == FontFaceLoadStatus::Error) {
     if (mSourceType == eSourceType_Buffer) {
-      mLoaded->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
+      Reject(NS_ERROR_DOM_SYNTAX_ERR);
     } else {
-      mLoaded->MaybeReject(NS_ERROR_DOM_NETWORK_ERR);
+      Reject(NS_ERROR_DOM_NETWORK_ERR);
     }
   }
 }
 
 bool
 FontFace::ParseDescriptor(nsCSSFontDesc aDescID,
                           const nsAString& aString,
                           nsCSSValue& aResult)
@@ -565,19 +552,17 @@ FontFace::SetDescriptors(const nsAString
                        aDescriptors.mDisplay,
                        mDescriptors->mDisplay)) {
     // XXX Handle font-variant once we support it (bug 1055385).
 
     // If any of the descriptors failed to parse, none of them should be set
     // on the FontFace.
     mDescriptors = new CSSFontFaceDescriptors;
 
-    if (mLoaded) {
-      mLoaded->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
-    }
+    Reject(NS_ERROR_DOM_SYNTAX_ERR);
 
     SetStatus(FontFaceLoadStatus::Error);
     return false;
   }
 
   return true;
 }
 
@@ -743,16 +728,50 @@ FontFace::RemoveFontFaceSet(FontFaceSet*
 
   if (mFontFaceSet == aFontFaceSet) {
     mInFontFaceSet = false;
   } else {
     mOtherFontFaceSets.RemoveElement(aFontFaceSet);
   }
 }
 
+void
+FontFace::Reject(nsresult aResult)
+{
+  if (mLoaded) {
+    mLoaded->MaybeReject(aResult);
+  } else if (mLoadedRejection == NS_OK) {
+    mLoadedRejection = aResult;
+  }
+}
+
+void
+FontFace::EnsurePromise()
+{
+  if (mLoaded) {
+    return;
+  }
+
+  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(mParent);
+
+  // If the pref is not set, don't create the Promise (which the page wouldn't
+  // be able to get to anyway) as it causes the window.FontFace constructor
+  // to be created.
+  if (global && FontFaceSet::PrefEnabled()) {
+    ErrorResult rv;
+    mLoaded = Promise::Create(global, rv);
+
+    if (mStatus == FontFaceLoadStatus::Loaded) {
+      mLoaded->MaybeResolve(this);
+    } else if (mLoadedRejection != NS_OK) {
+      mLoaded->MaybeReject(mLoadedRejection);
+    }
+  }
+}
+
 // -- FontFace::Entry --------------------------------------------------------
 
 /* virtual */ void
 FontFace::Entry::SetLoadState(UserFontLoadState aLoadState)
 {
   gfxUserFontEntry::SetLoadState(aLoadState);
 
   for (size_t i = 0; i < mFontFaces.Length(); i++) {
--- a/layout/style/FontFace.h
+++ b/layout/style/FontFace.h
@@ -194,22 +194,34 @@ private:
                nsCSSPropertyID aPropID,
                nsString& aResult) const;
 
   /**
    * Returns and takes ownership of the buffer storing the font data.
    */
   void TakeBuffer(uint8_t*& aBuffer, uint32_t& aLength);
 
+  // Acts like mLoaded->MaybeReject(aResult), except it doesn't create mLoaded
+  // if it doesn't already exist.
+  void Reject(nsresult aResult);
+
+  // Creates mLoaded if it doesn't already exist. It may immediately resolve or
+  // reject mLoaded based on mStatus and mLoadedRejection.
+  void EnsurePromise();
+
   nsCOMPtr<nsISupports> mParent;
 
-  // A Promise that is fulfilled once the font represented by this FontFace
-  // is loaded, and is rejected if the load fails.
+  // A Promise that is fulfilled once the font represented by this FontFace is
+  // loaded, and is rejected if the load fails. This promise is created lazily
+  // when JS asks for it.
   RefPtr<mozilla::dom::Promise> mLoaded;
 
+  // Saves the rejection code for mLoaded if mLoaded hasn't been created yet.
+  nsresult mLoadedRejection;
+
   // The @font-face rule this FontFace object is reflecting, if it is a
   // rule backed FontFace.
   RefPtr<nsCSSFontFaceRule> mRule;
 
   // The FontFace object's user font entry.  This is initially null, but is set
   // during FontFaceSet::UpdateRules and when a FontFace is explicitly loaded.
   RefPtr<Entry> mUserFontEntry;